oliyh / martian

The HTTP abstraction library for Clojure/script, supporting OpenAPI, Swagger, Schema, re-frame and more
MIT License
530 stars 44 forks source link

martian-re-frame martian instance has empty `:handlers` #93

Closed enspritz closed 4 years ago

enspritz commented 4 years ago

Using martian-re-frame 0.1.13 and friends. The code sample in the martian-re-frame documentation fails with Uncaught Error: No protocol method ISet.-disjoin defined for type cljs.core/List: ([:my-operation-id {... params ...} :http-success :http-failure]). I noted issue #57 and now have martian initialization completing before performing a re-frame dispatch, but that still results ::http-failure being called with :unknown-route followed by the aforementioned error.

Inspecting the martian value in re-frame's app-db reveals that the :handlers key holds a blank seq. Wondering if this could be source of trouble, I tried feedingmartian.re-frame/init various swagger json documents, such as ones from the official examples repo, our own that we have on hand here, and martian/core/test-resources/swagger.json and martian/core/test-resources/swagger.json. Every document resulted in an empty :handlers. The :base-url changes appropriately, and the :interceptors seem to contain everything that it should, but :handlers is conspicuously empty. This is the case in Firefox and Google Chrome.

At the Clojure-based lein repl, these JSON files uniformly produce a perfectly usable martian, just as advertised, and :handlers is flush with the correct entries. Also, martian's own re-frame test suite cd re-frame && lein test passes 100%. I ran the test suite with the dependency versions in project.clj as tagged in martian v0.1.13 and also separately with the versions we are using here.

oliyh commented 4 years ago

Hi,

Thanks for letting me know. It sounds like there are possibly two issues here - the first problem relating to disjoin and the second with having no handlers (which very much sounds like a failed bootstrap).

The readme should be updated so that it doesn't guide you into repeating the error in #57, as well - I will update that.

There were some reasonably big changes regarding OpenAPI in the latest release and it's possible a mistake was made. Could you try 0.1.12 if possible to see if it worked then?

Many thanks

oliyh commented 4 years ago

Hi,

I've updated the example in the readme and ensured it works. Part of this was updating the example Swagger server, https://pedestal-api.herokuapp.com/, to allows cross origin requests. You should be able to do this in a REPL:

(martian/init "http://pedestal-api.herokuapp.com/swagger.json")
(martian.core/explore @(re-frame/subscribe [::martian/instance]))
;; => [[:all-pets Get all pets in the store] [:create-pet Create a pet] [:get-pet Get a pet by id] [:update-pet Update a pet] [:delete-pet Delete a pet by id]]
enspritz commented 4 years ago

Just checking to see whether I'm running up against an issue patterned after #58 , seems like I am. All the documents I tested (as noted above) were served as text/plain. After changing the web server's behavior to server the OpenAPI definition with Content-Type: application/json;charset=UTF-8 I see that :handlers now contains paths.

Still receiving the uncaught error regarding ISet.-disjoin; I need to tease apart the issues here.

enspritz commented 4 years ago

Much obliged. Cursory experimentation shows that martian-re-frame 0.1.12 appears to exhibit the same behavior regarding whether Content-Type is application/json or not.

enspritz commented 4 years ago

The ISet.-disjoin error is resolved with you change in 0.1.14-SNAPSHOT.

enspritz commented 4 years ago

Yes, material changes in OpenAPI 3... What I am attempting to do here is to consume an OpenAPI 3.0.1 JSON document with martian, but have found that parameters are not being recognized at all. This seems to be implicated in whatever is causing the :unknown-route failures now.

Using martian/explore to inspect at a CLJ REPL (and not CLJS), martian's core/test-resources/openapi.json file seems to work fine, but with my own file all parameters are empty maps. I'm now comparing the two files to iteratively eliminate differences and have parameters recognized, but to no avail so far.

enspritz commented 4 years ago

Sample OpenAPI def and REPL session:

{
  "openapi": "3.0.1",
  "info": {
    "title": "My API",
    "version": "1"
  },
  "paths": {
    "/project/{projectKey}": {
      "get": {
        "summary": "Get specific values from a configuration for a specific project",
        "operationId": "getProjectConfiguration",
        "parameters": [
          {
            "name": "projectId",
            "in": "path",
            "description": "Project ID",
            "required": true,
            "schema": {
              "type": "string",
              "format": "string"
            }
          },
          {
            "name": "key",
            "in": "query",
            "description": "Obtains values corresponding to these keys from a project's configuration",
            "schema": {
              "type": "string",
              "format": "string"
            }
          }
        ],
        "responses": {
          "200": {
            "description": "Configuration for the specified project",
            "content": {
              "application/json": {
                "schema": {
                  "type": "string",
                  "format": "string"
                }
              }
            }
          },
          "403": {
            "description": "Refusing access to requested resource, perhaps due to insufficient privilege"
          },
          "404": {
            "description": "Requested resource was not found"
          }
      }
   }
}

In lein repl:

user=> (require '[martian.core :as martian])
nil
user=> (require '[martian.clj-http :as martian-http])
nil
user=> (def m (martian-http/bootstrap-openapi "http://localhost:8000/api"))
#'user/m
user=> (martian/explore m :get-project-configuration)
{:summary "Get specific values from a configuration for a specific project", :parameters {}, :returns {200 java.lang.String, 403 nil, 404 nil}}
enspritz commented 4 years ago

Tried converting the OpenAPI 3 doc to Swagger 2 and directing martian to use that Swagger 2 output, and it seems to function correctly ^_^

me $ api-spec-converter --from=openapi_3 --to=swagger_2 --syntax=json ./src/api.json

In lein repl:

user=> (martian/explore m :get-project-configuration)
{:summary "Get specific values from a configuration for a specific project", :parameters {:project-key java.lang.String, #schema.core.OptionalKey{:k :key} (maybe java.lang.String)}, :returns {200 java.lang.String, 403 Any, 404 Any}}
user=> (martian/request-for m :get-project-configuration {:projectKey "kitty"})
{:method :get, :url "http://localhost:8000/api/project/123", :as :text, :headers {"Accept" "application/json"}}
enspritz commented 4 years ago

So one thing I draw from this exercise is that either my OpenAPI document, while other tooling and the online validators are satisfied with it, requires a little more refinement in order to satisfy martian's needs, or, perhaps martian requires a little more TLC with regard to parsing OpenAPI 3 as you had mentioned..

oliyh commented 4 years ago

Hi,

Thank you so much for such a detailed investigation. The OpenAPI stuff is very new and I would imagine the issue lies there. Thanks for the small reproducible case - that really helps.

Martian should be able to cope with any valid schema, with the one caveat about operation-ids.

oliyh commented 4 years ago

Hi,

There was indeed a mistake, I have pushed 0.1.14-SNAPSHOT which should fix your problem. If so I will do a new release. Thanks again for the report and the help reproducing!

enspritz commented 4 years ago

I can confirm 0.1.14-SNAPSHOT fixes the params problem as well. Thank you kindly!