Uploaded image for project: 'OFBiz'
  1. OFBiz
  2. OFBIZ-5658

checkExistsImage method in ImageManagementServices

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Trivial
    • Resolution: Fixed
    • Affects Version/s: Release Branch 13.07
    • Fix Version/s: 14.12.01, 12.04.06, 13.07.02
    • Component/s: product
    • Labels:
      None
    • Environment:

      any

      Description

      I think indexOf(".") in these code should be lastIndexOf(".")

      public static File checkExistsImage(File file) {
      if (!file.exists())

      { imageCount = 0; imagePath = null; return file; }

      imageCount++;
      String filePath = imagePath.substring(0, imagePath.indexOf("."));
      String type = imagePath.substring(imagePath.indexOf(".") + 1);
      file = new File(filePath + "(" + imageCount + ")." + type);
      return checkExistsImage(file);
      }

        Activity

        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Before applying this change, I have some questions. Did you ever get images upload to work? If yes could you give here the steps you used.

        Thanks

        Show
        jacques.le.roux Jacques Le Roux added a comment - Before applying this change, I have some questions. Did you ever get images upload to work? If yes could you give here the steps you used. Thanks
        Hide
        chunlinyao chunlinyao added a comment -

        from CATALOG -> Image Management -> Upload view. upload some Images it will call this method. This method is intended to add (count) to the image name, when the file name was already exist. I debugged in it, the imagePath was the absolute path of the image file. So If any parent folder name contains "." , it will not works correctly.
        And after review the whole ImageManagementServices.java file. I found it use static variables (ex. imageCount imagePath). It is not thread safe.

        Show
        chunlinyao chunlinyao added a comment - from CATALOG -> Image Management -> Upload view. upload some Images it will call this method. This method is intended to add (count) to the image name, when the file name was already exist. I debugged in it, the imagePath was the absolute path of the image file. So If any parent folder name contains "." , it will not works correctly. And after review the whole ImageManagementServices.java file. I found it use static variables (ex. imageCount imagePath). It is not thread safe.
        Hide
        jacques.le.roux Jacques Le Roux added a comment - - edited

        Thanks for the explanation, I agree about your fix and I will commit it.

        I know this Jira is only about this issue. But, the thread safe issue apart, I still wonder if you got this feature (the Image Management as a whole) to work? And If yes, if you could share how you did?

        There are so much things I don't like in this feature, that I don't know where to begin. But the major one is, despite nothing working as expected in the UI (with errors when uploading for instance), that the images are loaded under framework/images/webapp/images/products/management. I'd prefer those to be under runtime (like we have the Lucene indexes for instance) or even a specific hot-deploy folder, etc.

        Show
        jacques.le.roux Jacques Le Roux added a comment - - edited Thanks for the explanation, I agree about your fix and I will commit it. I know this Jira is only about this issue. But, the thread safe issue apart, I still wonder if you got this feature (the Image Management as a whole) to work? And If yes, if you could share how you did? There are so much things I don't like in this feature, that I don't know where to begin. But the major one is, despite nothing working as expected in the UI (with errors when uploading for instance), that the images are loaded under framework/images/webapp/images/products/management. I'd prefer those to be under runtime (like we have the Lucene indexes for instance) or even a specific hot-deploy folder, etc.
        Hide
        pfm.smits Pierre Smits added a comment -

        I believe that is why Sascha started to work on the integration of Apache JackRabbit.. To have a configurable setup to store all kinds of documents etc in a external scalable system.

        Regards,

        Pierre

        Show
        pfm.smits Pierre Smits added a comment - I believe that is why Sascha started to work on the integration of Apache JackRabbit.. To have a configurable setup to store all kinds of documents etc in a external scalable system. Regards, Pierre
        Hide
        chunlinyao chunlinyao added a comment -

        I started learning OFBiz recently. So I haven't use Image Management in other place. Just playing with the vanilla OFBiz. Upload images from Image Management works. And I can view the uploaded images from ecommerce webapp.
        The Image Management can not upload at first, I created a tmp folder under runtime to fix it.

        The images for products is not processed consistently. We can upload from Product -> Content view OR upload from Image Management. And they saved in different places.

        Show
        chunlinyao chunlinyao added a comment - I started learning OFBiz recently. So I haven't use Image Management in other place. Just playing with the vanilla OFBiz. Upload images from Image Management works. And I can view the uploaded images from ecommerce webapp. The Image Management can not upload at first, I created a tmp folder under runtime to fix it. The images for products is not processed consistently. We can upload from Product -> Content view OR upload from Image Management. And they saved in different places.
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Thanks chunlinyao,

        Your patch is in
        trunk r1604554
        R13.07 r1604556
        R12.04 r1604557
        R11.04 r1604558

        Show
        jacques.le.roux Jacques Le Roux added a comment - Thanks chunlinyao, Your patch is in trunk r1604554 R13.07 r1604556 R12.04 r1604557 R11.04 r1604558
        Hide
        jacques.le.roux Jacques Le Roux added a comment - - edited

        Hi Pierre,

        Actually Sascha started to work on the integration of Apache JackRabbit after we both finished the jQuery merge with Ankit Jain and Atul Vani. Then he asked me which other task would be worth to work on, and I suggested Apache JackRabbit which I had in mind for a while.

        He did a great job but unfortunately at this moment I was not available enough to help him and after he left from Lynx his interest for OFBiz decreased at the point it is now. It's is a pity, Sascha was really a such driving force! But that's how the world goes

        And yes, that's one of the reasons having Apache JackRabbit integrated in OFBiz would be great. Unfortunately like some other main interesting features (SEO branch for instance) I have not enough continued time to really tackle the challenges alone. Only few cycles here and there where I try to cope with easier stuff...

        Show
        jacques.le.roux Jacques Le Roux added a comment - - edited Hi Pierre, Actually Sascha started to work on the integration of Apache JackRabbit after we both finished the jQuery merge with Ankit Jain and Atul Vani. Then he asked me which other task would be worth to work on, and I suggested Apache JackRabbit which I had in mind for a while. He did a great job but unfortunately at this moment I was not available enough to help him and after he left from Lynx his interest for OFBiz decreased at the point it is now. It's is a pity, Sascha was really a such driving force! But that's how the world goes And yes, that's one of the reasons having Apache JackRabbit integrated in OFBiz would be great. Unfortunately like some other main interesting features (SEO branch for instance) I have not enough continued time to really tackle the challenges alone. Only few cycles here and there where I try to cope with easier stuff...
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Hi chunlinyao,

        Did you not encouters errors (reported in UI for instance)? Are you using a *nix OS (I use Windows hence the question)?

        >The Image Management can not upload at first, I created a tmp folder under runtime to fix it.
        But we have already a tmp folder under runtime

        >The images for products is not processed consistently. We can upload from Product -> Content view OR upload from Image Management. And they saved in different places.
        Yes the content way existed before and is reliable. I initially thought that the Image Management was an improvement upon it, but it seems it's totally dissociated, which is a pity (the content way is a bit tedious). It would be great if we could combine them. But anyway I was just curious, the project I'm currently working on does not use the OFBiz UI, only OFBiz services. So I'm not that interested for now, thanks to share your experience!

        Show
        jacques.le.roux Jacques Le Roux added a comment - Hi chunlinyao, Did you not encouters errors (reported in UI for instance)? Are you using a *nix OS (I use Windows hence the question)? >The Image Management can not upload at first, I created a tmp folder under runtime to fix it. But we have already a tmp folder under runtime >The images for products is not processed consistently. We can upload from Product -> Content view OR upload from Image Management. And they saved in different places. Yes the content way existed before and is reliable. I initially thought that the Image Management was an improvement upon it, but it seems it's totally dissociated, which is a pity (the content way is a bit tedious). It would be great if we could combine them. But anyway I was just curious, the project I'm currently working on does not use the OFBiz UI, only OFBiz services. So I'm not that interested for now, thanks to share your experience!
        Hide
        pfm.smits Pierre Smits added a comment -

        Jacques,

        I know. He and I collaborated on getting the multi-tenancy aspect of the JackRabbit integration working.

        Regards,

        Pierre

        Show
        pfm.smits Pierre Smits added a comment - Jacques, I know. He and I collaborated on getting the multi-tenancy aspect of the JackRabbit integration working. Regards, Pierre
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Ha did not remember that, thanks for the reminder. Hopefully one day we will find enough HR to finish this work...

        Show
        jacques.le.roux Jacques Le Roux added a comment - Ha did not remember that, thanks for the reminder. Hopefully one day we will find enough HR to finish this work...
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Also I was able to have the Image Management working on the newly available official demos in both trunk and stable.

        So I apologies for the rants, it seems it's either my installation... or it does not work on Windows (which is still bad )...

        Show
        jacques.le.roux Jacques Le Roux added a comment - Also I was able to have the Image Management working on the newly available official demos in both trunk and stable. So I apologies for the rants, it seems it's either my installation... or it does not work on Windows (which is still bad )...
        Hide
        chunlinyao chunlinyao added a comment -

        Hi Jacques,

        > Did you not encouters errors (reported in UI for instance)? Are you using a *nix OS (I use Windows hence the question)?
        Yes I use OSX not Windows.

        Show
        chunlinyao chunlinyao added a comment - Hi Jacques, > Did you not encouters errors (reported in UI for instance)? Are you using a *nix OS (I use Windows hence the question)? Yes I use OSX not Windows.
        Hide
        pandeypranay Pranay Pandey added a comment -

        Hi Jacques,

        Are we good to close this issue?

        Show
        pandeypranay Pranay Pandey added a comment - Hi Jacques, Are we good to close this issue?
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        There is nothing missing there. I will also close OFBIZ-5501 after fixing it.

        Show
        jacques.le.roux Jacques Le Roux added a comment - There is nothing missing there. I will also close OFBIZ-5501 after fixing it.
        Hide
        pandeypranay Pranay Pandey added a comment -

        Thanks Jacques.

        Show
        pandeypranay Pranay Pandey added a comment - Thanks Jacques.

          People

          • Assignee:
            jacques.le.roux Jacques Le Roux
            Reporter:
            chunlinyao chunlinyao
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development