Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support XML? #2188

Open
fjf2002 opened this issue Mar 9, 2022 · 14 comments
Open

feat: support XML? #2188

fjf2002 opened this issue Mar 9, 2022 · 14 comments
Labels
enhancement a feature, ready for implementation

Comments

@fjf2002
Copy link
Contributor

fjf2002 commented Mar 9, 2022

Is there any interest in this project to support creating SOAP endpoints? I don't think it would be hard.

I am building upon #1927, especially:
Uploading raw text to a function. The function must be defined with a single unnamed text param and Content-Type: text/plain must be specified.

My investigation until now with postgrest 9.0: It works with...

  1. I have a function soap_endpoint(text) RETURNS text ...
  2. The HTTP request must contain header "Content-Type: text/plain"
  3. The HTTP request must contain header "Accept: text/plain" (otherwise the body seems to get enclosed in double quotes)
  4. My function currently sets PERFORM set_config('response.headers', '[{"content-type": "text/xml; charset=utf-8"}]', true);
  5. If my function returned data type xml, this would fail with Postgres error "Function string_agg(xml, unknown) does not exist".

But it would be nicer to be able to write a function soap_endpoint(xml) RETURNS xml ... and especially not having to deal with the HTTP header stuff.

Suggested changes:

  • To remedy my point 2, I think we would need (analogous to feat: RPC POST for function w/single unnamed param  #1927):
    Uploading XML to a function. The function must be defined with a single unnamed xml param and Content-Type: text/xml or application/xml or application/soap+xml must be specified.

  • To remedy my points 3 and 4, I think we would need: If function return type is xml, automatically set response content-type to text/xml (or something like that)

  • To remedy my point 5, I think we would need: Allow returning xml data type, i. e. solve the string_agg issue - perhaps use xmlagg, or only allow scalar xml return values?

@steve-chavez
Copy link
Member

Yeah, we'd like to support XML endpoints as well.

I get your point about messing with headers. We had a long discussion on #1582 about how to send raw bodies with the right content-type headers.

Uploading XML to a function. The function must be defined with a single unnamed xml param and Content-Type: text/xml or application/xml or application/soap+xml must be specified.
If function return type is xml, automatically set response content-type to text/xml (or something like that)
perhaps use xmlagg, or only allow scalar xml return values??

That would be another option yes, it's less flexible than the approach in #1582 but seems much simpler to implement. Would you like to give it a shot?

For me the right approach would be the generic one in #1582 but I don't think there's any harm in providing a shortcut for XML responses in postgrest's core.

@wolfgangwalther wolfgangwalther added the enhancement a feature, ready for implementation label Mar 10, 2022
@wolfgangwalther wolfgangwalther changed the title feat: support SOAP? feat: support XML? Mar 10, 2022
@wolfgangwalther
Copy link
Member

I don't think we should support SOAP out of the box, but I fully agree with XML support in general. And then maybe ppl can build on top of that for SOAP support.

@fjf2002
Copy link
Contributor Author

fjf2002 commented Mar 17, 2022

Thank you for your replies.

@steve-chavez:

  • Do I understand correctly I currently can't try the suggestion in Add pgrst.accept setting to RPC #1582 with "pgrst.accept" because it's not yet implemented? That would be a convenient solution to my bullet 3.

  • What about the other problems I've mentioned?

Regards,
fjf2002

@wolfgangwalther
Copy link
Member

Do I understand correctly I currently can't try the suggestion in #1582 with "pgrst.accept" because it's not yet implemented?

Correct, this is not implemented, yet.

@fjf2002
Copy link
Contributor Author

fjf2002 commented Mar 23, 2022

bump
What about my two remaining problems, i. e.

  • Allow functions with a single unnamed XML parameter with a request header "Content-Type: text/xml"; in addition to the Content-Types in feat: RPC POST for function w/single unnamed param  #1927
  • Allow function return type XML (this currently fails with Postgres error "Function string_agg(xml, unknown) does not exist").

@wolfgangwalther
Copy link
Member

* Allow functions with a single unnamed XML parameter with a request header "Content-Type: text/xml"; in addition to the Content-Types in [feat: RPC POST for function w/single unnamed param  #1927](https://github.com/PostgREST/postgrest/pull/1927)

This should be straight-forward to implement. Feel free to open a Pull Request.

* Allow function return type XML (this currently fails with Postgres error "Function string_agg(xml, unknown) does not exist").

For now, you could try to create a custom aggregate with the same name and type, that would just cast xml to text and aggregate that?

Not sure how easy it would be to implement using xmlagg(...) in PostgREST, but certainly possible.

@fjf2002
Copy link
Contributor Author

fjf2002 commented Mar 24, 2022

This should be straight-forward to implement. Feel free to open a Pull Request.

It seems I am too dumb for Haskell. I did the following; it fails with 'Could not find the public.x function with a single unnamed xml parameter in the schema cache'. Any clues?

DB:

create or replace function public.x(xml) returns text language sql as $$
	select '42'
$$

Curl:

curl -vvvv --header "Content-Type: text/xml" --data '<xml>xmltest</xml>' localhost:3000/rpc/x

Haskell:

diff --git a/src/PostgREST/ContentType.hs b/src/PostgREST/ContentType.hs
index 7be307e..40c4a13 100644
--- a/src/PostgREST/ContentType.hs
+++ b/src/PostgREST/ContentType.hs
@@ -20,6 +20,7 @@ data ContentType
   | CTSingularJSON
   | CTTextCSV
   | CTTextPlain
+  | CTTextXML
   | CTOpenAPI
   | CTUrlEncoded
   | CTOctetStream
@@ -41,6 +42,7 @@ toMime :: ContentType -> ByteString
 toMime CTApplicationJSON = "application/json"
 toMime CTTextCSV         = "text/csv"
 toMime CTTextPlain       = "text/plain"
+toMime CTTextXML         = "text/xml"
 toMime CTOpenAPI         = "application/openapi+json"
 toMime CTSingularJSON    = "application/vnd.pgrst.object+json"
 toMime CTUrlEncoded      = "application/x-www-form-urlencoded"
@@ -55,6 +57,7 @@ decodeContentType ct =
     "application/json"                  -> CTApplicationJSON
     "text/csv"                          -> CTTextCSV
     "text/plain"                        -> CTTextPlain
+    "text/xml"                          -> CTTextXML
     "application/openapi+json"          -> CTOpenAPI
     "application/vnd.pgrst.object+json" -> CTSingularJSON
     "application/vnd.pgrst.object"      -> CTSingularJSON
diff --git a/src/PostgREST/Error.hs b/src/PostgREST/Error.hs
index 60880c1..2147493 100644
--- a/src/PostgREST/Error.hs
+++ b/src/PostgREST/Error.hs
@@ -135,6 +135,7 @@ instance JSON.ToJSON ApiRequestError where
       (case (hasPreferSingleObject, isInvPost, contentType) of
         (True, _, _)                 -> " function with a single json or jsonb parameter"
         (_, True, CTTextPlain)       -> " function with a single unnamed text parameter"
+        (_, True, CTTextXML)         -> " function with a single unnamed xml parameter"
         (_, True, CTOctetStream)     -> " function with a single unnamed bytea parameter"
         (_, True, CTApplicationJSON) -> prms <> " function or the " <> schema <> "." <> procName <>" function with a single unnamed json or jsonb parameter"
         _                            -> prms <> " function") <>
diff --git a/src/PostgREST/Request/ApiRequest.hs b/src/PostgREST/Request/ApiRequest.hs
index 609192d..b9745b4 100644
--- a/src/PostgREST/Request/ApiRequest.hs
+++ b/src/PostgREST/Request/ApiRequest.hs
@@ -251,6 +251,7 @@ apiRequest conf@AppConfig{..} dbStructure req reqBody queryparams@QueryParams{..
       let paramsMap = M.fromList $ (T.decodeUtf8 *** JSON.String . T.decodeUtf8) <$> parseSimpleQuery (LBS.toStrict reqBody) in
       Right $ ProcessedJSON (JSON.encode paramsMap) $ S.fromList (M.keys paramsMap)
     (CTTextPlain, True) -> Right $ RawPay reqBody
+    (CTTextXML, True) -> Right $ RawPay reqBody
     (CTOctetStream, True) -> Right $ RawPay reqBody
     (ct, _) -> Left $ "Content-Type not acceptable: " <> ContentType.toMime ct
   topLevelRange = fromMaybe allRange $ M.lookup "limit" ranges -- if no limit is specified, get all the request rows
@@ -421,7 +422,7 @@ requestContentTypes conf action path =
 
 rawContentTypes :: AppConfig -> [ContentType]
 rawContentTypes AppConfig{..} =
-  (ContentType.decodeContentType <$> configRawMediaTypes) `union` [CTOctetStream, CTTextPlain]
+  (ContentType.decodeContentType <$> configRawMediaTypes) `union` [CTOctetStream, CTTextPlain, CTTextXML]
 
 {-|
   Search a pg proc by matching name and arguments keys to parameters. Since a function can be overloaded,
@@ -452,6 +453,7 @@ findProc qi argumentsKeys paramsAsSingleObject allProcs contentType isInvPost =
       (CTApplicationJSON, "json")  -> True
       (CTApplicationJSON, "jsonb") -> True
       (CTTextPlain, "text")        -> True
+      (CTTextXML, "xml")           -> True
       (CTOctetStream, "bytea")     -> True
       _                            -> False
     hasSingleUnnamedParam _ = False
@@ -465,7 +467,7 @@ findProc qi argumentsKeys paramsAsSingleObject allProcs contentType isInvPost =
         then length params == 1 && (firstType == Just "json" || firstType == Just "jsonb")
       -- If the function has no parameters, the arguments keys must be empty as well
       else if null params
-        then null argumentsKeys && not (isInvPost && contentType `elem` [CTTextPlain, CTOctetStream])
+        then null argumentsKeys && not (isInvPost && contentType `elem` [CTOctetStream, CTTextPlain, CTTextXML])
       -- A function has optional and required parameters. Optional parameters have a default value and
       -- don't require arguments for the function to be executed, required parameters must have an argument present.
       else case L.partition ppReq params of

@fjf2002
Copy link
Contributor Author

fjf2002 commented Jun 2, 2022

It seems I am too dumb for Haskell. I did the following; it fails with 'Could not find the public.x function with a single unnamed xml parameter in the schema cache'. Any clues?

The problem was not in Haskell code, but in the sql code for getting the procedures - which already filtered single-unnamed-xml-param procedures out. See PR #2300.

@fjf2002
Copy link
Contributor Author

fjf2002 commented Oct 31, 2023

As stated in https://postgrest.org/en/stable/how-tos/create-soap-endpoint.html ,

Unfortunately the Accept: text/xml header is currently mandatory concerning PostgREST, otherwise it will respond with a Content-Type: application/json header and enclose the response with quotes. (You can check the returned headers by adding -v to the curl call.)

I've noticed the work on #2825. Great! Will it work when I omit the Accept: text/xml request header?

@steve-chavez
Copy link
Member

steve-chavez commented Nov 3, 2023

@fjf2002. Not yet. That would mean overriding the default */* handler and:

-- TODO initial */* is not overridable

That still defaults to application/json.

On #1582 (comment), there was the idea of doing:

CREATE DOMAIN "*/*" AS TEXT;
CREATE DOMAIN "text/csv" AS TEXT;

-- does this mean text/csv is now my default return type?
CREATE CAST ("text/csv" AS "*/*") WITH INOUT;

To be able to change the */* handler. Not sure if that's the best way though because it would always be global.

@steve-chavez
Copy link
Member

@fjf2002 I've made a proposal for solving the above on #3037. Let me know what you think.

cc @wolfgangwalther

@steve-chavez
Copy link
Member

@fjf2002 Check https://postgrest.org/en/v12.0/references/api/media_type_handlers.html#the-any-handler, now possible to handle */* and application/soap+xml

@fjf2002
Copy link
Contributor Author

fjf2002 commented Dec 4, 2023

Thank You! Can that work with a function with scalar-valued return type?

@steve-chavez
Copy link
Member

@fjf2002 Yes, there are tests for that (no docs though):

create or replace function ret_any_mt ()
returns "*/*" as $$
select 'any'::"*/*";
$$ language sql;
create or replace function ret_some_mt ()
returns "*/*" as $$
declare
req_accept text := current_setting('request.headers', true)::json->>'accept';
resp bytea;
begin
case req_accept
when 'app/chico' then resp := 'chico';
when 'app/harpo' then resp := 'harpo';
when '*/*' then
perform set_config('response.headers', '[{"Content-Type": "app/groucho"}]', true);
resp := 'groucho';
else
raise sqlstate 'PT415' using message = 'Unsupported Media Type';
end case;
return resp;
end; $$ language plpgsql;

context "any media type" $ do
context "on functions" $ do
-- TODO not correct, it should return the generic "application/octet-stream"
it "returns application/json for */* if not explicitly set" $ do
request methodGet "/rpc/ret_any_mt" (acceptHdrs "*/*") ""
`shouldRespondWith` "any"
{ matchStatus = 200
, matchHeaders = [matchContentTypeJson]
}
it "accepts any media type and sets it as a header" $ do
request methodGet "/rpc/ret_any_mt" (acceptHdrs "app/bingo") ""
`shouldRespondWith` "any"
{ matchStatus = 200
, matchHeaders = ["Content-Type" <:> "app/bingo"]
}
request methodGet "/rpc/ret_any_mt" (acceptHdrs "text/bango") ""
`shouldRespondWith` "any"
{ matchStatus = 200
, matchHeaders = ["Content-Type" <:> "text/bango"]
}
request methodGet "/rpc/ret_any_mt" (acceptHdrs "image/boingo") ""
`shouldRespondWith` "any"
{ matchStatus = 200
, matchHeaders = ["Content-Type" <:> "image/boingo"]
}
it "returns custom media type for */* if explicitly set" $ do
request methodGet "/rpc/ret_some_mt" (acceptHdrs "*/*") ""
`shouldRespondWith` "groucho"
{ matchStatus = 200
, matchHeaders = ["Content-Type" <:> "app/groucho"]
}
it "accepts some media types if there's conditional logic" $ do
request methodGet "/rpc/ret_some_mt" (acceptHdrs "app/chico") ""
`shouldRespondWith` "chico"
{ matchStatus = 200
, matchHeaders = ["Content-Type" <:> "app/chico"]
}
request methodGet "/rpc/ret_some_mt" (acceptHdrs "app/harpo") ""
`shouldRespondWith` "harpo"
{ matchStatus = 200
, matchHeaders = ["Content-Type" <:> "app/harpo"]
}
request methodGet "/rpc/ret_some_mt" (acceptHdrs "text/csv") ""
`shouldRespondWith` 415

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement a feature, ready for implementation
Development

No branches or pull requests

3 participants