[CELUM-34] Saving public URLs in JCR creates redundant and ambiguous nodes Created: 05/Apr/22  Updated: 21/Dec/22  Resolved: 19/Oct/22

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

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

Issue Links:
Cloners
clones CELUM-32 PublicURLs never refresh Closed
Template:
Acceptance criteria:
Empty
Date of First Response:
Team: Services

 Description   

Status quo (code)

info.magnolia.external.dam.celum.service.CelumJcrServiceImpl#savePublicUrlToJcr:

public void savePublicUrlToJcr(String url, String description, String assetId) {
  [...] Node node = findNode(session, assetId);
  [...] node = NodeUtil.createPath(session.getRootNode(), createPath() + assetId, ContentNode.NAME); 
} 

info.magnolia.external.dam.celum.service.CelumJcrServiceImpl#findNode:

protected Node findNode(Session session, String assetId) {
  [...] Query query = jcrQueryManager.createQuery(String.format(GET_QUERY, assetId), Query.JCR_SQL2);
} 

info.magnolia.external.dam.celum.service.CelumJcrServiceImpl#GET_QUERY

private static final String GET_QUERY = "SELECT * from ['mgnl:contentNode'] as t WHERE name(t) ='%s'";

The following is tremendously prone to conflicts and causes a bug with the used GET_QUERY:
info.magnolia.external.dam.celum.service.CelumJcrServiceImpl#createPath:

private String createPath() {
  return "" + new Random().nextInt(1000) + CelumConstants.SLASH;
}

Explaination

The above results in multiple nodes for one single celum asset, e.g. with asset-id 100
/0/102, /100/102, /567/102, /999/102, etc.

Each rendition is potentially written into a different node, e.g.
/0/102.original=https://cdn.url/original/asset-102.jpg,
/100/102.small=https://cdn.url/small/asset-102.jpg, etc.

Given the asset id 100, the GET_QUERY finds all nodes which are named 100, but the findNode-method returns only the first found node:

if (iterator.hasNext()) {
  return iterator.nextNode();
}

Suggestion

Get rid of createPath(), in particular of the random parent node name ranging 0-999
Use the same Path as in the Asset's itemKey (assetId) as the absolut path of the public-urls-node
Write all public-urls as properties under the same node
And please write unit tests, because this kind of design issues come to light very quickly during test-driven development



 Comments   
Comment by Teresa Miyar [ 10/Oct/22 ]

Hi,

This two points are valid:

Write all public-urls as properties under the same node
And please write unit tests, because this kind of design issues come to light very quickly during test-driven development

but we cannot save the assets using the same path they have in the Celum server since we want to avoid the problem of having too many nodes under the same path due to JCR limitation and the fact that you have folders with more than 40 thousand items

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