agco / hapi-harvester

A JSONAPI 1.0 plugin for Hapi.
14 stars 3 forks source link

Allow the definition of unique field constraints #39

Open dclucas opened 8 years ago

dclucas commented 8 years ago

Tricky one: the JSONAPI option for UUIDs as PKs enforces a surrogate key model, causing natural keys to not be explicitly represented. For example, a user may be identified by his/her email, but still have a UUID as the record key.

In the email example, it is still important to enforce uniqueness. Currently there is no way in hapi-harvester to declare that restriction (and none in Joi or even JSON Schema, as far as I could tell), but this is still a constraint that should be validated and communicated.

My suggestion is to break this in three steps:

  1. first, document the workaround with our current functionalities in readme.io
  2. second, to find an elegant way to declare that constraint within the API (joi seems to have extension points for that). This can be tricky because some constraints may be based on composite keys (eg: can't have the same product code repeated under a brand)
  3. last, to provide functionalities that enforce this rule
mavdi commented 8 years ago

Hm this is tricky. joi was never purposed to be handling uniqueness as this requires a reference to DB and ultimately an async operation. There seems to be a thread about this: https://github.com/hapijs/joi/issues/577

Also there is yup which supports async ops:

https://github.com/jquense/yup

Also we could see if we can use something like this:

validate: {
  payload: function (value, options, next) {
    var r = Joi.validate(value, schema, options);
    if (r.errors) {
      return next(r.errors);
    }
    doSomethingAsync(r.value, next); // re-use the joi mutated r.value for type casts
  }
}
dclucas commented 8 years ago

Frankly, dunno even if we want to provide the full functionality (of enforcing the rule), since it requires, as you mention, heavy interaction with the db.

I would be happy to settle on a good way to just document uniqueness so it gets into swagger.

kristofsajdak commented 8 years ago

I completely agree that we should communicate these kind of constraints within our API docs, however after browsing the swagger spec for a bit I couldn't find any straightforward answer on how to do this.

There are no dedicated spec elements that describe (composite) field value uniqueness as far as I can see, so would suggest to align on this first and work our way backwards from there

kristofsajdak commented 8 years ago

My first instinct is that we should capture this as part of the swagger 'responses' section to describe what is returned when a constraint is violated. This can be generated with hapi-swagger from the response/status section in the route definition.

https://github.com/glennjones/hapi-swagger/blob/a8728f086e9c09f88a9b32ecd99d56d8bc82e0a4/test/responses-tests.js#L240

This seems to be quite limited though in terms of spec support, it appears you can only define 1 response message for 1 error code. https://github.com/OAI/OpenAPI-Specification/issues/398

kristofsajdak commented 8 years ago

More chat on this topic : https://github.com/OAI/OpenAPI-Specification/issues/270