fortunejs / fortune

Non-native graph database abstraction layer for Node.js and web browsers.
http://fortune.js.org
MIT License
1.45k stars 135 forks source link

Weird ids breaks route #130

Closed jlcarvalho closed 9 years ago

jlcarvalho commented 9 years ago

I got this generated id: /f9XL5u+VDM426bToRc1. So, when i tried to access GET /people//f9XL5u+VDM426bToRc1 i got:

{
    "errors": [
        {
            "title": "NotFoundError",
            "detail": "The field \"f9XL5u+VDM426bToRc1\" is not a link on the type \"people\"."
        }
    ]
}
jlcarvalho commented 9 years ago

Again LvHdwDFA/d/1uSscKNmwand again O/hYIlwLjUChLAm73dhG.

I'm using Fortune.request to create these documents.

const store = fortune.create({
  adapter: {
    type: mongodbAdapter,
    options: {
      url: config.database
    }
  },
  serializers: [ {
    type: MySerializer
  } ]
})

store.request({
  method: methods.create,
  type: 'people',
  payload: [{
    name: record.name,
    email: record.email
  }]
}).then(function (res) {
  console.dir(res.payload[0].id)
})
jlcarvalho commented 9 years ago

In fact, this is a bug in the mongodb adapter. Here is the issue (https://github.com/fortunejs/fortune-mongodb/issues/14) in the right repo.

gr0uch commented 9 years ago

The value of the ID should not matter actually! The problem is URI encoding. I see that you are using a custom MySerializer, which may not be encoding URIs properly.

Here is testing for weird IDs: https://github.com/fortunejs/fortune-json-api/blob/master/test/index.js#L232-L239 https://github.com/fortunejs/fortune-micro-api/blob/master/test/index.js#L56-L69

Is your client configured to encode URIs? if you're using one of the hypermedia serializers your client should only follow the URI given from the server which is already encoded.

If you don't want to bother with URI encoding altogether, you can configure the adapter you're using to not generate base64 IDs: https://github.com/fortunejs/fortune-mongodb#options

{
  options: {
    generateId () { return randomBytes(12).toString('hex') }
  }
}
jlcarvalho commented 9 years ago

My adapter extends the JsonApiSerializer which already handles this.

class MySerializer extends JsonApiSerializer {
    processRequest (context, request, response) {
        let ctx = super.processRequest(...arguments)

        if(request.url === '/users' && context.request.method === methods.create) return ctx

        let token = ctx.request.meta['x-access-token'];
        if (token) {
            let user = (deasync(jwt.verify)(token, app.get('superSecret')))

            if(!user){
                throw new errors.UnauthorizedError(`Falha ao validar o token.`)
            } else {
                ctx.request.session = user
                return ctx
            }
        } else {
            throw new errors.UnauthorizedError(`Token não informado.`)
        }
    }
}
gr0uch commented 9 years ago

Are you sure your request URLs are encoded? I don't think this is a server-side problem.

jlcarvalho commented 9 years ago

You're right. The client is not configured to encode URI. I thought that the IDs are encoded on the server response.

gr0uch commented 9 years ago

The IDs are plaintext, but the URIs should be encoded, correct me if I'm wrong but I'll close this issue now. You should generally rely on the URIs provided by the server, the client does not need to build URIs.

jlcarvalho commented 9 years ago

You're right. Thank you again, and sorry for the dumb questions.