Project

General

Profile

bug #9683

Auto-initialize root bean

Added by Andreas Müller 4 months ago. Updated 3 months ago.

Status:
Closed
Priority:
Highest
Category:
cdmlib
Target version:
Start date:
06/28/2021
Due date:
% Done:

100%

Severity:
normal
Found in Version:

Description

Currently only related objects are auto-initialized by the AbstractBean.beanAutoInitializers . The root bean itself is not initialized this way.
This leads to a situation where loading e.g. a field unit without any property path will throw an exception when fieldUnit.getTitleCache() is called outside the initializing transaction.


Related issues

Related to Edit - task #9678: Remove Derived- and FieldUnitFacadeCacheStrategies Closed 06/25/2021
Related to Edit - bug #7874: TeamOrPersonBase entity can become unusable due to replacement of the title caches on using getters Closed 10/26/2018

Associated revisions

Revision 5348613b (diff)
Added by Andreas Müller 4 months ago

fix #9683 autoinitialize root bean

Revision bcbe5633 (diff)
Added by Andreas Müller 4 months ago

ref #9683 add loadProxy method to services and adapt test to work with current auto initialize settings

Revision c1e0cb15 (diff)
Added by Andreas Müller 4 months ago

ref #9683. ref #9664 fix tests in AgentServiceImplTest

Revision 61c33dfd (diff)
Added by Andreas Müller 4 months ago

ref #9683 adapt method name and javadoc

History

#1 Updated by Andreas Müller 4 months ago

  • Status changed from New to Resolved
  • % Done changed from 0 to 50

#2 Updated by Andreas Müller 4 months ago

  • Related to task #9678: Remove Derived- and FieldUnitFacadeCacheStrategies added

#3 Updated by Andreas Müller 4 months ago

  • Assignee changed from Andreas Müller to Andreas Kohlbecker

#4 Updated by Andreas Müller 4 months ago

  • Related to bug #7874: TeamOrPersonBase entity can become unusable due to replacement of the title caches on using getters added

#5 Updated by Andreas Kohlbecker 4 months ago

  • Status changed from Resolved to Feedback
  • Assignee changed from Andreas Kohlbecker to Andreas Müller

I think the new method, which by the way is only used for test classes, is irritating:

public T loadProxy(int id);

The method name suggests that a proxy method is returned always. This is not the case as explained in the java doc of org.hibernate.Session.load(Class<T> theClass, Serializable id) : "This method might return a proxied instance that is initialized on-demand, when a non-identifier method is accessed."

This additional method, which is similar to other find*(..) methods makes the Cdm*Dao-API unclear as there are already many similar methods.

Can't you use getSession().load() in the test class directly or do you expect that the loadProxy() will be used in non test classes frequently? In the latter case I suggest renaming it to just

public T load(int id);

#6 Updated by Andreas Müller 4 months ago

  • Assignee changed from Andreas Müller to Andreas Kohlbecker

Andreas Kohlbecker wrote:

I think the new method, which by the way is only used for test classes, is irritating:

public T loadProxy(int id);

The method name suggests that a proxy method is returned always. This is not the case as explained in the java doc of org.hibernate.Session.load(Class<T> theClass, Serializable id) : "This method might return a proxied instance that is initialized on-demand, when a non-identifier method is accessed."

This additional method, which is similar to other find*(..) methods makes the Cdm*Dao-API unclear as there are already many similar methods.

Can't you use getSession().load() in the test class directly or do you expect that the loadProxy() will be used in non test classes frequently? In the latter cased I suggest renaming it it to just

public T load(int id);

Yes, there is a need for this method if you want to attache an object to another one without initializing the object and without invoking cascade during save afterwards. This is for performance reasons and it is e.g. needed for imports.

We may discuss the label for the method. I first thought about loadWithoutInitializing but it is a bit long but maybe semantically better.

Purely "load(int)" has many disadvantages as all service.load(...) methods do explicitly initialize the return object(s) so it is very unexpected that only load(int) does not initialize (while eg. load(uuid) or load(int, null) do initialize.

Also the service.loadProxy(int) will always return a proxy except for the situation where it is used within another transaction. This is because it will only return a non-proxy if the object is already in the session which can only be the case if used within another transaction.

#7 Updated by Andreas Müller 4 months ago

My comment was about the method in service. Maybe in dao it is slightly different as it is always used within a session. As this is not API we could more easily rename it to loadWithoutInitializing as long method names are maybe not so relevant here.

#8 Updated by Andreas Kohlbecker 4 months ago

  • Status changed from Feedback to Resolved
  • Assignee changed from Andreas Kohlbecker to Andreas Müller

Ok, thank you for the explanation. I would prefer having the same method names in the DAP and service layer. We can keep loadProxy(int) if the java doc of the methods are comprehensive enough to fully understand the implications of these methods. You could use the text of your comment above for extending the java doc.

#9 Updated by Andreas Müller 4 months ago

  • Status changed from Resolved to Feedback
  • Assignee changed from Andreas Müller to Andreas Kohlbecker
  • % Done changed from 50 to 90

I finally decided to rename the methods (in service and persistence) to loadWithoutInitializing as this is semantically more correct. Also I slightly adapted the javadoc.

So I guess we can close the ticket or are there any other open issues?

#10 Updated by Andreas Kohlbecker 3 months ago

  • Status changed from Feedback to Closed
  • Assignee changed from Andreas Kohlbecker to Andreas Müller
  • % Done changed from 90 to 100

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 40 MB)