Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API Explorer ignores filter options where, fields, and order #2208

Closed
7 tasks done
ewrayjohnson opened this issue Jan 3, 2019 · 20 comments
Closed
7 tasks done

API Explorer ignores filter options where, fields, and order #2208

ewrayjohnson opened this issue Jan 3, 2019 · 20 comments
Assignees
Labels
API Explorer bug developer-experience Issues affecting ease of use and overall experience of LB users Docs major OpenAPI
Milestone

Comments

@ewrayjohnson
Copy link

ewrayjohnson commented Jan 3, 2019

I am using the example Todo model with the memory connector. The GET/ operations show the following example filter object, but "where", "fields", and "order "o are stripped before getting to the controller's find method.

{
  "where": {},
  "fields": {},
  "offset": 0,
  "limit": 0,
  "skip": 0,
  "order": [
    "string"
  ]
}

From other posts, this may be a coercion issue, but I would expect this functionality from LB3 to work flawlessly in LB4. I strongly disagree with the announcement that "...LoopBack 4 GA is Now Ready for Production Use!" Also, I cannot find ANY documentation on filters in the LB4 documentation. My test filter is as simple as:

{
  "where": {
        "id" : 1
   } 
}

Acceptance Criteria:

  • - this is a breaking fix, needs semver major release
  • - change existing param.query.object decorator to support json objects in query params.
    including content field in param spec, helps to support both exploded filter conditions like filter[limit] = 0 and json objects as per spike PR feat: spike on API Explorer supporting complex objects in query param #4141
  • - parameter schema cannot have content field along with schema, explode, type fields. swagger validation fails (check with https://editor.swagger.io/) if they appear together.
  • - loopback currently does not support extracting parameter schema from the content field. if we are including schema under content and not schema field, then it means additional changes in tests and source.
  • - add additional unit, acceptance and regression tests
  • - add documentation for filter conditions and above changes
  • - Close related stories
@bajtos
Copy link
Member

bajtos commented Jan 10, 2019

"where", "fields", and "order "o are stripped before getting to the controller's find method.

I assume this is happening when making the request from the API Explorer, is that correct?

I am afraid this is a known bug/limitation of swagger-ui and swagger-js, see the discussion in #1679 and the tracking issue swagger-api/swagger-js#1385.

You can see this yourself in API Explorer UI. When you send the request, the UI shows the full URL, request headers and response headers. There you can see that the request URL created by API Explorer (swagger-js) does not contain the filter properties.

Unfortunately swagger-ui does not warn the user about this problem, which makes the entire experience less than ideal.

Having said that, the actual implementation in LoopBack4 does support where/fields/order, you need to create the request URL manually or use a different OpenAPI client.

I cannot find ANY documentation on filters in the LB4 documentation. My test filter is as simple as:

LoopBack 4 uses the same format as LoopBack 3 does. I agree our documentation should be more explicit about this fact. To keep things simple, maybe it's enough to add a link pointing to existing LB3 docs? Would you like to contribute such change yourself? See https://loopback.io/doc/en/contrib/doc-contrib.html to get started.

@ghsamm
Copy link
Contributor

ghsamm commented Jan 12, 2019

I found this tricky workaround:

loopback-nested

There seems to be no other way to pass nested filters from the Swagger UI.

A more convenient workaround would be to use something something like Postman and pass the filter as a string.

@masalinas
Copy link

masalinas commented Jan 29, 2019

Me and my team we are using this great framework, loopback 2 and 3 from more 3 year and we are testig the new loopback 4 version to upgrade some of our apps.

I installed the last versión of Loopback from CLI @loopback/cli version: 1.5.2. and the problem related with the atributes "where", "fields", and "order not works. Of course if generated the services from swagger generator for angular 6 the problem persist if you try to pass any filters, orders or fields from the services created.

Like a user of loopback for many years and like @ewrayjohnson said, LoopBack 4 GA can't be launch to production!. Where do you resolve this problem related with Open API?

Regards.

@yiiliveext
Copy link

yiiliveext commented Mar 23, 2019

There seems to be no other way to pass nested filters from the Swagger UI.

A more convenient workaround would be to use something something like Postman and pass the filter as a string.


"parameters": [
          {
            "name": "expand",
            "in": "query",
            "description": "User relations",
            "explode": false,
            "schema": {
              "type": "array",
              "items": {
                "type": "string"
              }
            }
          },
          {
            "name": "filter",
            "in": "query",
            "description": "User filter",
            "explode": true,
            "schema": {
              "properties": {
                "filter[field][cond]": {
                  "type": "string"
                }
              },
              "type": "object"
            }
          }
        ],

Filter parameter value in UI:

{
"filter[name][like]": "peter",
"filter[created_at][gt]": 1536843061
}

Generated query:
curl -X GET "http://mysite.com/api/v1/users?filter[name][like]=peter&filter[created_at][gt]=1536843061" -H "accept: application/json"

@sergiomata
Copy link

Hello, any update for this issue?

@bajtos
Copy link
Member

bajtos commented Jun 7, 2019

Not really. Re-posting one of my earlier comments:

I am afraid this is a known bug/limitation of swagger-ui and swagger-js, see the discussion in #1679 and the tracking issue swagger-api/swagger-js#1385.

(...)

Unfortunately swagger-ui does not warn the user about this problem, which makes the entire experience less than ideal.

Having said that, the actual implementation in LoopBack4 does support where/fields/order, you need to create the request URL manually or use a different OpenAPI client.

@sergiomata Would you like to contribute the necessary changes to swagger-js yourself?

@bharathgrk
Copy link

I found this tricky workaround:

loopback-nested

There seems to be no other way to pass nested filters from the Swagger UI.

A more convenient workaround would be to use something something like Postman and pass the filter as a string.

This works for "where" clauses as well.

Screen Shot 2019-06-20 at 19 20 43
Screen Shot 2019-06-20 at 19 23 39

@bajtos
Copy link
Member

bajtos commented Sep 19, 2019

Based on OAI/OpenAPI-Specification#1706 (comment) (cross-posted below), maybe we can switch filter encoding from deepObject style into JSON?

Here is an experiment we can try:

  1. Let's keep @param.query.object with no changes for backwards compatibility.
  2. Introduce a new helper @param.query.jsonObject that will use JSON encoding instead of deepObject style.
  3. Modify the function parseJsonIfNeeded to detect JSON encoding and parse the string value - see packages/rest/src/coercion/coerce-parameter.ts
  4. Update our example Todo controller to use @param.query.jsonObject - see examples/todo/src/controllers/todo.controller.ts
  5. Start the Todo app, open API Explorer and check how is the filter parameter handled.

The comment:

@ewrayjohnson

what if content (each qs param value) be subject to JSON.parse/JSON.stringify subject to URL/URI decoding/encoding as needed with whitespace removal.
For example:

{
  "name": "ivan",
  "birthdate": {
    "gte": "1970-01-01"
  },
  "numbers": [5, 10]
}

can have whitespace removed as

{"name":"ivan","birthdate":{"gte":"1970-01-01"},"numbers":[5,10]}

encoded as

?param1=%7B%22name%22%3A%22ivan%22%2C%22birthdate%22%3A%7B%22gte%22%3A%221970-01-01%22%7D%2C%22numbers%22%3A%5B5%2C10%5D%7D

I believe this particular use case is already covered by the content keyword instead of schema/style/explode.

parameters:
  - in: query
    name: param1
    content:
      application/json:
        schema:
          type: object
          properties:
            name:
              type: string
              example: ivan
          ...

@vikramkh
Copy link

vikramkh commented Nov 8, 2019

In the url I passed in ?where={"id":"1"} and it worked in the controller if I put this.userRepository.find({where}). However, if I put ?where={"is":"1"} (an invalid where), then it returns all the users and ignores the where clause. Is this expected behaviour/a limitation?

@bajtos bajtos changed the title Filter where, fields, and order are not supported API Explorer ignores filter options where, fields, and order Nov 15, 2019
@bajtos
Copy link
Member

bajtos commented Nov 15, 2019

In the url I passed in ?where={"id":"1"} and it worked in the controller if I put this.userRepository.find({where}). However, if I put ?where={"is":"1"} (an invalid where), then it returns all the users and ignores the where clause. Is this expected behaviour/a limitation?

This is a known bug in our SQL connector(s), see loopbackio/loopback-connector#125 and loopbackio/loopback-connector#132

@dhmlau
Copy link
Member

dhmlau commented Nov 19, 2019

@deepakrkris, do you think you can add acceptance criteria for this task? Thinking whether we can include this in Dec. Thanks.

@bajtos
Copy link
Member

bajtos commented Nov 21, 2019

do you think you can add acceptance criteria for this task? Thinking whether we can include this in Dec. Thanks.

IMO, we need to finish the spike story #3770 first. Until then, we don't know how the solution is going to look like.

@dhmlau dhmlau added this to the Jan 2020 milestone Jan 7, 2020
deepakrkris added a commit that referenced this issue Jan 23, 2020
@dhmlau dhmlau added the Slipped label Feb 3, 2020
@dhmlau dhmlau modified the milestones: Jan 2020, Feb 2020 Feb 3, 2020
@deepakrkris
Copy link
Contributor

all tasks completed, acceptance criteria satisfied.

@ibmjm
Copy link
Contributor

ibmjm commented Mar 24, 2020

all tasks completed, acceptance criteria satisfied.

@deepakrkris Just wanted to thank you for this work. It's a welcome relief to be able to write my filters normally, instead of weird workarounds like "include][][relation": "someRelation" and so on.

Cheers!

@ewrayjohnson
Copy link
Author

@deepakrkris: I just looked back to when I first reported this. Its been a long journey. Thanks for persevering and resolving this impediment!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Explorer bug developer-experience Issues affecting ease of use and overall experience of LB users Docs major OpenAPI
Projects
None yet
Development

No branches or pull requests