search

cognitect-labs / vase

Data driven microservices
Eclipse Public License 1.0
373 stars 42 forks source link

Support edn coercion in vase/transact #68

Closed deg closed 5 years ago

deg commented 7 years ago

The current Vase syntax does not make it easy to create new entities with non-string types. :edn-coerce is supported for #vase/query, but not for #vase/transact.

So, this common operation can only be done currently with an interceptor. This pattern appears a couple of times in the current samples, e.g. https://github.com/cognitect-labs/vase/blob/master/samples/petstore-full/src/petstore_full/interceptors.clj#L10. To me, this pattern appears verbose and fragile, especially for such a common operation.

I see several possible ways to extend the syntax:

  1. Support :edn-coerce in #vase/transact maps. But, this is awkward, since the current syntax refers only to the fully-namespaced keywords in the :properties vector, with no :params bindings.
  2. Extend the syntax of :properties. Currently this is a vector of keywords. Each keyword could optionally be a map including, e.g., :edn-coerce true.
  3. Extend the syntax of #vase/transact. Add a :params bindings clause. This can then support end coercion in a syntax exactly parallel to #vase/query. It could also offer an option to define entity-creating routes where the exposed API parameter name does not have to exactly match the internal database attribute keyword.
  4. Extend :params in both #vase/transact and #vase/query. This addresses an additional issue too. :edn-coerce feels a bit unnatural to me. It would be cleaner to have this typing request be part of each parameter in the :params vector.
ohpauleez commented 7 years ago

On master, the transact action literal supports JSON, Transit, and edn. The default data exchange format for a Vase service is JSON, with all of the limitations of JSON (as well as its ubiquity). transact doesn't have :edn-coerce because the value types are determined by the data exchange format -- You should only have string data if you actually send string values as part of the JSON payload.

If you have UUID/Instant/Custom data type representations in your schema, you ether have to "double-parse" as done in the link you provided (this is typically what most JSON services do, regardless of language/platform), or change data formats for your given API (which can be seen in this example)

deg commented 7 years ago

I see, thanks.

My misunderstanding came from a very different perspective, which may be useful:

The current implementation is thinking of type information coming only from the exchange format. But, especially in the case of transact, we have a second source of type clues: the type associated with the entity attribute in the :vase.norm/txes.

I've already declared [:myNs/date :one :instant "..."]. Vase can reduce the amount of boilerplate I need to write, by letting me specify an option to cast the incoming string into an instant. The same holds true, of course, for all the other non-string types.

ddeaguiar commented 5 years ago

I think it's preferable to keep entity coercions based on entity attributes out of core. That change is of low value considering that changing data exchange format should be sufficient in the majority of cases. If anything, the exceptions would likely require knowledge of the application context (i.e., domain, etc.), requiring a custom implementation, to return the correct result. Each data exchange format has it's tradeoffs, it's up to the service developer to consider these and accounting for them during development.