You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by ja...@apache.org on 2012/07/26 09:03:46 UTC

svn commit: r1365895 - /ofbiz/trunk/framework/service/src/org/ofbiz/service/engine/AbstractEngine.java

Author: jacopoc
Date: Thu Jul 26 07:03:46 2012
New Revision: 1365895

URL: http://svn.apache.org/viewvc?rev=1365895&view=rev
Log:
Made locationMap thread safe using static initialization and immutability.

Modified:
    ofbiz/trunk/framework/service/src/org/ofbiz/service/engine/AbstractEngine.java

Modified: ofbiz/trunk/framework/service/src/org/ofbiz/service/engine/AbstractEngine.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/engine/AbstractEngine.java?rev=1365895&r1=1365894&r2=1365895&view=diff
==============================================================================
--- ofbiz/trunk/framework/service/src/org/ofbiz/service/engine/AbstractEngine.java (original)
+++ ofbiz/trunk/framework/service/src/org/ofbiz/service/engine/AbstractEngine.java Thu Jul 26 07:03:46 2012
@@ -18,12 +18,12 @@
  *******************************************************************************/
 package org.ofbiz.service.engine;
 
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.Map;
 import java.util.List;
 import java.util.Iterator;
 
-import javolution.util.FastMap;
-
 import org.ofbiz.service.ServiceDispatcher;
 import org.ofbiz.service.ModelService;
 import org.ofbiz.service.GenericServiceException;
