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

Filter/Where as a standalone concept #5957

Open
bajtos opened this issue Jul 17, 2020 · 4 comments
Open

Filter/Where as a standalone concept #5957

bajtos opened this issue Jul 17, 2020 · 4 comments

Comments

@bajtos
Copy link
Member

bajtos commented Jul 17, 2020

Refactor the current implementation of LoopBack's Filter to allow other packages to leverage the filtering implementation and/or support LoopBack's Filter syntax.

  1. The syntax of Filter/Where objects should be well defined and properly documented, to allow 3rd-party implementations to understand all possible values.

  2. There should be TypeScript typings describing Filter, Where, etc., these typings must be accessible independently on @loopback/repository (e.g. via a new package).

  3. The implementation used by our memory connector should be refactored/extracted into a package that can be used independently from juggler (something like https://www.npmjs.com/package/loopback-filters). It's important to update the connector to use that new package, to ensure there is only single reference implementation.

Nice to have:

  • JSON Schema describing Filter syntax
  • Some sort of a test suite to allow 3rd-party implementations to verify conformance
@bajtos bajtos changed the title Filter/Where as a standalone package Filter/Where as a standalone concept Jul 17, 2020
@rem-code-s
Copy link

Really looking forward to this! The only way around the filter and where missing types is by declaring them in the 3rd-party application i suppose?

@achrinza
Copy link
Member

@RAW4RMCS Currently, these can be imported from @loopback/repository. However, that package bundles other things such as the Juggler ORM, repository classes, etc.

See: https://loopback.io/doc/en/lb4/apidocs.repository.filter.html

@rem-code-s
Copy link

rem-code-s commented Jul 20, 2020

@achrinza hmm maybe this isn't the fix for the issue i'm currenly facing if i understand your response correctly,
The issue i'm facing is the following;

I'm using Nswag for generating a frontend client to consume the api endpoints based on the openapi spec,

Example of the generated code base on a controller + model;

/**
     * @param filter (optional) 
     * @return Container model instance
     */
    getContainerById(id: number, filter: any | undefined): Promise<ContainerWithRelations> {
        let url_ = this.baseUrl + "/containers/{id}?";
        if (id === undefined || id === null)
            throw new Error("The parameter 'id' must be defined.");
        url_ = url_.replace("{id}", encodeURIComponent("" + id));
        if (filter === null)
            throw new Error("The parameter 'filter' cannot be null.");
        else if (filter !== undefined)
            url_ += "filter=" + encodeURIComponent("" + filter) + "&";
        url_ = url_.replace(/[?&]$/, "");

        let options_ = <RequestInit>{
            method: "GET",
            headers: {
                "Accept": "application/json"
            }
        };

        return this.transformOptions(options_).then(transformedOptions_ => {
            return this.http.fetch(url_, transformedOptions_);
        }).then((_response: Response) => {
            return this.processGetContainerById(_response);
        });
    }

    protected processGetContainerById(response: Response): Promise<ContainerWithRelations> {
        const status = response.status;
        let _headers: any = {}; if (response.headers && response.headers.forEach) { response.headers.forEach((v: any, k: any) => _headers[k] = v); };
        if (status === 200) {
            return response.text().then((_responseText) => {
            let result200: any = null;
            let resultData200 = _responseText === "" ? null : JSON.parse(_responseText, this.jsonParseReviver);
            result200 = ContainerWithRelations.fromJS(resultData200);
            return result200;
            });
        } else if (status !== 200 && status !== 204) {
            return response.text().then((_responseText) => {
            return throwException("An unexpected server error occurred.", status, _responseText, _headers);
            });
        }
        return Promise.resolve<ContainerWithRelations>(<any>null);
    }

As shown here in the function the filter has a any type (and it's required), but when i check the swaggerUI the required properties are included in the call. I thought this would fix this issue but i guess it does not.

image

Do you recommend a client generator for Loopback where this issue doesn't occur?
Sorry if this comment isn't related to this issue.

@achrinza
Copy link
Member

achrinza commented Jul 23, 2020

@RAW4RMCS Based on the code, the filter is considered optional, hence the any | undefined type.

The filter shown on the Swagger UI is auto-generated by Swagger UI itself.

Also, it seems to be a limitation of nswag to use an any type for the filter, not fully utilizing the OpenAPI 3.0 spec.

A popular OAS3 generator would be https://github.com/OpenAPITools/openapi-generator. However, I've not tried any of the generators myself.

achrinza added a commit to achrinzafork/loopback-next that referenced this issue Aug 24, 2020
see: loopbackio#5957

Signed-off-by: Rifa Achrinza <25147899+achrinza@users.noreply.github.com>
achrinza added a commit to achrinzafork/loopback-next that referenced this issue Aug 24, 2020
see: loopbackio#5957

Signed-off-by: Rifa Achrinza <25147899+achrinza@users.noreply.github.com>
achrinza added a commit to achrinzafork/loopback-next that referenced this issue Aug 24, 2020
see: loopbackio#5957

Signed-off-by: Rifa Achrinza <25147899+achrinza@users.noreply.github.com>
achrinza added a commit to achrinzafork/loopback-next that referenced this issue Aug 24, 2020
see: loopbackio#5957

Signed-off-by: Rifa Achrinza <25147899+achrinza@users.noreply.github.com>
achrinza added a commit to achrinzafork/loopback-next that referenced this issue Aug 29, 2020
see: loopbackio#5957

Signed-off-by: Rifa Achrinza <25147899+achrinza@users.noreply.github.com>
achrinza added a commit to achrinzafork/loopback-next that referenced this issue Aug 29, 2020
see: loopbackio#5957

Signed-off-by: Rifa Achrinza <25147899+achrinza@users.noreply.github.com>
achrinza added a commit that referenced this issue Sep 4, 2020
see: #5957

Signed-off-by: Rifa Achrinza <25147899+achrinza@users.noreply.github.com>
@stale stale bot added the stale label Sep 27, 2021
@achrinza achrinza removed the stale label Sep 27, 2021
@loopbackio loopbackio deleted a comment from stale bot Sep 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants