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

[9.x] Add Route::singleton() method #44872

Merged
merged 10 commits into from
Nov 28, 2022
Merged

[9.x] Add Route::singleton() method #44872

merged 10 commits into from
Nov 28, 2022

Conversation

jessarcher
Copy link
Member

@jessarcher jessarcher commented Nov 8, 2022

This PR introduces a helper to register routes for "singleton resources" - resources that are either "one of one" or "zero or one".

Example:

Route::singleton('profile', ProfileController::class);
Verb URI Action Route Name
GET /profile show profile.show
GET /profile/edit edit profile.edit
PUT/PATCH /profile update profile.update
DELETE /profile destroy profile.destroy

Many of the options that apply to resource controllers can also be used here, such as only and except:

Route::singleton('profile', ProfileController::class)->except('destroy');

When the resource can be zero or one, a PUT request can be used for both creating and updating the resource because we always know the resource's location. The update name that is traditionally attached to the PUT doesn't make this entirely clear, so we could potentially name it something like upsert instead. Personally, I prefer keeping the existing name rather than introducing a new action name.

Singleton resources can also be nested within a standard resource for cases where a resource is singular in the scope of the parent resource:

Route::singleton('videos.thumbnail', VideoThumbnailController::class);
Verb URI Action Route Name
GET /videos/{video}/thumbnail show videos.thumbnail.show
GET /videos/{video}/thumbnail/edit edit videos.thumbnail.edit
PUT/PATCH /videos/{video}/thumbnail update videos.thumbnail.update
DELETE /videos/{video}/thumbnail destroy videos.thumbnail.destroy

Route model binding would work for the Video model in the above example, but there is no model binding for singleton resources because the URL does not provide enough information to resolve it. In many cases, there may not even be a dedicated model for the singleton resource. In the above example, the thumbnail could be a separate model, but it could equally just be an attribute on the Video model that warrants its own routes.

For parity with the resource methods, there is also an apiSingleton method that excludes the edit action, and there are singletons and apiSingletons methods for bulk registration.

* @param array $options
* @return void
*/
public function __construct(ResourceRegistrar $registrar, $name, $controller, array $options)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use PHP 8 constructor property here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class matches the PendingResourceRegistration class that was created prior to PHP 8.

@lukebouch
Copy link

This looks super useful, @jessarcher!

@bjhijmans
Copy link

In your examples you have routes to destroy a singleton. Shouldn't you also be able to create and store one if it doesn't exist?

@jessarcher
Copy link
Member Author

In your examples you have routes to destroy a singleton. Shouldn't you also be able to create and store one if it doesn't exist?

The intention would be that you'd always make a PUT request, regardless of whether the resource exists or not.

@jessarcher jessarcher marked this pull request as ready for review November 9, 2022 00:22
@lukebouch
Copy link

That makes sense, @jessarcher.

@bjarn
Copy link

bjarn commented Nov 9, 2022

What about Route::crud(...). Might be a better descriptor for this case as there can be many videos or many profiles, but you basically want to generate these crud routes.

Anyway, nice addition!

@jasonmccreary
Copy link
Contributor

Nice. I've definitely created these types of resources by hand before.

My only change would be the name. If this is always a resource, which it seems to be, I'd suggest singletonResource (or soleResource) to align with apiResource. I appreciate it's a bit more to type, but singleton alone feels like the container.

@ajcastro
Copy link

ajcastro commented Nov 9, 2022

I think soleResource might be better as per @jasonmccreary

@niladam
Copy link

niladam commented Nov 9, 2022

IMHO for some reason I think it'd be more clear if it would be named as singleResource; nonetheless great job and I think it would make a great addition!

Thank you for your work!

@jessarcher
Copy link
Member Author

I don't mind adding "resource" to the method name to give more context. I like the definition of "singleton" but I'm not married to it either.

An alternative could be something like this:

Route::resource('foo', FooController::class)->singleton(); // or sole, single, etc...

I haven't put much thought into it, but I think the underlying code might be a little more complex if we share the same pending object for both. There are also methods on the pending resource object that would be ineffective for a singleton, like the scoping stuff.

@niladam
Copy link

niladam commented Nov 9, 2022

For some reason your last proposal while it does look better, i think it would be confusing when you'd read the routes.

Having a different name at the beginning is the best route (ha!) to take.

I think it provides you with a quick and direct overview instead of looking at the end to see the difference.

I guess it's just a matter of naming, but I really think that singleton isn't the best option. Obviously, IMHO:)

@huangdijia
Copy link
Contributor

I don't like the name singleton, because its meaning is not very intuitive. Perhaps it is more intuitive to call it restful.

@reshadman
Copy link

reshadman commented Nov 9, 2022

This is a good addition.
Trying to see everything as resources as much as possible helps, and most projects have multiple explicit definitions for singular resources which this addition solves that and makes the route definition file more declarative.

singleton is not a good name:

  • New developers that are trying to get onboarded will be confused to distinguish singleton objects and this route-specific convention
  • Their online searches get affected by two different concepts
  • Singleton is a reserved keyword in object-oriented programming space

Better options would be Route::singular('profile', ProfileController::class); or Route::resource('profile', ProfileController::class)->singular();

@lukebouch
Copy link

Or even singularResource?

