Routes with parameters are not working in some cases

[ambienthack]

Description / Steps to reproduce / Feature proposal

Create two routes like this

  @patch('/users/{userId}/meals/{id}')
  async updateById(
    @param.path.string('userId') userId: string,
    @param.path.string('id') id: string
  ): Promise {
    //anyting
  }

  @patch('/users/{id}')
  async updateById(
    @param.path.string('id') id: string
  ): Promise {
    //anything
  }

Make a request PATCH /users/5c1d78842566764efc8b9605

Current Behavior

{
  "error": {
    "statusCode": 404,
    "name": "NotFoundError",
    "message": "Endpoint \"PATCH /users/5c1d78842566764efc8b9605\" not found."
  }
}

If the error doesn't arise, try changing parameter names in path patterns, like this:
@patch('/users/{id}/meals/{mealId}')
@patch('/users/{userId}')

Expected Behavior

Controller method should be invoked

I believe there is a bug here

    for (const child of children) {
        const result = search(keys, index + 1, params, child.node);
        if (result) {
            Object.assign(params, child.params);
            return result;
        }
    }

Should be if (result && isNodeWithValue(result.node)) {.

When multiple children are found, if the first child checked is @patch('/users/{userId}/meals/{id}'), search() method assumes route is found & doesn't check other routes.

[bajtos]

@ambienthack thank you for reporting the issue, your analysis looks reasonable to me. Would you like to contribute the fix yourself? See https://loopback.io/doc/en/contrib/code-contrib.html to get started.

@raymondfeng you are the author of the trie router, could you PTAL?

[Joel-Raju]

I seem to have run into the same issue. These are my controller actions

Action 1

@get('/restaurants/{id}')
async getRestaurantById(
@param.path.number('id') id: number,
  ): Promise<Restaurant> {
    console.log(id);
    return await this.restaurantRepository.findById(id);
  }

Action 2

@get('/restaurants/', {
async find(
@param.query.object('filter', getFilterSchemaFor(Restaurant))
filter?: Filter,
): Promise<Restaurant[]> {
  return await this.restaurantRepository.find(filter);
}

Current Behavior
Action 1 and Action 2 throws a 404

{
  "error": {
    "statusCode": 404,
    "name": "NotFoundError",
    "message": "Endpoint \"GET /restaurants/1\" not found."
  }
}

Expected Behavior

Controller method should be invoked

Other Observations

Even after removing Action 2 and all other actions in the controller that matches the same route Action 1 is still not being invoked.

[Joel-Raju]

would really appreciate if this could be resolved soon.

[raymondfeng]

@Joel-Raju Do you have both methods in the same controller class?

[raymondfeng]

BTW, do you know if return await this.restaurantRepository.findById(id); is invoked and returns an instance?

[raymondfeng]

Can you remove trailing / and see?

@get('/restaurants', {
async find(
@param.query.object('filter', getFilterSchemaFor(Restaurant))
filter?: Filter,
): Promise<Restaurant[]> {
  return await this.restaurantRepository.find(filter);
}

[raymondfeng]

@Joel-Raju Please verify #2355. Let us know whether the newly added test cover your case or not.

[Joel-Raju]

@raymondfeng thanks for the quick follow-up

@Joel-Raju Do you have both methods in the same controller class?

Yes.

BTW, do you know if return await this.restaurantRepository.findById(id); is invoked and returns an instance?

It doesn't seem to hit route @get('/restaurants/{id})', I tried logging before querying the repository

Can you remove trailing / and see?

@get('/restaurants', {
async find(
@param.query.object('filter', getFilterSchemaFor(Restaurant))
filter?: Filter,
): Promise<Restaurant[]> {
  return await this.restaurantRepository.find(filter);
}

When removing the trailing space, the @get('/restaurants)' is being invoked.

@Joel-Raju Please verify #2355. Let us know whether the newly added test cover your case or not.

this covers my use case.

[raymondfeng]

When I remove the trailing space, the @get('/restaurants)' is being invoked. @get('/restaurants/{id})' doesn't work

That's weird. What order do you define two routes in the controller?

Which request URL do you use to invoke the API? With trailing slash or not?

[Joel-Raju]

I first have @get(/restaurants) and then @get(/restaurants/{id}). Also tried reversing the order, still doesn't work.

Even tried just with @get(/restaurants/{id}) commenting out all other methods in the controller but doesn't work.

These are the request URLs

this works
curl -X GET "http://localhost:3001/restaurants" -H "accept: application/json"

this doesnt work
curl -X GET "http://localhost:3001/restaurants/1" -H "accept: application/json"

[raymondfeng]

@Joel-Raju Can you create a simple app that can reproduce your problem and share it with us on github?
You can folk loopback-next and add your app to sandbox directory.

[Joel-Raju]

@raymondfeng forked and added a sample project that illustrates the issue

https://github.com/Joel-Raju/loopback-next/tree/master/sandbox/test-app

this route works
http://localhost:3000/restaurants

this doesn't work
http://localhost:3000/restaurants/1

[Joel-Raju]

Guess I didn't mention that I have a hasMany relationship. It worked before adding the relationship.

[raymondfeng]

@Joel-Raju Thanks for the sample app. I was able to reproduce the problem and fix it in #2355.

[raymondfeng]

Guess I didn't mention that I have a hasMany relationship. It worked before adding the relationship.

That's the root cause as you added /restaurants/{restaurantId}/orders, which overlaps with /restaurants/{id} using different var names.

[Joel-Raju]

oh okay. This will get resolved with #2355 ?

[raymondfeng]

Yes. Before it's landed, you can probably work around the issue by changing the path /restaurants/{restaurantId}/orders to be /restaurants/{id}/orders.

[Joel-Raju]

thanks 👍