TransactionManager and EntityManager Changes(Added unitname for Entit…

[selvarajkumarasamy]

…yManager and Identifier for TransactionManager)

[selvarajkumarasamy]

@jricher. Please review my changes as soon as possible,we are eagerly waiting to use the code.

Thanks
Selvaraj K

[jricher]

The abstract DefaultEntityManager class seems completely unnecessary.

[selvarajkumarasamy]

How to keep the EntityManager Unique and Global?. If we have it in every class that should be duplicate right?

[selvarajkumarasamy]

Are you ok with remaining changes?

[praseodym]

Spring will ensure that it's a singleton.

[selvarajkumarasamy]

Yes thats fine.. If we have it in every class then what is the use of code reusability?,I don't have any issue to keeping EntityManager in every class.Is it ok for you to keep in evey class?

[praseodym]

I personally think the abstract class approach is neater than repeating the PersistenceContext unit name in ever single class.

[jricher]

The problem I have with it is that it's limiting in the class structure due to Java's single inheritance structure. It's true that we don't need it today, but if we were to switch to using Spring's auto-generated DAO repository classes, we would have to back this out anyway.

[selvarajkumarasamy]

Ok fine. i will revert the EntityManager Changes.

[selvarajkumarasamy]

Need to create one more pull request right? because i am in hurry to use this code

[jricher]

You can push changes to this pull request. You can continue to use your fork until it's been incorporated into the baseline project and released.

[selvarajkumarasamy]

@Jricher.Thanks for your update,i will do the changes and push now.

[selvarajkumarasamy]

Changes are done please verify as soon as possible and Make it avail this changes in Master

[selvarajkumarasamy]

Hi Jricher, does the code moved to master environment?

[jricher]

Not yet, this pull request will be closed when it does.

[ghost]

Hello Justin,

Thanks for accepting this proposed pull request.

We have the code base and able to build and move further for our release. But, our reviewers may stop the pull request for the release, and this is our initial version and we would like to push clean code base with Mitreid's stable version OpenID Connect implementation.

We couldnt see any milestones for this, so Could you please let us know when this pull request will be merged? (approximate time line should be good, so that we can inform the team accordingly)..

Regards
Pounraj Manikandan

[jricher]

This pull request has not been fully reviewed or tested yet and we don't have a specific timeline for doing so. You'll get notified whenever this artifact is updated.

[jricher]

After further review, I'm afraid we can't take this pull request as-is into the code base as there are a number of side effects to the changes:

• entity managers are now marked as "public" instead of "private" on repositories
• entire repositories are marked as "transactional" instead of just the potentially destructive operations
• entity manager variable needlessly renamed throughout entire project from "em" to "manager"

We'll consider reimplimenting the proposed changes in a less invasive manner.

[selvarajkumarasamy]

Hello Jricher,
Thanks for your update.
1.I agree first point and changed the entity manager public to private.
2.I don't agree second point,we have used both repositories level and operation level transaction in some classes,do we get any benefits of this, please suggest.
3.I have some concern in third point in which we follow differnt property mapping for entity manger in differnt classes, please suggest your thought on this.

If i resolve the issue, is it ok to consider the PR for now and we can consider other as enhancements?.
I can work on your thoughts.

Thanks
Selvaraj.K

[selvarajkumarasamy]

Hi Jricher,
We have implemented all changes,what you have mentioned in previous post.We are ok with the testing. Shall i push the code to same PR?
Thanks
Selvaraj.K

[jricher]

Go ahead and push the changes and we'll take a look. Thank you!

[selvarajkumarasamy]

Hello Jricher,
Thank you for your update,We have pushed the code to same PR with you mentioned the changes.
Can you please check and accept PR if its ok,We are eagerly waiting to use the code.

Thanks
Selvaraj.K

[selvarajkumarasamy]

Hello Jricher,
Could you please let us know when this pull request will be merged?,Please verify the code changes and update me.
Thanks
Selvaraj.K

[jricher]

You'll receive notifications when comments are added and when the pull request has been closed. There is no timeline for review, implied or otherwise.

[jricher]

Re-implemented in a clean patch with no side effects. Let us know if this does what you need it to.