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

Use EntityUtilProperties to get the WebSiteProperties

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: Release Branch 12.04, Release Branch 13.07, Release Branch 14.12, Trunk
    • Fix Version/s: 14.12.01, 13.07.02, 16.11.01
    • Component/s: framework
    • Labels:
      None

      Description

      Use EntityUtilProperties to get the WebSiteProperties, As per current implementation its using UtilProperties and its affecting the multitenant environment.

      Wrong https url creation for a tenant in multi-tenant environment. Its redirecting to default instance.

        Issue Links

          Activity

          Hide
          adrianc@hlmksw.com Adrian Crum added a comment -

          It would be nice if we stopped using that awful implementation and started using the EntityClassLoader instead. See rev 1649620.

          Show
          adrianc@hlmksw.com Adrian Crum added a comment - It would be nice if we stopped using that awful implementation and started using the EntityClassLoader instead. See rev 1649620.
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Hi Arun, is it related with http://markmail.org/message/ty7uczwsucurytim ?

          Show
          jacques.le.roux Jacques Le Roux added a comment - Hi Arun, is it related with http://markmail.org/message/ty7uczwsucurytim ?
          Hide
          arunpati Arun Patidar added a comment -

          Hi Jacques Le Roux

          no, its not related to that.

          Show
          arunpati Arun Patidar added a comment - Hi Jacques Le Roux no, its not related to that.
          Hide
          jacopoc Jacopo Cappellato added a comment -

          That is an interesting solution: however I am concerned that overloading the class loaders with all the lookups for entity/system resources could have an impact on performance and system's load.
          I am not saying it is a bad idea, however I would feel more comfortable if these issues are fixed with the existing approach and then converted all at once into EntityClassLoader calls when we are sure we know the impact on performance (with profiling and performance tests).

          Show
          jacopoc Jacopo Cappellato added a comment - That is an interesting solution: however I am concerned that overloading the class loaders with all the lookups for entity/system resources could have an impact on performance and system's load. I am not saying it is a bad idea, however I would feel more comfortable if these issues are fixed with the existing approach and then converted all at once into EntityClassLoader calls when we are sure we know the impact on performance (with profiling and performance tests).
          Hide
          arunpati Arun Patidar added a comment -

          Attached is the patch with the current approach. I like Jacopo's idea to replace all at once when we finalize the EntityClassLoader fix and its performance impact if any.

          Show
          arunpati Arun Patidar added a comment - Attached is the patch with the current approach. I like Jacopo's idea to replace all at once when we finalize the EntityClassLoader fix and its performance impact if any.
          Hide
          pfm.smits Pierre Smits added a comment - - edited

          EntityUtilProperties.getPropertyValue is also in older branches. r1649620 was implemented in jan 5, 2015. Having an associated JIRA issue will ensure that it pops up in release notes.

          Show
          pfm.smits Pierre Smits added a comment - - edited EntityUtilProperties.getPropertyValue is also in older branches. r1649620 was implemented in jan 5, 2015. Having an associated JIRA issue will ensure that it pops up in release notes.
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          +1 with some more explanations on how to use it for everybody to understand what's at stake

          Show
          jacques.le.roux Jacques Le Roux added a comment - +1 with some more explanations on how to use it for everybody to understand what's at stake
          Hide
          adrianc@hlmksw.com Adrian Crum added a comment -

          Issue has been created - OFBIZ-6210.

          There is no performance impact using EntityClassLoader. Others are welcome to run performance tests if they wish, but a simple review of the code will make it clear that the difference boils down to reading a file from the local file system versus reading a file from the database (that same difference exists now with the EntityUtilProperties implementation).

          Show
          adrianc@hlmksw.com Adrian Crum added a comment - Issue has been created - OFBIZ-6210 . There is no performance impact using EntityClassLoader. Others are welcome to run performance tests if they wish, but a simple review of the code will make it clear that the difference boils down to reading a file from the local file system versus reading a file from the database (that same difference exists now with the EntityUtilProperties implementation).
          Hide
          jacopoc Jacopo Cappellato added a comment -

          But the way class loaders work is that when you search for an (entity) resource, the class loader will first call the parent class loader (and up at the root of the tree) before using the EntityClassLoader's methods.

          Show
          jacopoc Jacopo Cappellato added a comment - But the way class loaders work is that when you search for an (entity) resource, the class loader will first call the parent class loader (and up at the root of the tree) before using the EntityClassLoader's methods.
          Hide
          jacques.le.roux Jacques Le Roux added a comment - - edited

          Does this not relies on the file system? Then the OS (Linux at least) should cache, no?
          I mean to find the class file.

          Show
          jacques.le.roux Jacques Le Roux added a comment - - edited Does this not relies on the file system? Then the OS (Linux at least) should cache, no? I mean to find the class file.
          Hide
          adrianc@hlmksw.com Adrian Crum added a comment -

          Can we continue this discussion in OFBIZ-6210 please?

          Show
          adrianc@hlmksw.com Adrian Crum added a comment - Can we continue this discussion in OFBIZ-6210 please?
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Right , We can't resist to a reply button

          Show
          jacques.le.roux Jacques Le Roux added a comment - Right , We can't resist to a reply button
          Hide
          deepak.dixit Deepak Dixit added a comment -

          Thanks Arun your changes has been committed at
          Trunk at r#1672846
          14.12 at r#1672848
          13.07 at r#1672849

          EntityUtilProperties implementation exists for 13.07 branches as well, so committing patch for trunk and related branches.
          As Jacopo said will convert all EntityUtilProperties implementation at once into EntityClassLoader when we are sure about performance impact.

          Show
          deepak.dixit Deepak Dixit added a comment - Thanks Arun your changes has been committed at Trunk at r#1672846 14.12 at r#1672848 13.07 at r#1672849 EntityUtilProperties implementation exists for 13.07 branches as well, so committing patch for trunk and related branches. As Jacopo said will convert all EntityUtilProperties implementation at once into EntityClassLoader when we are sure about performance impact.

            People

            • Assignee:
              deepak.dixit Deepak Dixit
              Reporter:
              arunpati Arun Patidar
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development