Uploaded image for project: 'Celum DAM Connector '
  1. Celum DAM Connector
  2. CELUM-34

Saving public URLs in JCR creates redundant and ambiguous nodes

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Neutral Neutral
    • 1.0.4
    • 1.0.2
    • None

      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

        Acceptance criteria

              rfalvo Raphael Falvo
              tszczepanski Tobias Szczepanski
              Services
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Created:
                Updated:
                Resolved: