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

route model binding (Variable name cannot be longer than 32 characters in route pattern) #275

Open
matthijs opened this issue Apr 18, 2024 · 9 comments
Assignees

Comments

@matthijs
Copy link

Hi,

Due to the naming of one of our resources the variable name became too long. (more then 32 characters)

Simplest solution was to short the variable name by using the following:

$server->resource('some-very-long-name-activities', '\\' . JsonApiController::class)
    ->parameter('short_name')
    ->readOnly();

Now when executing a request like:

GET /api/v1/some-very-long-name-activities/<id>

Now I get a 404 Not found. The record that I am trying to fetch exists.

Usual without laravel-json-api I fix this by using:

Route::model('short_name', SomeModel::class);

But due to the fact that laravel-json-api uses its own substitute bindings this does not seem to work.

The question now is: What am I missing?

@ben221199
Copy link

Can you show the php artisan route:list?

@matthijs
Copy link
Author

Sure, here it is (only the routes related to this problem):

GET|HEAD        api/v1/external-service-provider-activities ....................................................... api.v1.external-service-provider-activities.index › LaravelJsonApi\Laravel › JsonApiController@index
GET|HEAD        api/v1/external-service-provider-activities/{esp_activity} .......................................... api.v1.external-service-provider-activities.show › LaravelJsonApi\Laravel › JsonApiController@show

The index is action is working correctly.

@ben221199
Copy link

I think the '\\' . is not neccessary.

Do you also have the schema?

@matthijs
Copy link
Author

Ok, nevermind. I found the problem.

In our schema we had a double ID::make() definition like:

    /**
     * Get the resource fields.
     */
    public function fields(): array
    {
        return [
            ID::make()->uuid(),

            Str::make('name')->sortable(),
            Str::make('description')->sortable(),

            ID::make('chain')->sortable(); // causing the trouble...

            DateTime::make('created_at')->sortable()->readOnly(),
            DateTime::make('updated_at')->sortable()->readOnly(),
        ];
    }

The fact that ID::make('chain')->sortable() had no ->uuid() did change the regex for the route pattern to an integer regex. And that caused the 404 not found exception.

Thanks for trying to help out and sorry for the trouble.

Maybe check if there are multiple ID::make() definitions returned and throw an exception in that case? That could help catching this kind of bug earlier...

@ben221199
Copy link

ben221199 commented Apr 18, 2024

Yeah, that makes sense. I had the feeling it had something to do with the ID anyway.

@ben221199
Copy link

@lindyhopchris Is this an option in some way?

Maybe check if there are multiple ID::make() definitions returned and throw an exception in that case? That could help catching this kind of bug earlier...

@lindyhopchris
Copy link
Contributor

@lindyhopchris Is this an option in some way?

Maybe check if there are multiple ID::make() definitions returned and throw an exception in that case? That could help catching this kind of bug earlier...

So "checking something" has cost, that you incur every single time the schema is created in a request. Just to catch a mistake a developer has made...

I'll take a look but will only do something if it doesn't add in any extra processing.

@lindyhopchris lindyhopchris self-assigned this Apr 19, 2024
@ben221199
Copy link

Maybe you could do something with debug mode? Performance is less important when debugging locally why something doesn't work.

@matthijs
Copy link
Author

So "checking something" has cost, that you incur every single time the schema is created in a request. Just to catch a mistake a developer has made...

I'll take a look but will only do something if it doesn't add in any extra processing.

I completely agree! For debugging purposes it would be handy if this could be catched but if that has a performance hit in production then please leave it out. Maybe add an extra note to the documentation about this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ready
Development

No branches or pull requests

3 participants