loopbackio / loopback-connector-rest

Connect Loopback to a REST API
http://loopback.io/doc/en/lb2/REST-connector.html
Other
75 stars 81 forks source link

CRUD connected models do not pass filter parameter properly for all method #136

Closed mgabeler-lee-6rs closed 4 years ago

mgabeler-lee-6rs commented 5 years ago

Description/Steps to reproduce

  1. Setup a repository binding a model to the rest connector with the crud flag
  2. Try to call repo.find with a filter parameter
  3. Observe that the filter parameter is passed to the remote server incorrectly and your filter is thus ignored

Link to reproduction sandbox

https://github.com/mgabeler-lee-6rs/lb4-rest-filter

Repro steps for the sandbox app:

The app:

Expected result

GET request is made to /Model?filter... instead of /Model?where... and thus only the requested objects are returned

Actual result

All objects in the repository are returned regardless of filter. Filter options like include and fields are ignored just like where is.

mgabeler-lee-6rs commented 5 years ago

This is closely related to https://github.com/strongloop/loopback-next/issues/2788

mgabeler-lee-6rs commented 5 years ago

findById and findOne are broken by the same issue. It's possible to monkey-patch around the find and findOne bugs by overriding the methods and inserting the extra layer of filter: {...}, but the way findById is written, that trick doesn't work and monkey-patching around its bug requires a more complex solution.

mgabeler-lee-6rs commented 5 years ago

Actually, monkey-patching findOne is hard too

mgabeler-lee-6rs commented 5 years ago

Also finding that 404 handling for even a basic findById (which does issue the correct REST request) is broken -- it returns the HTTP exception, fails to translate it into an EntityNotFoundError

mukunzidd commented 5 years ago

@mgabeler-lee-6rs for me this error has stopped the project build process and only when I remove the block which contains the .find call... only then the project can run. Even, I was trying this in the Todo and the TodoList examples in the official doc. Even in any new project that I start 0and generate a CRUD controller, as well as your demo for this issue. I am currently using @loopback/core: "^1.7.1", I wanted to try using Loopback in my upcoming project but I can't get past this... Any thoughts?

mgabeler-lee-6rs commented 5 years ago

Here's a gist with a slightly simplified version of my monkey patching: https://gist.github.com/mgabeler-lee-6rs/6f30a3e117182060cc88a8d20e6ed914

The idea is that you extend this base class instead of the normal one that it extends when creating a repository that uses a rest datasource under the hood. And only when it's using rest under the hood -- this will almost certainly break things if you use any other datasource.

I would caution if you adopt something like this in your own code base to write very careful acceptance tests, and not to assume any methods not "fixed" are working unless you have such tests that prove it. Speaking from experience, it is very easy to make mistakes in the acceptance tests and think you have patched the methods correctly when you have not.

bajtos commented 5 years ago

@raymondfeng PTAL

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

mgabeler-lee-6rs commented 5 years ago

still broken as described

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

mgabeler-lee-6rs commented 4 years ago

image

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 4 years ago

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.