Details
-
Bug
-
Resolution: Fixed
-
Neutral
-
1.0.2
-
None
-
-
Empty show more show less
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
Checklists
Attachments
Issue Links
- clones
-
CELUM-32 PublicURLs never refresh
-
- Closed
-