bleupen / halacious

a better HAL processor for Hapi
MIT License
108 stars 22 forks source link

Wrong self link in POST response #101

Open bfoerster opened 7 years ago

bfoerster commented 7 years ago

I'm currently playing a little with this plugin and found an issue with POST requests.

If I POST a resource to my server the response has HAL format. But unfortunately the self link does not point to the created resource. Am I missing something?

My route implementation looks like this:

{
    method: 'POST',
    path: '/pets',

    config: {
        handler: (request, reply) => {
            const body = request.payload;
            body.id = randomId();
            allPets.push(body);
            return reply(body);
        },
        description: 'POST a new Pet',
        notes: 'Creates a new pet',
        tags: ['api'],
        plugins: {
            hal: {
                ignore: 'id',
            }
        }
    }
}

If I make a POST request curl -X POST --header 'Accept: application/hal+json' --header 'Content-Type: application/vnd.api+json' -d '{"name":"Bodo","category":"Dog"}' http://localhost:8000/pets I get the following response:

{
  "_links": {
    "self": {
      "href": "/pets"
    }
  },
  "name": "Bodo",
  "category": "Dog"
}

I think the self link should contain the id of the resource:

{
  "_links": {
    "self": {
      "href": "/pets/pet_id"
    }
  },
  "name": "Bodo",
  "category": "Dog"
}

My workaround is to add the id to the link in the prepare function:

{
    method: 'POST',
    path: '/pets',

    config: {
        handler: (request, reply) => {
            const body = request.payload;
            body.id = randomId();
            allPets.push(body);
            return reply(body);
        },
        description: 'POST a new Pet',
        notes: 'Creates a new pet',
        tags: ['api'],
        plugins: {
            hal: {
                ignore: 'id',
                prepare: (rep, done) => {
                    rep._links.self.href = '/pets/' + rep.entity.id;
                    done();
                }
            }
        }
    }
}

You can see my example "application" here https://github.com/bfoerster/nodejs-hapi-hal/blob/master/src/routes/pets.js

alexb-uk commented 6 years ago

I've also come across this issue and couldn't see where I might have done something wrong.

However due to using swaggerize-hapi I've had to use the "toHal() on a domain entity" approach.

This is a workaround for that:

User.prototype.toHal = function(rep, next) {
    if (rep.request.method === 'post') {
    rep._links.self.href += `/${this.id}`;
    }
    next();
};

Hope this helps someone.

I'll try to find a proper fix but I'm not too familiar with the internals of this library.

alexb-uk commented 6 years ago

This might be a potential fix but would need someone more knowledgeable than I to check it :) For instance this fix feels like it assumes a fair bit about the API and entity:

In lib/representation.js

RepresentationFactory.prototype.create = function(entity, self, root) {
    entity = entity || {};
    self = self || this._request && this._request.path;

    if (this._request.method === 'post' && 'id' in entity === true) {
      self += `/${entity.id}`;
    }

    self = this._halacious.link(self);
    return new Representation(this, self, entity, root);
};

Thoughts?

bleupen commented 6 years ago

halacioius will use the location header to rewrite the self rel. Here's what I do in nearly all of my POST routes.

assuming I have a GET route to look up a pet like this:

{
   method: 'get',
   path: '/pets/{id}',
   config: {
      id: 'pet.lookup',
      handler: function(req, reply) {
           // look up the pet
      }
   }
}

my POST route to create one would look like:

{
   method: 'post',
   path: '/pets',
   config: {
      handler: function(req, reply) {
         req.server.createPet(req.payload)
            .then(pet => reply(pet).created(req.server.plugins.halacious.route('pet.lookup', { id: pet.id })))
            .catch(reply);
      }
   }
}
alexb-uk commented 6 years ago

@bleupen Thanks for the suggestion, unfortunately for me swaggerize-hapi routes don't appear to have id's.

I'll post a message on their list to see if this is something difficult to add in the future.

Cheers

bleupen commented 6 years ago

you dont have to use that technique. you can also set the location manually:

req.server.createPet(req.payload)
   .then(pet => reply(pet).created(`/pets/${pet.id}`))
   .catch(reply);