search

thephpleague / tactician-doctrine

Tactician plugins for the Doctrine ORM, primarily transactions
MIT License
57 stars 13 forks source link

Private attribute in TransactionMiddleware #22

Closed rubenrubiob closed 5 years ago

rubenrubiob commented 5 years ago

Hi there!

I am using this package and recently and recently I have come across a problematic situation. The thing is that I am uploading files and saving its information in the database. But I want to upload them in a transactional way, i.e., avoid uploading the file if there is an error in the database, or avoid saving the record in the database if there is an error moving the file.

For that purpose I have created a Middleware that extends from this packages' TransactionMiddleware, where I start the transaction, commit it or rollback it for both the database and the filesystem.

I am using the protected method rollbackTransaction of TransactionMiddleware in order to rollback the transaction using the code that is already written. But I had to redeclare the entityManager attribute in my class, as it is private.

That is my question: why is the entityManager attribute private whilst the rollbackTransaction method is protected?

I understand that if the rollbackTransaction is protected and the TransactionMiddleware is not declared as final, the class can be extended. But then maybe the entityManager attribute should be declared as protected? Or the opposite, if the class should not be extended, both must be private.

That is my question, just an clarification :)

Thank you for this package!

Regards!

rosstuck commented 5 years ago

Honestly, both ought to be private, it's probably just an oversight or leftover legacy. This class was originally written in a time before the final keyword, otherwise it would have it.

Rather than extending the middleware, I would recommend just duplicating the code OR splitting your file rollback into a separate middleware entirely.

Hope that helps clarify the situation! :)

rubenrubiob commented 5 years ago

Yes, it helps a lot! I would duplicate the code.

I can not move my file rollback to another middleware. That is how I had it first, but then, if the file or database commit failed, there would be inconsistencies as only one rollback would be performed.

Thank you!