@@ -41,40 +41,38 @@ import org.w3c.dom.Element;
 public abstract class AbstractEngine implements GenericEngine {
 
     public static final String module = AbstractEngine.class.getName();
-    protected static Map<String, String> locationMap = null;
+    protected static final Map<String, String> locationMap = createLocationMap();
 
     protected ServiceDispatcher dispatcher = null;
 
     protected AbstractEngine(ServiceDispatcher dispatcher) {
         this.dispatcher = dispatcher;
-        initLocations();
     }
 
     // creates the location alias map
-    protected synchronized void initLocations() {
-        if (locationMap == null) {
-            locationMap = FastMap.newInstance();
-
-            Element root = null;
-            try {
-                root = ServiceConfigUtil.getXmlRootElement();
-            } catch (GenericConfigException e) {
-                Debug.logError(e, module);
-            }
+    protected static Map<String, String> createLocationMap() {
+        Map<String, String> tmpMap = new HashMap<String, String>();
+
+        Element root = null;
+        try {
+            root = ServiceConfigUtil.getXmlRootElement();
+        } catch (GenericConfigException e) {
+            Debug.logError(e, module);
+        }
 
-            if (root != null) {
-                List<? extends Element> locationElements = UtilXml.childElementList(root, "service-location");
-                if (locationElements != null) {
-                    for (Element e: locationElements) {
-                        locationMap.put(e.getAttribute("name"), e.getAttribute("location"));
-                    }
+        if (root != null) {
+            List<? extends Element> locationElements = UtilXml.childElementList(root, "service-location");
+            if (locationElements != null) {
+                for (Element e: locationElements) {
+                    tmpMap.put(e.getAttribute("name"), e.getAttribute("location"));
                 }
             }
-            Debug.logInfo("Loaded Service Locations : " + locationMap, module);
         }
+        Debug.logInfo("Loaded Service Locations: " + tmpMap, module);
+        return Collections.unmodifiableMap(tmpMap);
     }
 
-    // uses the lookup map to determin if the location has been aliased in serviceconfig.xml
+    // uses the lookup map to determine if the location has been aliased in serviceconfig.xml
     protected String getLocation(ModelService model) {
         if (locationMap.containsKey(model.location)) {
             return locationMap.get(model.location);



Re: svn commit: r1365895 - /ofbiz/trunk/framework/service/src/org/ofbiz/service/engine/AbstractEngine.java

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
Thank you Jacques, I have improved that old comment in rev. 1366294

Jacopo


On Jul 27, 2012, at 7:15 AM, Jacques Le Roux wrote:

>> -    // uses the lookup map to determin if the location has been aliased in serviceconfig.xml
>> +    // uses the lookup map to determine if the location has been aliased in serviceconfig.xml
>>    protected String getLocation(ModelService model) {
>>        if (locationMap.containsKey(model.location)) {
>>            return locationMap.get(model.location);
> 
> If serviceconfig.xml  means serviceengine.xml here and location means service-location, could we change the comment here? Else just
> discard this comment (not enough time to digg in)
> 
> Jacques


Re: svn commit: r1365895 - /ofbiz/trunk/framework/service/src/org/ofbiz/service/engine/AbstractEngine.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
From: <ja...@apache.org>
> Author: jacopoc
> Date: Thu Jul 26 07:03:46 2012
> New Revision: 1365895
>
> URL: http://svn.apache.org/viewvc?rev=1365895&view=rev
> Log:
> Made locationMap thread safe using static initialization and immutability.
>
> Modified:
>    ofbiz/trunk/framework/service/src/org/ofbiz/service/engine/AbstractEngine.java
>
> Modified: ofbiz/trunk/framework/service/src/org/ofbiz/service/engine/AbstractEngine.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/engine/AbstractEngine.java?rev=1365895&r1=1365894&r2=1365895&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/service/src/org/ofbiz/service/engine/AbstractEngine.java (original)
> +++ ofbiz/trunk/framework/service/src/org/ofbiz/service/engine/AbstractEngine.java Thu Jul 26 07:03:46 2012
> @@ -18,12 +18,12 @@
>  *******************************************************************************/
> package org.ofbiz.service.engine;
>
> +import java.util.Collections;
> +import java.util.HashMap;
> import java.util.Map;
> import java.util.List;
> import java.util.Iterator;
>
> -import javolution.util.FastMap;
> -
> import org.ofbiz.service.ServiceDispatcher;
> import org.ofbiz.service.ModelService;
> import org.ofbiz.service.GenericServiceException;
> @@ -41,40 +41,38 @@ import org.w3c.dom.Element;
> public abstract class AbstractEngine implements GenericEngine {
>
>     public static final String module = AbstractEngine.class.getName();
> -    protected static Map<String, String> locationMap = null;
> +    protected static final Map<String, String> locationMap = createLocationMap();
>
>     protected ServiceDispatcher dispatcher = null;
>
>     protected AbstractEngine(ServiceDispatcher dispatcher) {
>         this.dispatcher = dispatcher;
> -        initLocations();
>     }
>
>     // creates the location alias map
> -    protected synchronized void initLocations() {
> -        if (locationMap == null) {
> -            locationMap = FastMap.newInstance();
> -
> -            Element root = null;
> -            try {
> -                root = ServiceConfigUtil.getXmlRootElement();
> -            } catch (GenericConfigException e) {
> -                Debug.logError(e, module);
> -            }
> +    protected static Map<String, String> createLocationMap() {
> +        Map<String, String> tmpMap = new HashMap<String, String>();
> +
> +        Element root = null;
> +        try {
> +            root = ServiceConfigUtil.getXmlRootElement();
> +        } catch (GenericConfigException e) {
> +            Debug.logError(e, module);
> +        }
>
> -            if (root != null) {
> -                List<? extends Element> locationElements = UtilXml.childElementList(root, "service-location");
> -                if (locationElements != null) {
> -                    for (Element e: locationElements) {
> -                        locationMap.put(e.getAttribute("name"), e.getAttribute("location"));
> -                    }
> +        if (root != null) {
> +            List<? extends Element> locationElements = UtilXml.childElementList(root, "service-location");
> +            if (locationElements != null) {
> +                for (Element e: locationElements) {
> +                    tmpMap.put(e.getAttribute("name"), e.getAttribute("location"));
>                 }
>             }
> -            Debug.logInfo("Loaded Service Locations : " + locationMap, module);
>         }
> +        Debug.logInfo("Loaded Service Locations: " + tmpMap, module);
> +        return Collections.unmodifiableMap(tmpMap);
>     }
>
> -    // uses the lookup map to determin if the location has been aliased in serviceconfig.xml
> +    // uses the lookup map to determine if the location has been aliased in serviceconfig.xml
>     protected String getLocation(ModelService model) {
>         if (locationMap.containsKey(model.location)) {
>             return locationMap.get(model.location);

If serviceconfig.xml  means serviceengine.xml here and location means service-location, could we change the comment here? Else just
discard this comment (not enough time to digg in)

Jacques