bug: improve path variable detection logic in config generator.

[laststylebender14]

Present Behaviour

Currently, the path variable detection logic is simplistic. It checks if the URL has a number between forward slashes, which works correctly in the following scenarios:

For URL /api/v1/users/12 and /api/v1/users/12/comments is correctly converted to

@http(path: "/api/v1/users/{{.args.p1}}")
@http(path: "/api/v1/users/{{.args.p1}}/comments")

However, the logic fails in the following scenarios where the path variable is a string or contains non-numeric/mixed characters:

For URL /v1/albums/4a and URL /v1/albums/wpa it is incorrectly converted to

album(p1: Int!): Album @http(path: "/v1/albums/{{.args.p1}}a")
album(p1: Int!): Album @http(path: "/v1/albums/wpa")

Expected Behaviour

The path variable detection logic should be enhanced to detect path variables in all scenarios, including numbers, strings, or mixed characters, and generate a valid @http directive:

For URL /v1/albums/4aand /v1/albums/wpa, it should be converted to

album(p1: String!): Album @http(path: "/v1/albums/{{.args.p1}}").
album(p1: String!): Album @http(path: "/v1/albums/{{.args.p1}}").

Technical Requirements:

• update the path variable detection logic to handle cases like above.
• update the test cases if necessary.
• add integration test that cover's this case.

[laststylebender14]

here the logic for the path variable detection is written.

https://github.com/tailcallhq/tailcall/blob/1f74665c7e00b7b9f39aa450c73dd324cbb0f787/src/core/generator/json/http_directive_generator.rs#L61-L80

[bnchi]

are you working on this ?

If not I can try to spend some time to fix it

[laststylebender14]

@bnchi No, i'm not working on this, please go ahead and start working on it. if you have any doubt feel free to ping me.

[bnchi]

@laststylebender14

For this case :

album(p1: Int!): Album @http(path: "/v1/albums/wpa")

How can we determine if wpa is an identifier should we expect a best practice endpoint structure from the user in order to determine what goes as a variable ?

[bnchi]

Maybe a better use case would be to let the user explicitly mark this as an identifier with a curly bracket or something I could be wrong here correct me if there's a case where this might be an issue

[amitksingh1490]

I feel using something like below will make more sense

curl:
  src: "https://example.com/api/v1/order/$var1/"
  arguments: 
    var1:  "be2574f3-8d25-4d95-9ac8-adf4b745381f"
  fieldName: "orderDetails"
  headers: *headers

[bnchi]

@amitksingh1490 I agree letting the user just be explicit on what goes as an argument is alot more correct, currently the feature breaks in many cases specially with complicated endpoint structure. Not necessarily the best option when the user have a big configuration file to write however it doesn't break in cases when the parameter type is just as complicated as be2574f3-8d25-4d95-9ac8-adf4b745381f.

I also don't think a best effort strategy(like what's implemented in my inital PR is viable) guess work is hard and not always correct the user might end up thinking the feature is broken if something doesn't come out right.

[github-actions[bot]]

Action required: Issue inactive for 30 days. Status update or closure in 7 days.

[github-actions[bot]]

Issue closed after 7 days of inactivity.