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

explore using listeners for password encoding and canonicalization #48

Closed
lsmith77 opened this issue Feb 14, 2011 · 6 comments · Fixed by #142
Closed

explore using listeners for password encoding and canonicalization #48

lsmith77 opened this issue Feb 14, 2011 · 6 comments · Fixed by #142

Comments

@lsmith77
Copy link
Member

No description provided.

@ornicar
Copy link

ornicar commented Feb 14, 2011

Using doctrine listeners made me hate them.
They are not explicit, they do stuff without you knowing it. They are hard to debug. When a listener has a reference to the document manager, things become very dangerous.

I really prefer using 'doer' objects that wrap doctrine actions and explicitly add logic; the way the user manager actually works is fine IMO.

In my current project I removed all doctrine listeners. To replace listeners on entity removal for example, I use a remover service.

Instead of

$objectManager->remove($stuff);
$objectManager->flush();
// unpredictible and mysterous listeners logic (maybe, if they were registered)

I prefer

$stuffRemover->remove($stuff); // stuff removal logic is contained into this method
$objectManager->flush();

I think services that act on entities should not have access to the manager, and I prefer flushing manually after beeing sure everything went fine.

I know the problem is that removing $stuff without using the dedicated service may be disastrous. I have nothing to argue about that, just use the freaking service.

My 0.02 bucks

@schmittjoh
Copy link

I think we already decided against this, not sure why we should revisit this decision, has anything changed since then?

@stof
Copy link
Member

stof commented Feb 14, 2011

The issue is that UserManager#updateUser currently do a hidden flush which can break things if you add a new related entity in the user and call updateUser before persisting the other entity. Thus it makes it hard to do a single flush.
And I think many newcomers to the bundle will just persist their entity as their other ones and get an exception because the canonical fields are empty.
It is possible to call updatePassword and updateCanonicalFields manually as these methods are public but this is not documented and requires two calls.

@lsmith77
Copy link
Member Author

i just didnt remember the reasons ..

@stof
Copy link
Member

stof commented Apr 11, 2011

A solution would be to provide a Doctrine listener calling UserManager::updateUser and register it according to a configuration option. This would keep the best of both world:

  • automatically updating the fields for newcomers using Doctrine events so they don't have WTF when the use the EntityManager directly
  • allowing advanced user to disable it if they prefer calling it explicitly instead of relying on a listener.

What do you think about this way to go ?

@lsmith77
Copy link
Member Author

maybe because i havent suffered from Doctrine listeners yet, i am open to this suggestion :)

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

Successfully merging a pull request may close this issue.

4 participants