[RFC] Splitting ZfcUser

adamlundrigan commented 9 years ago

The Pre(r)amble

I've used ZfcUser in a significant number of projects over the past two years, and while it works great for that core use case of sticking it into ZendSkeletonApplication to get a quick user login solution up and running, I find that when you start to stray into console or API it fights more than it helps.

I'm also a strong believer that we should be building simple, uni-purpose modules. These simple building blocks can be composed into complex systems while complex modules are harder to use for simpler user cases. Each module should implement a single minimum viable use case, then combine those modules into a nice multi-purpose package (composer meta-package?) for those who prefer to consume it that way.

The Proposal

Split ZfcUser into at least two parts:

Core: entity, service, mapper, forms, hydrator, authentication
MVC: controller, route definitions, view and controller helpers

I haven't given much thought to the naming at this point, but "MVC module" could retain the name ZfcUser for BC purposes and have a dependency on "Core module", whatever you'd like to call that.

The Rationale

Same as my rationale for #55 (Refactor of authentication adapter):

In a recent project I aimed to build a ZfcUser-based app to be used in both console and API (Apigility) contexts, and the authentication layer in it's current form is not well suited for either.

For instance, with an Apigility integration I have to insert an event listener to the config merger to strip out the routes registered by default in ZfcUser as they aren't applicable in that scenario. It's not a complex task, but more code is more code...and I hate writing more code because then I have to maintain it.

Thoughts?

Danielss89 commented 9 years ago

This has been discussed before, and of top of my head i like the idea. I need to do a little more thinking though.

DannyvdSluijs commented 9 years ago

I completely agree with Adam Lundrigan. See issue #541 that I reported.

A core would allow for easier extension for more extended use. On a project that we are doing there is a functional requirement to support login throttling and locking to protect against brute force attacks.

Danielss89 commented 9 years ago

@DannyvdSluijs You can do that with 1.x too. Just listen to the events :)

adamlundrigan commented 9 years ago

An alternative to the original proposal would be to leave everything in a single repository but place the configuration of the MVC entities in a dist file that can be copied to config/autoload if you want to enable the frontend.

claytondaley commented 9 years ago

Architecturally, the idea of separation is great. Practically speaking, you need to maintain a skeleton implementation. If ZfcUser wasn't a fully-functional skeleton, I'd be off using (and contributing to) someone else's system.

To avoid messing up all your Google love, I'd even suggest using ZfcUser as the skeleton and pulling the single purpose components out into modules like ZfcAuth and ZfcAuthMvc (which the refactored ZfcUser can depend upon).

adamlundrigan commented 9 years ago

@claytondaley we're advocating the same thing. "ZfcUser" would still be the complete package, and if that's how you want to use it nothing will change there, except maybe having to add multiple modules to your application config -- but that depends on how we choose to implement the split.

claytondaley commented 9 years ago

Obviously, this works out great in a lot of places. Modules like Goalio's Forgot Password are already working for folks (like me). It's also.easy to extend a Form with additional fields using a Delegator.

Is there an accepted pattern for storage, with an emphasis on cases where the data would (if they weren't plugins) be put in the same table. As a concrete example, we're removing status from the user table. I think we should remove name as well so users aren't forced into a specific representation. Even if you disagree, assume we did for the sake of my next question.

If we decided to create a status plugin and a name plugin to restore these features, what's the storage model? In Doctrine, at least, I couldn't find anything that works like Form's add() to extend an Entity.

DannyvdSluijs commented 9 years ago

@claytondaley If you are meaning to separate users from identities that would be a correct path to go down. The current implementation is not to easy to fit into an application where users can have multiple identities to authenticate an identity can be any of username/password combi, Facebook, google etc. I'm just not sure if this would be part of the split up, of this would be part of a bigger change/rewrite.

claytondaley commented 9 years ago

@DannyvdSluijs That may address my question by basically saying it's invalid. Let me try to rephrase what I was really asking:

Today, we have User entity with email, username, password, full_name, status, etc. If we split full_name and status into two different plugins, where is the data stored? Some options:

"They should be put back into the user table". However, I don't see anything in Doctrine that would make it possible for a plugin to add fields (status and name) to the User Doctrine entity.
Put them in a new entity (implied by your answer). To avoid the confusion of renaming the current user table to identity, let's instead call the new table profile. It seems to me this only moves the problem. Now you have a Profile entity that needs to dynamically add new columns with each plugin.
The developer must create a custom User entity. Technically, this solves the problem, but it kinda breaks the idea that these are simple plugins.
Don't support this sort of plugin. Build in a references to a profile to support this feature, but assume each developer will generate a custom one. End up with a dozen Profile plugins (or forks of a single plugin) each with is own particular set of columns.
Separate tables for each plugin. This is ugly and it still seems like the User entity needs to be updated with a OneToOne reference.
Create a key-value store (crude but possible in MySQL) that holds custom fields ('user_id', 'field', 'value') and override the magic methods (i.e. __get() and __set()) to mask the storage structure.

I had my Python hat on one day and was thinking through the last. I'm not sure if it's proper PHP and tried to ask the question without explaining to avoid biasing readers if there was a better (or at least simpler or standard) way.

adamlundrigan commented 9 years ago

@DannyvdSluijs this has been my approach of choice lately. ZfcUser handles only the authentication part and ignore it's other fields. The application's domain model has an internal User entity that's linked to the ZfcUser record by unique key (usually username or email) with event listeners keeping the necessary fields in sync between the two. The "user" has two entities: one for the application-level concern of authentication/authorization and one for the domain model representation of user (profile). In this setup ZfcUser becomes an implementation detail instead of having the ZfcUser entity intertwined with your application domain model.

This comes down to a documentation issue: the current practice is telling users they should extend the build-in ZfcUser entity class to customize that data it stores about the user. Perhaps we should provide a working example and explanation of what I've outlined above instead?

internalsystemerror commented 8 years ago

@claytondaley I'm able to add new fields to a doctrine entity by simply extending it and telling the application to use the child class as the entity.

Another approach should you decide to go down the separate identity/profile route, could be to use Doctrine Embeddables so that the profile module can embed an identity entity? That would allow the core identity module to be used independently of the profile one?

ZF-Commons / ZfcUser