Possible fix for ALF-4409: Locale-independent properties can be given different locales

- 3.4 de-Hibernate problem
 - The DAO was not *ever* persisting '.default' so could give back different IDs for getOrCreateDefaultLocale().
 - Obviously, the NodeService (and all other clients) would expect the same ID for the default locale.
 - No need to change the NodeService property code - it's behaving correctly.


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@21985 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

source/java/org/alfresco/repo/domain/locale/AbstractLocaleDAOImpl.java

@@ -81,21 +81,25 @@ public abstract class AbstractLocaleDAOImpl implements LocaleDAO
         this.localeEntityCache = new EntityLookupCache<Long, String, String>(new LocaleEntityCallbackDAO());
     }
     
+    /**
+     * {@inheritDoc}
+     */
     public Pair<Long, Locale> getLocalePair(Locale locale)
     {
-        if (locale == null)
-        {
-            throw new IllegalArgumentException("Locale cannot be null");
-        }
-        
         return getLocalePairImpl(locale);
     }
     
+    /**
+     * {@inheritDoc}
+     */
     public Pair<Long, Locale> getDefaultLocalePair()
     {
         return getLocalePairImpl(null);
     }
     
+    /**
+     * {@inheritDoc}
+     */
     public Pair<Long, Locale> getLocalePair(Long id)
     {
         if (id == null)
@@ -108,42 +112,44 @@ public abstract class AbstractLocaleDAOImpl implements LocaleDAO
         {
             throw new DataIntegrityViolationException("No locale exists for ID " + id);
         }
-        
+        String localeStr = entityPair.getSecond();
         // Convert the locale string to a locale
-        Locale locale = DefaultTypeConverter.INSTANCE.convert(Locale.class, entityPair.getSecond());
+        Locale locale = null;
+        if (LocaleEntity.DEFAULT_LOCALE_SUBSTITUTE.equals(localeStr))
+        {
+            locale = I18NUtil.getLocale();
+        }
+        else
+        {
+            locale = DefaultTypeConverter.INSTANCE.convert(Locale.class, entityPair.getSecond());
+        }
+        
         return new Pair<Long, Locale>(id, locale);
     }
     
+    /**
+     * {@inheritDoc}
+     */
     public Pair<Long, Locale> getOrCreateLocalePair(Locale locale)
     {
-        if (locale == null)
+        return getOrCreateLocalePairImpl(locale);
-        {
-            throw new IllegalArgumentException("Locale cannot be null");
-        }
-        
-        String localeStr = DefaultTypeConverter.INSTANCE.convert(String.class, locale);
-        Pair<Long, String> entityPair = localeEntityCache.getOrCreateByValue(localeStr);
-        if (entityPair == null)
-        {
-            throw new RuntimeException("Locale should have been created.");
-        }
-        return new Pair<Long, Locale>(entityPair.getFirst(), locale);
     }
     
+    /**
+     * {@inheritDoc}
+     */
     public Pair<Long, Locale> getOrCreateDefaultLocalePair()
     {
-        Pair<Long, Locale> localePair = getDefaultLocalePair();
+        return getOrCreateLocalePairImpl(null);
-        if (localePair != null)
-        {
-            return localePair;
-        }
-        
-        LocaleEntity localeEntity = new LocaleEntity();
-        localeEntity.setLocale(null);
-        
-        return getOrCreateLocalePair(localeEntity.getLocale());
     }
     
+    /**
+     * Find the locale pair
+     * 
+     * @param locale                the locale to get or <tt>null</tt> to indicate the
+     *                              {@link LocaleEntity#DEFAULT_LOCALE_SUBSTITUTE default locale}.
+     * @return                      Returns the locale pair (ID, Locale) or <tt>null</tt> if not found.
+     */
     private Pair<Long, Locale> getLocalePairImpl(Locale locale)
     {
         // Null means look for the default
@@ -160,7 +166,7 @@ public abstract class AbstractLocaleDAOImpl implements LocaleDAO
         
         if (localeStr == null)
         {
-            throw new IllegalArgumentException("Cannot look up entity by null ID.");
+            throw new IllegalArgumentException("Cannot look up entity by null locale.");
         }
         
         Pair<Long, String> entityPair = localeEntityCache.getByValue(localeStr);
@@ -174,6 +180,40 @@ public abstract class AbstractLocaleDAOImpl implements LocaleDAO
         }
     }
     
+    /**
+     * Find or create the locale pair
+     * 
+     * @param locale                the locale to get or <tt>null</tt> to indicate the
+     *                              {@link LocaleEntity#DEFAULT_LOCALE_SUBSTITUTE default locale}.
+     * @return                      Returns the locale pair (ID, Locale), never <tt>null
+     */
+    private Pair<Long, Locale> getOrCreateLocalePairImpl(Locale locale)
+    {
+        // Null means look for the default
+        final String localeStr;
+        if (locale == null)
+        {
+            localeStr = LocaleEntity.DEFAULT_LOCALE_SUBSTITUTE;
+            locale = I18NUtil.getLocale();
+        }
+        else
+        {
+            localeStr = DefaultTypeConverter.INSTANCE.convert(String.class, locale);
+        }
+        
+        if (localeStr == null)
+        {
+            throw new IllegalArgumentException("Cannot look up entity by null locale.");
+        }
+        
+        Pair<Long, String> entityPair = localeEntityCache.getOrCreateByValue(localeStr);
+        if (entityPair == null)
+        {
+            throw new RuntimeException("Locale should have been created.");
+        }
+        return new Pair<Long, Locale>(entityPair.getFirst(), locale);
+    }
+    
     /**
      * Callback for <b>alf_locale</b> DAO
      */

