loopbackio / loopback-next

LoopBack makes it easy to build modern API applications that require complex integrations.
https://loopback.io
Other
4.95k stars 1.07k forks source link

Loopback4 modifies filter parameter in repository 'findById' operation #5814

Closed Smixi closed 3 years ago

Smixi commented 4 years ago

Note: This issue is also published elsewhere

Steps to reproduce

Passing an object as a filter in the findById method modifies includes and nested includes objects. I'm using the postgresql connector.

Created a basic application with lb4 cli.

package.json: "@loopback/core": "^2.7.1", "@loopback/repository": "^2.6.0" "loopback-connector-postgresql": "^3.9.1"

Here is the interesting function :

  console.log(includedRelations.include[0].scope)
  const newLoanWithRelation = await this.loanRepository.findById(newLoan.id, includedRelations);
  console.log(includedRelations.include[0].scope)

Here is the filter:

const includedRelations = {
  include: [
    {
      relation: "licenseeloans",
      scope: {
        include: [
          {
            relation: "member"
          }
        ]
      }
    },
    {
      relation: "weaponloans"
    },
    {
      relation: "initiator"
    },
    {
      relation: "returnInitiator"
    }
  ]
}

Current Behavior

First console.log :

 { include: [ { relation: 'member' } ] }

Second console.log:

{ include: [ { relation: 'member' } ],
  where: { id: '42177991-308b-488c-b9d5-f50496d08281' } }

After one findById operation again :

{ include: [ { relation: 'member' } ],
  where: { and: [ [Object], [Object] ] } }

Expected Behavior

The filter is updated, why ? Reusing the same filter could then create bug. In my controller logic, I need to retrieve the relations with the object a lot of time, so I'm using a constant filter to do that. As it get populated, I got after 32 requests to that controller a "QUERY_OBJECT_TOO_DEEP"" error.

Link to reproduction sandbox

https://gitlab.com/Smixi/csgr-back

Additional information

linux x64 10.19.0

├── @loopback/authentication@4.2.6 ├── @loopback/authorization@0.5.11 ├── @loopback/boot@2.3.2 ├── @loopback/context@3.8.2 ├── @loopback/core@2.7.1 ├── @loopback/openapi-v3@3.4.2 ├── @loopback/repository@2.6.0 ├── @loopback/rest@5.1.0 ├── @loopback/rest-explorer@2.2.3 ├── @loopback/service-proxy@2.3.1 ├── loopback-connector-postgresql@3.9.1

Related Issues

If it's just my understanding of the repository methods, then please forgive me.

davidpestana commented 4 years ago

Relevant about this issue...

OpenAPI 3.0 Specification currently defines the deepObject behavior only for simple objects (with primitive properties) such as

{
  "id": 5,
  "name": "Bob"
}

but not for arrays and not for nested objects.

https://stackoverflow.com/questions/52892768/openapi-query-string-parameter-with-list-of-objects

Smixi commented 4 years ago

Relevant about this issue...

OpenAPI 3.0 Specification currently defines the deepObject behavior only for simple objects (with primitive properties) such as

{
  "id": 5,
  "name": "Bob"
}

but not for arrays and not for nested objects.

https://stackoverflow.com/questions/52892768/openapi-query-string-parameter-with-list-of-objects

Thanks for your answer. But I don't think this is relevant for the issue, as it doesn't happens because of the controller correctly handling the data, but because of the include filters in lb4 repository (as I use only the id param).

0x0aNL commented 3 years ago

This also results in only receiving included hasManyThrough entities for the first entity, if a scope is included. E.g.:

[
  {
    id: 1,
    name: 'Doctor Mario',
    patients: [{name: 'Luigi'}, {name: 'Peach'}],
  },
  {
    id: 2,
    name: 'Doctor Link',
    patients: [],
  },
];

This happens because:

This is currently a problem in production for us - we hope to see this rolled out soon.

agnes512 commented 3 years ago

@0x0aNL @sumnerwarren Could you check it with the latest version? I just tested it and I think it works for nested inclusion as well for HasManyThrough relation. I will push a PR (https://github.com/strongloop/loopback-next/pull/7101) to add related tests too.

0x0aNL commented 3 years ago

@0x0aNL @sumnerwarren Could you check it with the latest version? I just tested it and I think it works for nested inclusion as well for HasManyThrough relation. I will push a PR to add related tests too.

Yes, it was indeed fixed by #6955 in the latest version! Thanks for your hard work 👍 !