f-f / gogolica

Auto-generated Google APIs for Clojure
Eclipse Public License 1.0
2 stars 0 forks source link

Handle post and put methods #5

Closed f-f closed 7 years ago

f-f commented 7 years ago

Right now we're only handling get methods, while we should support all the http verbs.

zudov commented 7 years ago

Sadly, the API models don't seem to include any info regarding what the bodies should be. I hoped they'll have a parameter with location: body and required, description, format fields.

We were planning that all API functions will have a map as a last parameter into which the optional parameters should be placed. The body can be placed there under a body key and then we can encode it appropriately depending on the type (sometimes it would be just json object, sometimes InputStream, sometimes a string). Potentially something like that.

zudov commented 7 years ago

As it turns out "Media Upload" is well described and handled by their own endpoints to which the methods that require media upload refer via supportMediaUpload and mediaUpload. More details here.

E.g. haskell's gogol provides an upload function that handles that generically.

f-f commented 7 years ago

Oh this is very nice. I guess we can just port the same behavior as gogol.

f-f commented 7 years ago

I just realized that the title of this issue is probably not super nice.
What I really meant with it is that:

So with "fix post and put" I actually meant "handle query parameters". This means that we have to implement them also on GET requests, I guess putting them in the url instead of a body map. Should we have another issue for that?

f-f commented 7 years ago

@zudov I added some preliminary support for this, in commit https://github.com/f-f/googlica/commit/e522994d70b3c82f14fcfa30d77131f923da4288. Now the parameters are added to the request as :body if it's a POST, or as query params if it's a GET. Not sure if this is enough to consider this "closed", do we want to handle the upload thing here?

My goal with the latest commits is to get a PoC to easily test against the real Google services and see asap if all works as intended. Makes sense?

zudov commented 7 years ago

That all makes sense, and I think we can have another issue for upload things.

However.

Now the parameters are added to the request as :body if it's a POST, or as query params if it's a GET.

I checked around and that doesn't seem right. For instance storage.buckets.insert takes some of its arguments through the body and some as query parameters... Both body and query have required parameters.

f-f commented 7 years ago

Ouch, right.

So, things to fix:

  1. switch to using :query-params for GETs
  2. if the method description has a request key, then we need to supply the body too. E.g. this is what the specification for storage.buckets.inserts looks like:
    {:description "Creates a new bucket.",
    :httpMethod "POST",
    :id "storage.buckets.insert",
    :parameterOrder ["project"],
    :parameters {:predefinedAcl {:description "Apply a predefined set of access controls to this bucket.",
                              :enum ["authenticatedRead"
                                     "private"
                                     "projectPrivate"
                                     "publicRead"
                                     "publicReadWrite"],
                              :enumDescriptions ["Project team owners get OWNER access, and allAuthenticatedUsers get READER access."
                                                 "Project team owners get OWNER access."
                                                 "Project team members get access according to their roles."
                                                 "Project team owners get OWNER access, and allUsers get READER access."
                                                 "Project team owners get OWNER access, and allUsers get WRITER access."],
                              :location "query",
                              :type "string"},
              :predefinedDefaultObjectAcl {:description "Apply a predefined set of default object access controls to this bucket.",
                                           :enum ["authenticatedRead"
                                                  "bucketOwnerFullControl"
                                                  "bucketOwnerRead"
                                                  "private"
                                                  "projectPrivate"
                                                  "publicRead"],
                                           :enumDescriptions ["Object owner gets OWNER access, and allAuthenticatedUsers get READER access."
                                                              "Object owner gets OWNER access, and project team owners get OWNER access."
                                                              "Object owner gets OWNER access, and project team owners get READER access."
                                                              "Object owner gets OWNER access."
                                                              "Object owner gets OWNER access, and project team members get access according to their roles."
                                                              "Object owner gets OWNER access, and allUsers get READER access."],
                                           :location "query",
                                           :type "string"},
              :project {:description "A valid API project identifier.",
                        :location "query",
                        :required true,
                        :type "string"},
              :projection {:description "Set of properties to return. Defaults to noAcl, unless the bucket resource specifies acl or defaultObjectAcl properties, when it defaults to full.",
                           :enum ["full" "noAcl"],
                           :enumDescriptions ["Include all properties."
                                              "Omit owner, acl and defaultObjectAcl properties."],
                           :location "query",
                           :type "string"}},
    :path "b",
    :request {:$ref "Bucket"},
    :response {:$ref "Bucket"},
    :scopes ["https://www.googleapis.com/auth/cloud-platform"
          "https://www.googleapis.com/auth/devstorage.full_control"
          "https://www.googleapis.com/auth/devstorage.read_write"]}

But this means that we probably need to parse the JSON-schema descriptions for the objects, to define constructors generically. So I guess that for a PoC against their services some things (e.g. this one) can be hardcoded. Also, the schema parsing probably also deserves an issue.

f-f commented 7 years ago

Update 1: the 1. from the previous comment is fixed in commit https://github.com/f-f/googlica/commit/a707a05e51149e26bed6073cb98ec20951e88be4 Update 2: opened an issue for mediaDownload, #12

zudov commented 7 years ago

So I guess that for a PoC against their services some things (e.g. this one) can be hardcoded.

How about we just allow supplying whatever body for now, it would feel pretty lacking, but we won't need to do any temporary hardcoding. For example we could allow :body key in the last argument (currently named optional-params), the contents would be put in the body of a request as-is.

f-f commented 7 years ago

Currently the generate-args function knows when the method expects to have a body, and makes space for it as the first parameter, but the actual request just sends an empty body. I agree on just passing whatever comes in as the body, it's the simplest way to fix.

A tangential comment about the optional-params: we should probably have them as really optional, with a multiple arity version of the function that doesn't take the map as last argument. Having to pass an empty map makes for an awkward UI IMO.

f-f commented 7 years ago

@zudov I added the "supply whatever body" capability in this latest linked commit ^ Given that we are tracking schema/spec generation in #10, is this completed?

zudov commented 7 years ago

Awesome