Make getFilterJsonSchemaFor() model-aware

[achrinza]

Currently, getFilterJsonSchemaFor() isn't model-aware.

This means that the openapi.json order filter parameter will be generic:

In the case above, it's the /todos parameter from the Todo example.

This would lead to issues where the suggested order filter isn't a valid query:

(Notice the "order": [ "string" ])

Steps to reproduce

A quick way to reproduce is use the Todo example:

lb4 example todo

This also applies to any LoopBack 4 application that utilises @param.filter(ModelHere).

Expected Behavior

The order filter should be model-aware and not generic.

Link to reproduction sandbox

lb4 example todo

Additional information

$ node -e 'console.log(process.platform, process.arch, process.versions.node)' && \
>   npm ls --prod --depth 0 | grep loopback
win32 x64 12.16.1
@loopback/[email protected] C:\Users\rifaa\Documents\loopback4-example-todo
+-- @loopback/[email protected]
+-- @loopback/[email protected]
+-- UNMET PEER DEPENDENCY @loopback/[email protected]
+-- @loopback/[email protected]
+-- UNMET PEER DEPENDENCY @loopback/[email protected]
+-- @loopback/[email protected]
+-- @loopback/[email protected]
+-- @loopback/[email protected]
npm+-- [email protected]
 ERR! peer dep missing: @loopback/core@^1.12.0, required by @loopback/[email protected]
npm ERR! peer dep missing: @loopback/repository@^1.16.0, required by @loopback/[email protected]
npm ERR! peer dep missing: @loopback/core@^1.12.0, required by @loopback/[email protected]
npm ERR! peer dep missing: @loopback/repository@^1.16.0, required by @loopback/[email protected]
npm ERR! peer dep missing: @loopback/core@^1.12.0, required by @loopback/[email protected]
npm ERR! peer dep missing: @loopback/repository@^1.16.0, required by @loopback/[email protected]
npm ERR! peer dep missing: @loopback/core@^1.12.0, required by @loopback/[email protected]
npm ERR! peer dep missing: @loopback/core@^1.12.0, required by @loopback/[email protected]
npm ERR! peer dep missing: @loopback/core@^1.12.0, required by @loopback/[email protected]

Related Issues

N/A

See Reporting Issues for more tips on writing good issues

[emonddr]

@achrinza , apologies, but I don't follow. The Todo examples don't involve any Order object that I am aware of. To help investigate the issue, please provide a minimal app demonstrating the problem. Thanks.

[achrinza]

@emonddr, apologies if I wasn't clear - what I meant was the order filter that is passed to repository functions.

This is added to the OpenAPI spec's expected request parameter by @param.filter() from @loopback/rest.

This eventually calls getFilterJsonSchemaFor() which isn't model-aware, thereby leading to the API Explorer generating an unusable default filter parameter.

~~I'll see if I can create a minimal app later today.~~

[agnes512]

I was checking filters these days. order filter works well with API Explorer when I tested it. Both work:

{
  "order": [
    "number ASC"
  ]
}
or
{
  "order": [
    "number ASC", "name DESC"
  ]
}

The generated default parameter makes sense to me. Could you explain more about the issue?

[achrinza]

@agnes512 Currently, the default parameter generated by the API Explorer is:

{
  "order": [
    "string"
  ]
}

While it does makes sense, it can't be used as-is as string will need to be replaced with either ASC, DESC or propertyName ASC, etc.

Since the API docs for getFilterJsonSchema() suggests that this was a future improvement that was considered before, it may be a good opportunity to revisit it and see if we can improve the developers' experience with using API Explorer and potentially other OpenAPI Spec consumers/generators.

This improvement is especially useful since the default parameter generated by the API Explorer is rather cryptic for developers who are not familiar with the valid order filter structures.

Since we're already reviewing the order filter, we may want to make use of OpenAPI oneOf to suggest "order": "ASC" as the other valid order filter (IIRC, it's a valid filter)

[agnes512]

Agree, "order ASC" is not intuitive enough. Maybe we can have a layer to convert "order": "ASC" to "order ASC", thoughts? @raymondfeng

[bajtos]

A possible quick fix is to add an empty array as an example value for order. We can change the following block:

https://github.com/strongloop/loopback-next/blob/9478d319a472e1dca4122a29dbb414ce1525bdb0/packages/repository-json-schema/src/filter-json-schema.ts#L87-L92

as follows:

    order: {
      type: 'array',
      items: {
        type: 'string',
      },
      examples: [[]],
    },

That gives me the following default value in REST API Explorer:

{
  "offset": 0,
  "limit": 100,
  "skip": 0,
  "order": [],
  "where": {
    "additionalProp1": {}
  },
  "fields": {
    "id": true,
    "title": true,
    "desc": true,
    "isComplete": true,
    "remindAtAddress": true,
    "remindAtGeo": true,
    "tag": true
  }
}

While not ideal from the point of user/developer experience, it's at least a valid filter.

[bajtos]

Having said that, I am not a big fan of that large default filter value, and personally would prefer a much smaller one.

When I modify https://github.com/strongloop/loopback-next/blob/9478d319a472e1dca4122a29dbb414ce1525bdb0/packages/repository-json-schema/src/filter-json-schema.ts#L107-L113

as follows

  const schema: JsonSchema = {
    ...(options.setTitle !== false && {
      title: `${modelCtor.modelName}.Filter`,
    }),
    properties,
    additionalProperties: false,
    examples: [{limit: 100}],
  };

then the REST API Explorer offers me the following nice default:

{
  "limit": 100
}

[bajtos]

My comments above are showing quick fixes to improve the DX inside the REST API Explorer.

However, I agree with @achrinza that we should emit model-aware definition of the order schema. We can use getFieldsJsonSchemaFor for inspiration on how to get a list of all model properties:

https://github.com/strongloop/loopback-next/blob/9478d319a472e1dca4122a29dbb414ce1525bdb0/packages/repository-json-schema/src/filter-json-schema.ts#L185-L190