[CELUM-51] Make parent folder lazy loaded when instantiating a CelumAsset Created: 25/Apr/23  Updated: 13/Nov/23  Resolved: 13/Nov/23

Status: Closed
Project: Celum DAM Connector
Component/s: None
Affects Version/s: 1.0.4
Fix Version/s: 1.0.5

Type: Improvement Priority: Neutral
Reporter: Raphael Falvo Assignee: Raphael Falvo
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: PNG File image-2023-09-29-13-22-01-777.png    
Issue Links:
Problem/Incident
Relates
relates to CELUM-56 getWidth throws NullPointerException ... Closed
Template:
Acceptance criteria:
Empty
Date of First Response:
Team: Services
Work Started:

 Description   

Message from Stefan on slack union-investment-collab channel, march 24th 2023:

How can we make use of the publicURL without a celum connection? Currently, as soon as you access a CelumAsset a connection is established and without it, no access to the PublicURL via the asset interface.
I got this code reference of us, where we call getAsset(), which is doing the celum call

 

public Optional<String> createAssetLink(final String itemKey) {
final Predicate<String> isNotAnUrl = string -> !UrlValidator.getInstance().isValid(string);


return Optional.ofNullable(itemKey)
.filter(StringUtils::isNotBlank)
// The isNotAnUrl Predicate is an ugly hack for the hotfix. It's needed because Magnolia's
// ItemKey::isValid says, that an URL is a valid ItemKey, which is actually wrong.
// A support ticket has been created at Magnolia: SUPPORT-16075
.filter(isNotAnUrl)
.filter(ItemKey::isValid)
.map(this.damTemplatingFunctions::getAsset)
.map(Asset::getLink);
}

 

Main problem is when we do get asset we always check to set the parent folder, we need to find a clean way for this implementation

Let's try making it lazy loaded, only call Celum to fetch the parent when parent is really needed.



 Comments   
Comment by Raphael Falvo [ 23/Aug/23 ]

Fix got provided in 1.0.5-beta2 version to be tested by the customer.

Comment by Raphael Falvo [ 28/Sep/23 ]

CELUM-55 relates to this ticket (CELUM-51), as 1.0.5 beta2 version was dedicated to it.

Null pointer exception reported in CELUM-55 has been fixed.
It is available in celum-dam-connector 1.0.5-beta3 version.

Comment by Stefan Huettenrauch [ 29/Sep/23 ]

Hi Raphael,
We have tested the beta version and have the following question.

Is it correct that the implementation is currently working as in the following picture?

If so, this would still mean that we establish a Celum connection for each asset before we retrieve the publicUrl, which leads to major performance impacts during page rendering (should the cache be empty).
Could this be changed so that when we request the asset URL and a publicUrl is available, we get this URL directly from the Cache or JCR, without checking Celum's availability first?

 

Comment by Raphael Falvo [ 03/Oct/23 ]

Hi shuettenrauch 

That is correct. We rely on external DAM API for the implementation, and there is nothing that currently exists or that we can extend to proceed with that request. Before obtaining an asset URL, you first need an asset.

I must admit, I am not particularly fond of creating a fake Celum asset when Celum is down.

However, considering that the JCR caching of these URLs is specific to the Celum connector implementation, I can suggest having a dedicated Celum function available: celfn.getPublicUrl(id, description). This function would allow you to retrieve the Celum asset URL based on its ID and description.

Would that be a viable option for you? If this solution meets the original requirement, I am open to reverting the changes made in the previous 1.0.5 beta versions and providing a new beta.

Comment by Stefan Huettenrauch [ 04/Oct/23 ]

Hi Raphael,

thank you for your suggestion. As discussed in the chat, please also create a method celfn.getPublicUrl(id) to use an empty description.

The most important part is that for rendering images in our components we should not create a remote celum asset, if the publicUrl is already available in Cache or JCR. Only if this is not the case should we create a remote celum asset to get the publicUrl directly from Celum.

As mentioned in CELUM-57, it is important that if celum is not available, the method should return rather an empty string than throw an exception to not stop the rendering of the page.

Comment by Raphael Falvo [ 13/Oct/23 ]

Hi shuettenrauch 
As agreed, I've just built up a new beta version 1.0.5-beta4 of the module, embedding CELUM-51 with the new strategy.

When you need to get a public URL from the JCR URL cache, you can use now the templating function that the module defines. It is named: damcelfn

You can call that method : damcelfn.getPublicUrl(assetId) where assetId is the composite asset id usually stored in JCR when referencing an asset, e.g. celum:12345
It provides also the one with extra description parameter damcelfn.getPublicUrl(id, description), but you probably won't need that one.

It also contains fix for CELUM-57

I'm looking forward to your feedback.

 

Comment by Stefan Huettenrauch [ 19/Oct/23 ]

Hi rfalvo,

thank you for your beta4, which we have tested.

Here is some feedback from us.

The new methods of the damcelfn are useful for rendering the image and in this case preventing Celum access if the publicUrl is available.
However, we see that when using the new methods, we have to add an if statement to our code: if damcelfn.getPublicUrl(assetId) == "" then call asset using CelumAssetService.
Is our understanding here correct, that we need this decision in our template code?

From our perspective it would still be the best, if the Celum Connector provides an almost empty asset object, filled with all data available in JCR, before establishing a Celum connection. And then, if e.g. getName() was called on the asset the connection would be established and the requested attribute value would be put into the object. (You called it fake asset above, but we think it is really an almost empty asset object that would be created with local data only.)
Could you elaborate on the reason why you wouldn't want to implement it this way?
We think it would be better to have all decisions regarding data retrieval within a lazy-loading Celum Connector, than having to make a check on all places using the damcelfn methods.

What do you think?

Comment by Raphael Falvo [ 02/Nov/23 ]

Hello shuettenrauch 

The JCR caching is a specific implementation designed for the Celum connector, while the rest of the implementation is inherited from the external DAM connector pack. The external DAM connector assumes that the DAM system is up and running. 

If you want to access and check the JCR URLs cache, you’ll need to perform the test in Freemarker, that’s the recommended approach. We prefer not to mix these two aspects, especially because we don’t have full control, as we are dependent on the external DAM API.

Comment by Stefan Huettenrauch [ 06/Nov/23 ]

Hi rfalvo,

we still think that the connector should not assume an up DAM system when assets are played out to the end user.
However, we will now do the checks ourselves. This means we first check for the public URL from JCR using your new templating function. Then, if the return is empty, we will use the normal connector functions to handle the asset.

I believe that we can close this ticket.

Thank you again for your work and discussions around this topic.

 

Comment by Sebastian Pertinger [ 06/Nov/23 ]

Hi there,

but why are you not using the getPublicUrlQuery(CelumService) method in CelumFunctions.java. With that it would try to grasp the publicUrl and if there is none fetch it from Celum?

Comment by Raphael Falvo [ 13/Nov/23 ]

Included in final 1.0.5 version which got released today

Generated at Sun Feb 11 23:58:34 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.