@timacdonald
Copy link
Member

timacdonald commented Nov 9, 2022

I like both "singleton" and "singular" as it is already an establish term for this in other frameworks. Introducing another name for it seems like fragmenting again. I feel this naming is already descriptive of the feature.

"Singleton" also pops up in some JSON:API implementations (although, I don't believe this concept exists in JSON:API itself).

What's more, if I see the term / function "singleResources" I'm still going to go read the docs to find out what it is, because the term itself does not tell me what it is exactly or how to use it, and if I'm source diving, the underlying code will be the same.

@Haythamasalama
Copy link

I like both "singleton" and "singular" as it is already an establish term for this in other frameworks. Introducing another name for it seems like fragmenting again when I feel like this naming is already descriptive of the feature.

What's more, if I see the term / function "singleResources" I'm still going to go read the docs to find out what it is, because the term itself does not tell me what it is exactly or how to use it, and if I'm source diving, the underlying code will be the same.

It makes sense 💯

@driesvints
Copy link
Member

My vote goes to Route::singular('profile', ProfileController::class); 👍

Same as in Ruby: https://guides.rubyonrails.org/routing.html#singular-resources

@jessarcher
Copy link
Member Author

jessarcher commented Nov 11, 2022

The problem I have with "singular" is that we've already used that word in Illuminate\Routing\ResourceRegistrar (the class where this new functionality lives) for the process of generating the singular version of the route name as the default route param. E.g. Route::resource('users', ...) => /users/{user}:

public static function singularParameters($singular = true)
{
static::$singularParameters = (bool) $singular;
}

public function getResourceWildcard($value)
{
if (isset($this->parameters[$value])) {
$value = $this->parameters[$value];
} elseif (isset(static::$parameterMap[$value])) {
$value = static::$parameterMap[$value];
} elseif ($this->parameters === 'singular' || static::$singularParameters) {
$value = Str::singular($value);
}
return str_replace('-', '_', $value);
}

If we introduce a method on this class called singular and a new property called $singularDefaults (to match the $resourceDefaults property) then I think it gets pretty confusing when combined with the above.


I think I can summarize the two main problems people are having with the "singleton" name suggestion:

  1. People aren't used to seeing the word "singleton" refer to anything other than the singleton object pattern. I get this, but it is a standard dictionary word that can mean different things in different contexts. It also seems to be a common way of referring to this RESTful pattern.
  2. Words like "singleton" and "singular" on their own don't provide enough context when called via the Route facade - "What does a singleton/singular route mean?". As much as I'd like to avoid a multi-word method name, I think something like Route::soloResource() might be the only way to make that clearer.

@menthol
Copy link

menthol commented Nov 11, 2022

Why try to be the least verbose possible?
My vote goes to : Route::singletonResource()

Singular is already used.
Route::singular('user', UserController::class)->singular() is not clear at all.
but: Route::singletonResource('user', UserController::class)->singular() is clearer.

@driesvints
Copy link
Member

@jessarcher I don't think we should avoid adopting a good name because there's another similar concept in the routing component right now. I don't think these two clash enough to get confusing for users.

@taylorotwell
Copy link
Member

Added ability to do Route::singleton('profile', ProfileController::class)->creatable(); to indicate you want create and store routes. Note that Rails and Phoenix register these by default. Of course, in the case of apiSingleton(...)->creatable() only the store method would be added to the route registration - not create.

@jessarcher
Copy link
Member Author

jessarcher commented Nov 14, 2022

I worry the word "creatable" implies that POST requests are the preferred (and perhaps only) way that a singleton resource should be created.

Personally, I disagree with the other frameworks promoting the POST method on a specific resource's URI. It's like making a POST request to an individual resource in a collection (e.g. POST /photos/1) which Laravel doesn't support with Route::resource().

I think the HTTP spec is clear that PUT requests are specifically intended for scenarios like this. I do wish the framework world didn't have such a strong association between "PUT" and "update".

The create/edit routes are less clear-cut. With a resource collection, I typically put the create form somewhere on the index route (often in a modal) rather than on a dedicated create route. So I think with a "zero or one" or "nullable" singleton, I would probably display the create form on the show route when the record doesn't exist. That would save me from having to redirect from /thing to /thing/create when the singleton doesn't exist, and from /thing/create to /thing/edit when it does already exist.

Having said all that, I'm not completely opposed to it being opt-in. I just wonder whether there is an alternative name that doesn't imply that PUT is only for updating.

Also, the irony of me objecting to naming on this PR is not lost on me 😅

@dunhamjared
Copy link
Contributor

@jessarcher

I just wonder whether there is an alternative name that doesn't imply that PUT is only for updating.

How about set as an action name for create or update?

Sounds idempotent to me, for example:

"Set profile picture"
"Set settings"
"Set title"

@jimmyyem
Copy link

jimmyyem commented Jan 5, 2023

I wonder when this feature will be released ? Now I am using laravel 9.33 there is no Route::singleton() function , so maybe I should wait for a few days?

@niladam
Copy link

niladam commented Jan 5, 2023

@jimmyyem from what i can see this is available since v9.42.0 (released on 29 Nov);

Therefore you can just go ahead and update to > 9.42.0 :)

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

Successfully merging this pull request may close these issues.