Skip to content

QueryParams does not include ParamOptions as like as the documentation says? #160

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

Closed
ahmedsaedhub opened this issue May 24, 2017 · 13 comments
Assignees
Labels
type: fix Issues describing a broken feature.

Comments

@ahmedsaedhub
Copy link

I can not find ParamOptions while using the newly added feature QueryParams?

@ahmedsaedhub ahmedsaedhub changed the title QueryParams does not include ParamOptions like as like as the documentation says? QueryParams does not include ParamOptions as like as the documentation says? May 24, 2017
@MichalLytek
Copy link
Contributor

Yeah, looks like @pleerock forgot to add ParamOptions param to the @QueryParams() and other decorators like @HeaderParams, etc.:

export function QueryParams(): Function

So now we can't turn on/off validation or set class-transform options.

@pleerock
Copy link
Contributor

Thats a documentation issue. QueryParams do not accept param options because QueryParams is dummy - it simply take request.query and returns it. No any other operation applied to the extract data.

@MichalLytek
Copy link
Contributor

MichalLytek commented May 25, 2017

because QueryParams is dummy - it simply take request.query and returns it. No any other operation applied to the extract data.

@pleerock
It is really bad idea, if user want to validate the query object he can't do it because there is now no param option to turn on validation or pass validation settings. Without #122 we have to make a dirty hack with sending json in query param.
I know that other options like required makes no sense, but class-transformer and class-validator would be very useful on query object.

@pleerock
Copy link
Contributor

Type transformation wont work because we cant read data type of array, so classtransformation and validation wont work until explicit given type. Usually all params in query params are different which means array of different object types. How are we going to specify them to QueryParams() ? Via array of types? This wont look good and seems useless.

@ahmedsaedhub
Copy link
Author

Hello @pleerock , I think it will be nice if we add this feature as @19majkel94 says, it will be nice if we have the ability to enable the validations for query params, example: if i'm sending an email format inside a query string and want to validate it..etc

@MichalLytek
Copy link
Contributor

MichalLytek commented May 25, 2017

because we cant read data type of array

How are we going to specify them to QueryParams() ? Via array of types?

Wait... what? What array? 😨

@Get("/test/:paramOne/:paramTwo")
public async test(@Req() req: any) {
    const query = req.query;
    const params = req.params;

    return {
        query,
        params,
    }
}

GET /test/pleerock/19majkel94?express=better&koa=poor

2017-05-25 21_42_16-users ts - backend - visual studio code

{
  "query": {
    "express": "better",
    "koa": "poor"
  },
  "params": {
    "paramOne": "pleerock",
    "paramTwo": "19majkel94"
  }
}

Both route params and query params are an OBJECT, not an ARRAY. So you can declare a class, decorate it with class-validator decorator and class-transformer will transform plain object to class instance and allow to validate.

@pleerock
Copy link
Contributor

ah right I forgot that its an object 😆 . But what class do you want to define - special for the whole query object?, e.g. @QueryParams() params: UserRouteParams ? If so, then its a good solution and probably we can implement it

@MichalLytek
Copy link
Contributor

special for the whole query object?, e.g. @QueryParams() params: UserRouteParams

Yes, like I already have for body validation. Until #122 it's the only solution for auto validating route params.

@fabiob
Copy link
Contributor

fabiob commented Jul 21, 2017

Will this be reopened? It would be really useful if @QueryParams worked just like @Body.

@MichalLytek
Copy link
Contributor

Actually @QueryParams validation works right now. I even created a test case for this:

describe.only("should validate @QueryParams input", () => {

    class UserQuery {

        @MinLength(5)
        author: string;

        @MinLength(5)
        contributor: string;

        hello() {
            return "Hello world!";
        }
    }

    let requestQuery: UserQuery;
    beforeEach(() => {
        requestQuery = undefined;
    });

    before(() => {
        getMetadataArgsStorage().reset();
     
        @JsonController()
        class UserController {

            @Get("/github")
            getUsers(@QueryParams() query: UserQuery): any {
                requestQuery = query;
                return query.hello();
            }

        }
    });

    const options: RoutingControllersOptions = {
        validation: true
    };

    let expressApp: any, koaApp: any;
    before(done => expressApp = createExpressServer(options).listen(3001, done));
    after(done => expressApp.close(done));
    before(done => koaApp = createKoaServer(options).listen(3002, done));
    after(done => koaApp.close(done));

    assertRequest([3001], "get", "github?author=Umed&contributor=19majkel94&somethingPrivate=blablabla", response => {
        console.log(response.body.errors);
        expect(response).to.have.status(400);
        expect(response.body.errors).to.have.lengthOf(1);
        expect(response.body.errors[0].value).to.eql("Umed");
    });
});

But correct errors are returned in response only in express - in koa I get this in errors property:

TypeError: Cannot read property 'prototype' of undefined
       at E:\#Programowanie\#GitHub\routing-controllers\src\metadata\MetadataStorage.ts:79:36
       at Array.filter (native)
       at MetadataStorage.getTargetValidationMetadatas (E:\#Programowanie\#GitHub\routing-controllers\src\metadata\MetadataStorage.ts:75:61)
       at ValidationExecutor.execute (E:\#Programowanie\#GitHub\routing-controllers\src\validation\ValidationExecutor.ts:44:54)
       at Validator.coreValidate (E:\#Programowanie\#GitHub\routing-controllers\src\validation\Validator.ts:31:18)
       at Validator.<anonymous> (E:\#Programowanie\#GitHub\routing-controllers\src\validation\Validator.ts:73:35)
       at step (E:\#Programowanie\#GitHub\routing-controllers\node_modules\class-validator\validation\Validator.js:32:23)
       at Object.next (E:\#Programowanie\#GitHub\routing-controllers\node_modules\class-validator\validation\Validator.js:13:53)
       at E:\#Programowanie\#GitHub\routing-controllers\node_modules\class-validator\validation\Validator.js:7:71
       at Promise (<anonymous>) }

I will try to fix it and implement the ParamOptions feature 😉

@MichalLytek MichalLytek reopened this Jul 21, 2017
@adnan-kamili
Copy link

Waiting for this feature :)

@MichalLytek
Copy link
Contributor

Closing this via #289. Will be released in 0.8.0-beta.1 - routing-controllers@next, stay tuned! 😉

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: fix Issues describing a broken feature.
Development

No branches or pull requests

5 participants