Project

General

Profile

task #7214

discuss unexpected method behaviors in CdmTransientEntityCacher

Added by Andreas Kohlbecker over 1 year ago. Updated 5 months ago.

Status:
New
Priority:
Highest
Category:
cdmlib
Start date:
01/24/2018
Due date:
% Done:

0%

Severity:
normal
Tags:

Description

CdmTransientEntityCacher

 /**
     * Puts the passed <code>cdmEntity</code> into the cache as long it does not yet exist in the caches.
     * <p>
     * The adjacent <b>ENTITY GRAPH WILL NOT BE LOADED RECURSIVELY</b>
     */
    @Override
    public void put(CdmBase cdmEntity) {

CacheLoader

THIS IS OK!!! (see comment below)

    /**
     * Loads the {@link eu.etaxonomy.cdm.model.common.CdmBase cdmEntity}) in the
     * cache. 
     * <p>
     * <b>WARNING: Recursive updating of the cached entity will not take place 
     * in case there is a cached entity which is the same object as
     * <code>cdmEntity</code>.</b> 
     *
     * For in depth details on the mechanism see
     * {@link #loadRecursive(CdmBase, List, boolean)} and
     * {@link #getCdmBaseTypeFieldValue(CdmBase, CdmBase, String, List, boolean)}
     *
     * @param cdmEntity
     *            the entity to be put into the cache
     * @param recursive
     *            if <code>true</code>, the cache loader will load the whole
     *            entity graph recursively into the cache
     * @param update
     *            all fields of the cached entity will be overwritten by setting
     *            them to the value of the cdm entity being loaded
     */
    public <T extends CdmBase> T load(T cdmEntity, boolean recursive, boolean update) {

Associated revisions

Revision c045b6d6 (diff)
Added by Andreas Kohlbecker over 1 year ago

ref #7214 warning notes in method doc

Revision e5dfaec8 (diff)
Added by Andreas Kohlbecker over 1 year ago

ref #7214 fixing wrong usage of cacher methods

History

#1 Updated by Andreas Müller over 1 year ago

Can you give more details on the issue to discuss?

Who should be Asignee?

#2 Updated by Andreas Kohlbecker over 1 year ago

  • Subject changed from discuss unexpected method behaviors in CdmTransientEntityCacher and CacheLoader to discuss unexpected method behaviors in CdmTransientEntityCacher
  • Description updated (diff)

The potential issue regarding the CacheLoaderload() method mentioned above is correct behavior. I guess I was a bit tired yesterday when I wrote this ticket. The CdmTransientEntityCacher.put() implementation seems strange to me, thought. It only puts the passed entity into the cache skipping the adjacent subgraphs. When the same object is put into the cache using the load() method, the entity is recognized as being already in the cache and will not be updated. Using the put method is therefore quite dangerous since is will break all successive attempts to load its whole entity graph into cache as long as the same object is passed to the load method.

(This issue is assigned to you by default since it is not yet decided who will take responsibility for the cache issues)

#3 Updated by Andreas Kohlbecker over 1 year ago

Soweit ich den Code richtig interpretiere, gibt es ja zwei Möglichkeiten ein Objekt in den Cache zu laden, entweder nur dieses Objekt, dann wird nur load(CdmBase) aufgerufen, indem dann wiederum put aufgerufen wird und auch der SubGraph gar nicht geladen werden soll oder eben mit rekursiven Laden des Subgraphen, da wird das load(CdmBase) ganz am Anfang aufgerufen (und CdmBase mit put in den Cache geschrieben) und danach dann der rekursive Aufruf für den restlichen Objektgraph, diese Objekte sind dann ja noch nicht mit put im Cache gelandet. Da sehe ich nicht, warum das durch das put verhindert werden sollte.
Aber ich habe das noch nicht debugged, sondern nur mal in den Code geschaut.

Viele Grüße,
Katja

#4 Updated by Andreas Kohlbecker over 1 year ago

Oha, das war mir noch garnicht aufgefallen, danke für diesen Hinweis Katja:

diese methode verwendet am ende die load(CdmBase cdmEntity) methode in dieser wird, wie du sagst put() verwendet:

public <T extends CdmBase> T load(T cdmEntity, boolean recursive, boolean update){

...

if(isRecursiveEnabled && recursive) {
          ...
        } else {
            loadedCdmBase = load(cdmEntity);
        }
}

protected CdmBase load(CdmBase cdmEntity) {
        cdmCacher.put((CdmBase)ProxyUtils.deproxy(cdmEntity));
        return cdmCacher.getFromCache(cdmEntity);
}

die implementierung von put() führt aber zu dem von mir beschriebenen Problem.

#5 Updated by Katja Luther over 1 year ago

Andreas Kohlbecker wrote:

Oha, das war mir noch garnicht aufgefallen, danke für diesen Hinweis Katja:

diese methode verwendet am ende die load(CdmBase cdmEntity) methode in dieser wird, wie du sagst put() verwendet:

public <T extends CdmBase> T load(T cdmEntity, boolean recursive, boolean update){

...

if(isRecursiveEnabled && recursive) {
          ...
        } else {
            loadedCdmBase = load(cdmEntity);
        }
}

aber an dieser Stelle soll ja auch nicht rekursiv geladen werden, da es im else Zweig von

if (isRecursiveEnabled && recursive)

im Editor wird load auch nur rekursiv verwendet, wir sollten aber die put Methode entsprechend dokumentieren bzw. noch mal überarbeiten.

#6 Updated by Andreas Müller over 1 year ago

  • Target version changed from Release 4.13 to Release 4.14

#7 Updated by Andreas Müller over 1 year ago

  • Target version changed from Release 4.14 to Release 5.0

#8 Updated by Andreas Müller about 1 year ago

  • Target version changed from Release 5.0 to Release 5.1

#9 Updated by Andreas Müller about 1 year ago

  • Target version changed from Release 5.1 to Release 5.2

#10 Updated by Andreas Müller 11 months ago

  • Target version changed from Release 5.2 to Release 5.3

#11 Updated by Andreas Müller 11 months ago

  • Target version changed from Release 5.3 to Release 5.4

#12 Updated by Andreas Müller 9 months ago

  • Target version changed from Release 5.4 to Release 5.5

#13 Updated by Andreas Müller 5 months ago

  • Target version changed from Release 5.5 to Release 5.6

#14 Updated by Andreas Müller 5 months ago

  • Priority changed from New to Highest
  • Target version changed from Release 5.6 to Reviewed Next Major Release

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 40 MB)