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

Deserialize HAL responses #30

Closed
viesti opened this issue Dec 20, 2018 · 17 comments
Closed

Deserialize HAL responses #30

viesti opened this issue Dec 20, 2018 · 17 comments
Labels
bug Something isn't working

Comments

@viesti
Copy link

viesti commented Dec 20, 2018

Seems that API Gateway responses are not deserialized currently. I don't know the proper place/way to do deserialization, but here might be a pointer do get the embedded data of the HAL response: https://github.com/portkey-cloud/aws-clj-sdk/blob/master/src/portkey/aws.clj#L182-L197.

@dchelimsky
Copy link
Contributor

The whole response is not deserialized, or do you mean the body? Can you post what you're getting back and what you're expecting to get back?

@viesti
Copy link
Author

viesti commented Dec 20, 2018

Ah sorry, forgot to post a sample :(

user=> (aws/invoke apigw-client {:op :GetRestApis})
{}
user=> (-> (aws/invoke apigw-client {:op :GetRestApis}) meta :http-response :body slurp json/read-json keys)
(:_links :_embedded)

The HTTP response body in the metadata is a JSON string in HAL form. I guess it could be deserialized as the result of aws/invoke.

@viesti
Copy link
Author

viesti commented Dec 21, 2018

Just to clarify, the GetRestApis invocation is able to get the list of APIs.

I expected (aws/invoke apigw-client {:op :GetRestApis}) to return data in form given by the response spec:

user=> (s/describe (aws/response-spec-key apigw-client :GetRestApis))
(keys :opt-un [:cognitect.aws.apigateway.RestApis/position :cognitect.aws.apigateway.RestApis/items])

@dchelimsky dchelimsky added the bug Something isn't working label Dec 21, 2018
@dchelimsky
Copy link
Contributor

Thanks for that additional detail. I expect the same thing you do!

FYI, you can also see the response structure like this:

 user=> (-> apigw-client aws/ops :GetRestApis :response)
 {:position string,
  :items
  [:seq-of
   {:endpointConfiguration  {:types [:seq-of string]},
    :description            string,
    :name                   string,
    :warnings               [:seq-of string],
    :apiKeySource           string,
    :policy                 string,
    :binaryMediaTypes       [:seq-of string],
    :id                     string,
    :createdDate            timestamp,
    :version                string,
    :minimumCompressionSize integer}]}

@dchelimsky dchelimsky changed the title Deserialize API Gateway responses Deserialize HAL responses Dec 28, 2018
@viesti
Copy link
Author

viesti commented Jan 5, 2019

Had a quick look and found client/parse-http-response multimethod, which dispatches on the protocol name, which is rest-json for API Gateway. The content-type http header could further used for selecting how to parse the response:

user> (-> apigw (aws/invoke {:op :GetRestApis}) meta (get-in [:http-response :headers "content-type"]))
"application/hal+json"

@dchelimsky
Copy link
Contributor

Thanks @viesti - that's the same path I'm on. Just haven't had time to focus on this issue.

@viesti
Copy link
Author

viesti commented Jan 5, 2019

No problem, just had an inspiration to look into this. Focus&time are Muses that I find hard to catch :)

@dchelimsky
Copy link
Contributor

Happy muse-hunting! And thanks for checking in. This just might get some attention now ;)

@dchelimsky
Copy link
Contributor

So I started to look into this and discovered (and fixed) an edge-case bug parsing json structures. This is fixed on master, and was necessary for me to move forward w/ the HAL issue. Will keep you posted.

@dchelimsky
Copy link
Contributor

I've got a solution for this on the issue-30-hal-responsesbranch, but I have a few things I want to work through in the code before I merge it.

@viesti
Copy link
Author

viesti commented Jan 9, 2019

Nice! Good thing if this work generally improves the library :)

@dchelimsky
Copy link
Contributor

Agreed! There are other services that also return HAL responses, so this is a bit more general than just apigw.

@viesti
Copy link
Author

viesti commented Jan 9, 2019

True, I found that out myself only recently.

@maxthoursie
Copy link

Sorry for the duplicate! I can verify that the issue-30-hal-responses branch solves my issue!

@dchelimsky
Copy link
Contributor

Thanks for the feedback @maxthoursie. I'm aiming to get a release out today.

@dchelimsky
Copy link
Contributor

Fix merged to master. Release later today.

@dchelimsky
Copy link
Contributor

Released in 0.8.198.

dchelimsky pushed a commit that referenced this issue May 15, 2023
dchelimsky pushed a commit that referenced this issue Jul 10, 2023
scottbale added a commit that referenced this issue Sep 13, 2023
scottbale added a commit that referenced this issue Sep 13, 2023
scottbale added a commit that referenced this issue Sep 13, 2023
scottbale added a commit that referenced this issue Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants