feathersjs / feathers

The API and real-time application framework
https://feathersjs.com
MIT License
15.07k stars 752 forks source link

Query is always empty object within data resolver functions #2995

Closed tobiasheldring closed 1 year ago

tobiasheldring commented 1 year ago

Steps to reproduce

Call create, update or patch on a service with something set in the query, eg.

// comments.test.ts
await app.service("comments").create(
   {
       text: "This is a comment",
    },
    {
      user,
      query: { test: "Hello" },
    }
)

Try to access the query in a data resolver

// comments.hooks.ts
    create: [
      (context) => console.log(context.params.query), // Logs { test: "Hello" }
      schemaHooks.resolveData(commentDataResolver),
      ...
// comments.schema.ts
export const commentDataResolver = resolve<
  CommentData,
  HookContext<CommentsService>
>({
  text: async (value, _data, context) => {
    console.log(context.params.query); // Logs {}
    return value
  },
});

Expected behavior

Query object is available also in data resolvers in create, path or update hooks

Actual behavior

Query object is empty

System configuration

5.0.0-pre.35

daffl commented 1 year ago

This is likely a query validation problem (I ran into something similar recently) because of the removeAdditional setting. #3000 reverts the change in a generated application since the proper solution is to create our query validators with additionalProperties: false so that we get an error when unexpected parameters show up instead of just silently removing them. In your existing app, remove the

removeAdditional: true

Line from your validators file and explicitly add the properties you expect in a query to the commentsQuerySchema.

tobiasheldring commented 1 year ago

@daffl Yeah that would have similar effect but does not apply here since I wasn't even using the validators when testing this. Also as you can see in my code example the hook just before the data resolver has access to the query while the data resolver does not. Even hooks after the resolver has access to the query. So seems like it could be some issue in resolveData

(context) => console.log(context.params.query), // query: { test: "Hello" }
schemaHooks.resolveData(commentDataResolver), // query: {}
(context) => console.log(context.params.query), // query: { test: "Hello" }
daffl commented 1 year ago

Sorry, you are correct. I had to dig into it a bit more and realised that this is happening in https://github.com/feathersjs/feathers/blob/dove/packages/schema/src/hooks/resolve.ts#L10. The reason was that not resetting the query would make it impossible to pass context.params (to use e.g. the. authenticated user or a database session) to any nested call since they would then all fail because the query doesn't match. I'm not sure that was necessarily a great idea but concerned not that changing it now would cause regressions since it is the expected behaviour.

This might require further discussion but you can still get the original unchanged context in a resolver from the status object like this:

export const messageDataResolver = resolve<Message, HookContext>({
  userId: async (value, message, context, status) => {
    console.log(status.originalContext?.params?.query)
    return value
  },
})
tobiasheldring commented 1 year ago

Ah ok I see. Hadn't realised the existence of status.originalContext, that's great then there is a workaround to access the original query when needed which works for me 👍

Feel free to close this issue if you're not planning any changes to the current behaviour

daffl commented 1 year ago

I went ahead and changed this to how it is supposed to be in #3048. This may be a breaking change but it's probably better to change it now than be stuck with it for a while.