source/java/org/alfresco/repo/domain/locale/LocaleDAO.java

@@ -26,8 +26,8 @@ package org.alfresco.repo.domain.locale;
 
 import java.util.Locale;
 
-import org.alfresco.error.AlfrescoRuntimeException;
 import org.alfresco.util.Pair;
+import org.springframework.dao.DataIntegrityViolationException;
 
 /**
  * Data abstraction layer for Locale entities.
@@ -39,8 +39,8 @@ public interface LocaleDAO
 {
     /**
      * @param id            the unique ID of the entity
-     * @return              the locale (never null)
+     * @return              the locale pair (never null)
-     * @throws              AlfrescoRuntimeException if the ID provided is invalid
+     * @throws              DataIntegrityViolationException if the ID provided is invalid
      */
     Pair<Long, Locale> getLocalePair(Long id);
     
@@ -62,8 +62,7 @@ public interface LocaleDAO
      * Gets the locale ID for an existing instance or creates a new entity if
      * one doesn't exist.
      * 
-     * @param id            the locale to fetch or <tt>null</tt> to get or create the default
+     * @param locale        the locale to fetch or <tt>null</tt> to get or create the default locale.
-     *                      locale.
      * @return              the locale - never <tt>null</tt>
      */
     Pair<Long, Locale> getOrCreateLocalePair(Locale locale);

source/java/org/alfresco/repo/domain/locale/LocaleEntity.java

@@ -24,13 +24,7 @@
  */
 package org.alfresco.repo.domain.locale;
 
-import java.util.Locale;
+import org.alfresco.util.EqualsHelper;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
-import java.util.concurrent.locks.ReentrantReadWriteLock.ReadLock;
-import java.util.concurrent.locks.ReentrantReadWriteLock.WriteLock;
-
-import org.alfresco.service.cmr.repository.datatype.DefaultTypeConverter;
-import org.springframework.extensions.surf.util.I18NUtil;
 
 
 /**
@@ -49,15 +43,8 @@ public class LocaleEntity
     private Long version;
     private String localeStr;
     
-    private transient ReadLock refReadLock;
-    private transient WriteLock refWriteLock;
-    private transient Locale locale;
-    
     public LocaleEntity()
     {
-        ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
-        refReadLock = lock.readLock();
-        refWriteLock = lock.writeLock();
     }
     
     @Override
@@ -95,16 +82,7 @@ public class LocaleEntity
     }
     public void setLocaleStr(String localeStr)
     {
-        refWriteLock.lock();
+        this.localeStr = localeStr;
-        try
-        {
-            this.localeStr = localeStr;
-            this.locale = null;
-        }
-        finally
-        {
-            refWriteLock.unlock();
-        }
     }
     
     @Override
@@ -123,77 +101,12 @@ public class LocaleEntity
             return false;
         }
         LocaleEntity that = (LocaleEntity) obj;
-        return (this.getLocale().equals(that.getLocale()));
+        return EqualsHelper.nullSafeEquals(this.localeStr, that.localeStr);
     }
     
     @Override
     public int hashCode()
     {
-        return getLocale().hashCode();
+        return localeStr == null ? 0 : localeStr.hashCode();
-    }
-    
-    /**
-     * Lazily constructs a <code>Locale</code> instance referencing this entity
-     */
-    public Locale getLocale()
-    {
-        // The default locale cannot be cached as it depends on the running thread's locale
-        if (localeStr == null || localeStr.equals(DEFAULT_LOCALE_SUBSTITUTE))
-        {
-            return I18NUtil.getLocale();
-        }
-        // first check if it is available
-        refReadLock.lock();
-        try
-        {
-            if (locale != null)
-            {
-                return locale;
-            }
-        }
-        finally
-        {
-            refReadLock.unlock();
-        }
-        // get write lock
-        refWriteLock.lock();
-        try
-        {
-            // double check
-            if (locale == null )
-            {
-                locale = DefaultTypeConverter.INSTANCE.convert(Locale.class, localeStr);
-            }
-            return locale;
-        }
-        finally
-        {
-            refWriteLock.unlock();
-        }
-    }
-    
-    /**
-     * @param locale        the locale to set or <tt>null</tt> to represent the default locale
-     */
-    public void setLocale(Locale locale)
-    {
-        refWriteLock.lock();
-        try
-        {
-            if (locale == null)
-            {
-                this.localeStr = DEFAULT_LOCALE_SUBSTITUTE;
-                this.locale = null;
-            }
-            else
-            {
-                this.localeStr = DefaultTypeConverter.INSTANCE.convert(String.class, locale);
-                this.locale = locale;
-            }
-        }
-        finally
-        {
-            refWriteLock.unlock();
-        }
     }
 }