Uploaded image for project: 'eZ Publish / Platform'
  1. eZ Publish / Platform
  2. EZP-22021

PAPI image/binary should avoid using orignal name & hitting filesystem node limits

    Details

      Description

      When creating image objects through the Public API, image content is placed directly in the legacy images root, for example:

          var/<something>/storage/images/1712-1-eng-GB/Chrysanthemum.jpg
      

      However in this case where the Content name is "test", it should be something like

          var/<something>/storage/images/171/2/1/eng-GB/test.jpg
      

      The example is just an example, this can be solved different ways, here it makes sure all id's are splinted per 3 digits, it could be 4. Also the language folder can be skipped if field is untranslatable, but only if this is easily achievable.

        Issue Links

          Activity

          Hide
          Bertrand Dunogier added a comment - - edited

          Do we have anything that does string transformation yet in the Public API to sanitize the name to an URI ?

          We have faced such issues in the past, but I don't think we had to deal with this so far in eZ 5. The closest we have for now would be the SlugConverter we use for URL aliases.

          Show
          Bertrand Dunogier added a comment - - edited Do we have anything that does string transformation yet in the Public API to sanitize the name to an URI ? We have faced such issues in the past, but I don't think we had to deal with this so far in eZ 5. The closest we have for now would be the SlugConverter we use for URL aliases.
          Hide
          Bertrand Dunogier added a comment -

          Wouldn't it make sense to try the alternative text first, then the content's name if there's none ? It could make more sense when there are more than one image.

          Show
          Bertrand Dunogier added a comment - Wouldn't it make sense to try the alternative text first, then the content's name if there's none ? It could make more sense when there are more than one image.
          Hide
          André Rømcke added a comment -

          > Wouldn't it make sense to try the alternative text first, then the content's name if there's none ? It could make more sense when there are more than one image.

          yes

          As for existing code, that would be question for [~jerome.vieilledent@ez.no] and Petar Spanja.

          Show
          André Rømcke added a comment - > Wouldn't it make sense to try the alternative text first, then the content's name if there's none ? It could make more sense when there are more than one image. yes As for existing code, that would be question for [~jerome.vieilledent@ez.no] and Petar Spanja .
          Hide
          Petar Spanja (Inactive) added a comment -

          @Bertrand Dunogier

          SlugConverter could definitely be used for this, but first it should be moved up to the eZ\Publish\Core\Persistence namespace.

          Also +1 for using alt text first.

          Show
          Petar Spanja (Inactive) added a comment - @ Bertrand Dunogier SlugConverter could definitely be used for this, but first it should be moved up to the eZ\Publish\Core\Persistence namespace. Also +1 for using alt text first.
          Hide
          Bertrand Dunogier added a comment - - edited

          After discussing it here, some people (and I could actually agree) think that using the uploaded filename was actually a better solution:

          • it doesn't enforce a business rule that may or may not fit each user
          • it works as it did before
          • it gives freedom to really optimize how your files are named if you need to
          Show
          Bertrand Dunogier added a comment - - edited After discussing it here, some people (and I could actually agree) think that using the uploaded filename was actually a better solution: it doesn't enforce a business rule that may or may not fit each user it works as it did before it gives freedom to really optimize how your files are named if you need to
          Hide
          Bertrand Dunogier added a comment -
          Show
          Bertrand Dunogier added a comment - ezpublish-kernel#638
          Hide
          Bertrand Dunogier added a comment - - edited

          ezpublish-kernel#638 merged to ezpublish-kernel/master in e85c03b.

          Show
          Bertrand Dunogier added a comment - - edited ezpublish-kernel#638 merged to ezpublish-kernel/master in e85c03b .
          Hide
          Bertrand Dunogier added a comment -

          Documentation: need to mention how files stored by the FieldType are structured, and by whom (PathGenerator).

          Show
          Bertrand Dunogier added a comment - Documentation: need to mention how files stored by the FieldType are structured, and by whom (PathGenerator).
          Hide
          Bertrand Dunogier added a comment - - edited

          Documentation updated.

          Added https://confluence.ez.no/display/EZP/The+Image+FieldType, that (briefly) explains how images are stored. Linked from https://confluence.ez.no/display/EZP/FieldTypes.

          Should the doc be backported to the previous versions spaces ?

          Show
          Bertrand Dunogier added a comment - - edited Documentation updated. Added https://confluence.ez.no/display/EZP/The+Image+FieldType , that (briefly) explains how images are stored. Linked from https://confluence.ez.no/display/EZP/FieldTypes . Should the doc be backported to the previous versions spaces ?
          Hide
          Bertrand Dunogier added a comment -

          Damn... I went back & forth through multiple implementations, and didn't think I had ended up with this one.

          Not sure what I'll do, but it does sound like a recommit... the alternative would be to completely revert this, and ship a new, independent patch.

          Show
          Bertrand Dunogier added a comment - Damn... I went back & forth through multiple implementations, and didn't think I had ended up with this one. Not sure what I'll do, but it does sound like a recommit... the alternative would be to completely revert this, and ship a new, independent patch.
          Hide
          Bertrand Dunogier added a comment - - edited

          Okay. The previously referenced commits will have to be ignored.

          I have made a new PR that:

          • reverts the previously merged fix
          • refixes the issue, with corrections.

          New pull-request: ezpublish-kernel#641.

          Show
          Bertrand Dunogier added a comment - - edited Okay. The previously referenced commits will have to be ignored. I have made a new PR that: reverts the previously merged fix refixes the issue, with corrections. New pull-request: ezpublish-kernel#641 .
          Show
          Bertrand Dunogier added a comment - ezpublish-kernel#641 merged to ezpublish-kernel/master@84dda86 .
          Hide
          Joao Pingo (Inactive) added a comment -

          QA Approved

          Show
          Joao Pingo (Inactive) added a comment - QA Approved

            People

            • Assignee:
              Unassigned
              Reporter:
              André Rømcke
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Time Spent - 2 days, 3 hours, 30 minutes Remaining Estimate - 5 hours
                5h
                Logged:
                Time Spent - 2 days, 3 hours, 30 minutes Remaining Estimate - 5 hours
                2d 3h 30m