oliyh / martian

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

Patching schema after bootstrap #194

Closed telenieko closed 10 months ago

telenieko commented 10 months ago

Hi there,

First, thanks for Martian. After having used REST APIs in Python & Java Martian is simply amazing. What would take a couple hundred lines in Java takes a couple dozen in Clojure with it.

Now to the issue:

I am not sure if this is even possible; so not sure if this is a feature request, a documentation enhancement or a cry for help! I asked in Clojurians Slack and first answer was that it is probably not possible.

Is it possible to patch a schema after bootstrap?

Let me bring a concrete example, the Brevo API (formerly Sendinblue).

On this API you can define what they call "Contact Attributes" which are used defined fields within a Contact. On the API those go inside an attributes parameter on the related API calls.

So, let's bootstrap the client:

(require
   '[martian.core :as martian]
   '[martian.hato :as martian-http]
   '[schema.core :as s]
   '[clojure.set :as set])

(def brevo-api-spec "https://api.brevo.com/v3/swagger_definition.yml")
(def brevo-api-key (System/getenv "BREVO_API_KEY"))

(def add-brevo-authentication-header
  {:name ::add-authentication-header
   :enter (fn [ctx]
            (assoc-in ctx [:request :headers "Api-key"] brevo-api-key))})

(def brevo-api (martian-http/bootstrap-openapi
            brevo-api-spec
            {:interceptors (concat
                            [add-brevo-authentication-header]
                            martian-http/default-interceptors)}))

So, with create-contact for example, you can define those extra fields in attributes:

defn create-brevo-contact [brevo-api {:keys [email name control periodicidad]}]
  (martian/response-for brevo-api :create-contact
                        {::martian/body 
                         {:email email
                          :update-enabled true
                          :attributes
                          {:NOMBRE name
                           :FISCAL_GRIST control
                           :PERIODICIDAD_DOCS periodicidad}}}))

Which sadly will result in:

Execution error (ExceptionInfo) at schema-tools.coerce/coerce-or-error! (coerce.cljc:24).
Could not coerce value to schema: {:createContact {:attributes {:NOMBRE disallowed-key, :FISCAL_GRIST disallowed-key, :PERIODICIDAD_DOCS disallowed-key}}}

So, my idea was "who cares, I just have to patch the schema of create-contact to add my custom attributes:

(def brevo-attributes-schema
  {:NOMBRE s/Str
   :FISCAL_GRIST s/Bool
   :PERIODICIDAD_DOCS s/Str})

But... I have no idea how to do that. So...

Is it even possible to patch the schema/API definition of the client after bootstrap? or even during bootstrap; but after loading the Swagger definition?

thx.

oliyh commented 10 months ago

Hi,

That was actually me you were talking with on Slack :)

You have two options as it stands:

  1. Simply change your yaml spec file to include these attributes. Will work out of the box, the downside being that you can't conveniently update that yaml file from source anymore
  2. Take the bootstrapped martian, walk it to patch the handlers you want to modify, and use that data to construct a new martian using the non-swagger constructor. An instance of martian is just data, there is no special state or anything.

If I were to implement this in martian it would be based along the lines of (2), so if you try it and it feels like it could be a useful addition I'd love to see your implementation.

Cheers

telenieko commented 10 months ago

That was actually me you were talking with on Slack :)

🤦😅

  1. Take the bootstrapped martian, walk it to patch the handlers you want to modify, and use that data to construct a new martian using the non-swagger constructor. [...] so if you try it and it feels like it could be a useful addition I'd love to see your implementation.

Full disclaimer: I'm just starting with Clojure, this is the first "real world problem" I am solving. So my code might be ugly and not very "clojuristic" (I come from Python). Any feedback is most welcome, specially on how to write more "clojuresque" code.

I have not found a way to take an existing martian and bootstrap a new one from it, but I found my way with martian/update-handler.

Following from the code in the description above, this is what I have done:

(require '[schema-tools.core :as st])

(let [api brevo-api
      route-name :create-contact]
  (let [handler (martian/handler-for api route-name)
        schema (get-in handler [:body-schema :createContact])
        new-schema (st/assoc-in schema [:attributes] brevo-attributes-schema)]
    (martian/update-handler api route-name assoc-in [:body-schema :createContact] new-schema)))

This returns me a martian with the updated schema. And it seems to work. I am now reading about Clojure schemas & schema-tools to properly understand what I am doing and to fix corner cases (like if PERIODICIDAD_DOCS is nil I need to coerce to empty string, "")

This does work, the code looks ugly to me but again.. newbie! :)

oliyh commented 10 months ago

What a coincidence, I'm just starting out with Python!

You are quite right about update-handler, I had forgotten it existed. It gives you the ability to update a specific handler in any way you please: https://github.com/oliyh/martian#per-route-behaviour . It returns the new, updated martian (no mutation).

Your code is perfectly readable but there are a couple of small improvements I would suggest (and you should have a look at clj-kondo, a linter which will help you learn these conventions and idioms):

  1. Nested lets can be combined into a single let (although after refactor 2, you don't really need the let anyway as each binding is only used once)
  2. You could use update-in instead of first reading out the schema and then overwriting with your modified one
(let [api brevo-api
      route-name :create-contact]
   (martian/update-handler api route-name update-in [:body-schema :createContact] st/assoc-in [:attributes] brevo-attributes-schema))

It's much terser, but once you get used to the currying of update-in and assoc-in it should look clearer.

I'm going to close this issue as complete, but please feel free to continue to ask questions or reopen if you run into an issue using this approach.

Cheers

telenieko commented 10 months ago

Thank you very much for the tips. I will setup clj-kondo !

The nested let is to later change the first one into a defn.

I like your use of update-in, looks much nicer. Thx.

OT: What is driving me away of Python more and more is a) changes to the language, b) still today virtualenv & dependency management is... not optimal. But I do not do Data Science, which I guess is what drives most people to Python today. -- I started with Python around 2004 because of Django, just as many started with Ruby because of RoR :D