holidayextras / jsonapi-store-relationaldb

A relational database handler for jsonapi-server
MIT License
33 stars 32 forks source link

Creates work even if validation fails; Record not retrievable #37

Open bitsoflogic opened 8 years ago

bitsoflogic commented 8 years ago

Debugging (with handler logging through bunyan):

  jsonApi:requestCounter 3 +39s POST /users
  jsonApi:validation:input {"id":"2f14ceb5-81fd-4b29-ba93-48a87209b0ad","type":"users","name":"jason","email":"j@gmail.com"} +1ms
[2016-07-17T14:45:01.399Z] TRACE: test-jsonapi-server/3139 on jason-chaletos: Executing (50444f2a-4cd8-4b2a-9f9d-5ded28340b0f): START TRANSACTION;
[2016-07-17T14:45:01.400Z] TRACE: test-jsonapi-server/3139 on jason-chaletos: Executing (50444f2a-4cd8-4b2a-9f9d-5ded28340b0f): SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED;
[2016-07-17T14:45:01.403Z] TRACE: test-jsonapi-server/3139 on jason-chaletos: Executing (50444f2a-4cd8-4b2a-9f9d-5ded28340b0f): INSERT INTO "users" ("id","type","name","email") VALUES ('2f14ceb5-81fd-4b29-ba93-48a87209b0ad','users','jason','j@gmail.com') RETURNING *;
[2016-07-17T14:45:01.406Z] TRACE: test-jsonapi-server/3139 on jason-chaletos: Executing (50444f2a-4cd8-4b2a-9f9d-5ded28340b0f): COMMIT;
  jsonApi:handler:create {"type":"users","data":{"attributes":{"name":"jason","email":"j@gmail.com"},"type":"users"}} +20ms [null,{"id":"2f14ceb5-81fd-4b29-ba93-48a87209b0ad","type":"users","name":"jason","email":"j@gmail.com"}]
[2016-07-17T14:45:01.413Z] TRACE: test-jsonapi-server/3139 on jason-chaletos: Executing (default): SELECT "id", "type", "meta", "name", "email", "roles" FROM "users" AS "users" WHERE "users"."id" = '2f14ceb5-81fd-4b29-ba93-48a87209b0ad';
  jsonApi:store:relationaldb Produced +7ms {"id":"2f14ceb5-81fd-4b29-ba93-48a87209b0ad","type":"users","name":"jason","email":"j@gmail.com","roles":null}
  jsonApi:handler:find {"type":"users","data":{"attributes":{"name":"jason","email":"j@gmail.com"},"type":"users"},"id":"2f14ceb5-81fd-4b29-ba93-48a87209b0ad"} +0ms [null,{"id":"2f14ceb5-81fd-4b29-ba93-48a87209b0ad","type":"users","name":"jason","email":"j@gmail.com","roles":null}]
  jsonApi:validation:output {"id":"2f14ceb5-81fd-4b29-ba93-48a87209b0ad","type":"users","name":"jason","email":"j@gmail.com","roles":null} +0ms
  jsonApi:validation:error child "roles" fails because ["roles" must be a string] +2ms {"id":"2f14ceb5-81fd-4b29-ba93-48a87209b0ad","type":"users","name":"jason","email":"j@gmail.com","roles":null}

The `server.metrics.on('data'):

{ route: 'users/:id',
  verb: 'POST',
  httpCode: 201,
  error: null,
  duration: 30 }

The server never responds with the ID according to Ember Inspector (I'm guessing the data key is empty).

Attempts to query the new ID directly fail as well, which was the reason for #36.

Looking through the docs, I think this should return a 403 Forbidden (http://jsonapi.org/format/#crud-creating-responses-403).

To replicate this issue, this is the Ember.js code (roles is completely missing from the create POST):

      var user = this.get('store').createRecord('user', {
        name: 'jason',
        email: 'j@gmail.com'
      });

On the server:

var server = require('jsonapi-server');
...
server.define({
  resource: 'users',
  handlers: handler2,
  attributes: {
    name: server.Joi.string(),
    email: server.Joi.string(),
    roles: server.Joi.string().allow('')
  }
});
bitsoflogic commented 8 years ago

So I decided to dive into the code a bit and it looks like this may actually fall into a decision on your jsonapi-server package instead. I mistakenly assumed the validation was happening on this side.

In lib\routes\helper.js, adding a presence: 'required' param appears to make this work as I expected however at this point I don't know what/if-any adverse effects this may have on the implementation of the JSONAPI 1.0 spec. The change produces a 403 if I submit a POST without the roles param.

Joi.validate(someObject, someDefinition, { abortEarly: false, presence: 'required' }, function (err, sanitisedObject) {

pmcnr-hx commented 8 years ago

Hi @bitsoflogic. Thank you for your report. You are right that this a jsonapi-server issue rather than a relational database store issue. Can you please move this issue there, where we will be able to better address it?