OPENJPA-305: Add originalValue for Value and modify Configuration equality/hashCode to base on Value equality/hashCode

git-svn-id: https://svn.apache.org/repos/asf/openjpa/trunk@577164 13f79535-47bb-0310-9956-ffa450edef68

openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/ConfigurationImpl.java

@@ -778,8 +778,9 @@ public class ConfigurationImpl
     /////////////
 
     /**
-     * Performs an equality check based on the properties returned from
+     * Performs an equality check based on equality of values.
-     * {@link #toProperties}.
+     * {@link Value#equals(Object) Equality} of Values varies if the Value is
+     * {@link Value#isDynamic() dynamic}.  
      */
     public boolean equals(Object other) {
         if (other == this)
@@ -791,18 +792,32 @@ public class ConfigurationImpl
 
         // compare properties
         ConfigurationImpl conf = (ConfigurationImpl) other;
-        Map p1 = (_props == null) ? toProperties(false) : _props;
+        if (_vals.size() != conf.getValues().length)
-        Map p2 = (conf._props == null) ? conf.toProperties(false) : conf._props;
+        	return false;
-        return excludeDynamic(p1).equals(excludeDynamic(p2));
+        Iterator values = _vals.iterator();
+        while (values.hasNext()) {
+        	Value v = (Value)values.next();
+        	Value thatV = conf.getValue(v.getProperty());
+        	if (!v.equals(thatV)) {
+        		return false;
+        	}
+        }
+        return true;
     }
 
     /**
-     * Computes hash code based on the properties returned from
+     * Computes hash code based on the hashCodes of the values.
-     * {@link #toProperties}.
+     * {@link Value#hashCode() HashCode} of a Value varies if the Value is
+     * {@link Value#isDynamic() dynamic}.  
      */
     public int hashCode() {
-    	Map copy = (_props == null) ? toProperties(false) : _props;
+        Iterator values = _vals.iterator();
-    	return excludeDynamic(copy).hashCode();
+        int hash = 0;
+        while (values.hasNext()) {
+        	Value v = (Value)values.next();
+        	hash += v.hashCode();
+        }
+        return hash;
     }
 
     /**
@@ -978,16 +993,4 @@ public class ConfigurationImpl
         addValue(val);
         return val;
     }
-    
-    Map excludeDynamic(Map map) {
-    	if (map == null)
-    		return null;
-    	Map copy = new HashMap(map);
-    	Value[] values = getValues();
-    	for (int i=0; i<values.length; i++) {
-    		if (values[i].isDynamic())
-    			Configurations.removeProperty(values[i].getProperty(), copy);
-    	}
-    	return copy;
-    }
 }

openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/PluginListValue.java

@@ -110,7 +110,8 @@ public class PluginListValue extends ObjectValue {
         for (int i = 0; i < _names.length; i++) {
             if (i > 0)
                 buf.append(", ");
-            buf.append(Configurations.getPlugin(alias(_names[i]), _props[i]));
+            buf.append(Configurations.getPlugin(alias(_names[i]), 
+            		(i<_props.length) ? _props[i] : null));
         }
         if (buf.length() == 0)
             return null;

openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/Value.java

@@ -29,6 +29,7 @@ import org.apache.openjpa.lib.util.ParseException;
  * A configuration value.
  *
  * @author Marc Prud'hommeaux
+ * @author Pinaki Poddar (added dynamic Value support)
  */
 public abstract class Value implements Cloneable {
 
@@ -44,6 +45,7 @@ public abstract class Value implements Cloneable {
     private boolean aliasListComprehensive = false;
     private Class scope = null;
     private boolean isDynamic = false;
+    private String originalValue = null;
 
     /**
      * Default constructor.
@@ -278,12 +280,22 @@ public abstract class Value implements Cloneable {
      * empty and a default is defined, the default is used. If the given
      * string(or default) is an alias key, it will be converted to the
      * corresponding value internally.
+     * <br>
+     * If this Value is being set to a non-default value for the first time
+     * (as designated by <code>originalString</code> being null), then the
+     * value is remembered as <em>original</em>. This original value is used
+     * for equality and hashCode computation if this Value is
+     * {@link #isDynamic() dynamic}. 
+     *
      */
     public void setString(String val) {
     	assertChangeable();
         String str = unalias(val);
         try {
             setInternalString(str);
+            if (originalValue == null && val != null && !isDefault(val)) {
+            	originalValue = getString();
+            }
         } catch (ParseException pe) {
             throw pe;
         } catch (RuntimeException re) {
@@ -293,6 +305,13 @@ public abstract class Value implements Cloneable {
 
     /**
      * Set this value as an object.
+     * <br>
+     * If this Value is being set to a non-default value for the first time
+     * (as designated by <code>originalString</code> being null), then the
+     * value is remembered as <em>original</em>. This original value is used
+     * for equality and hashCode computation if this Value is
+     * {@link #isDynamic() dynamic}. 
+     * 
      */
     public void setObject(Object obj) {
         // if setting to null set as string to get defaults into play
@@ -301,6 +320,9 @@ public abstract class Value implements Cloneable {
         else {
             try {
                 setInternalObject(obj);
+                if (originalValue == null && obj != null && !isDefault(obj)) {
+                	originalValue = getString();
+                }
             } catch (ParseException pe) {
                 throw pe;
             } catch (RuntimeException re) {
@@ -309,6 +331,22 @@ public abstract class Value implements Cloneable {
         }
     }
     
+    /**
+     * Gets the original value. Original value denotes the Stringified form of 
+     * this Value, from which it has been set, if ever. If this Value has never 
+     * been set to a non-default value, then returns the default value, which 
+     * itself can be null. 
+     * 
+     * @since 1.1.0
+     */
+    public String getOriginalValue() {
+    	return (originalValue == null) ? getDefault() : originalValue;
+    }
+    
+    boolean isDefault(Object val) {
+    	return val != null && val.toString().equals(getDefault());
+    }
+
     /**
      * Returns the type of the property that this Value represents.
      */
@@ -374,6 +412,7 @@ public abstract class Value implements Cloneable {
      * Sets if this receiver can be mutated even when the configuration it 
      * belongs to has been {@link Configuration#isReadOnly() frozen}.
      *  
+     * @since 1.1.0
      */
     public void setDynamic(boolean flag) {
     	isDynamic = flag;
@@ -383,18 +422,31 @@ public abstract class Value implements Cloneable {
      * Affirms if this receiver can be mutated even when the configuration it 
      * belongs to has been {@link Configuration#isReadOnly() frozen}.
      *  
+     * @since 1.1.0
      */
     public boolean isDynamic() {
     	return isDynamic; 
     }
 
+    /**
+     * Use {@link #getOriginalValue() original value} instead of 
+     * {@link #getString() current value} because they are one and the same 
+     * for non-dynamic Values and ensures that modifying dynamic Values do not
+     * impact equality or hashCode contract.   
+     */
     public int hashCode() {
-        String str = getString();
+        String str = (isDynamic()) ? getOriginalValue() : getString();
         int strHash = (str == null) ? 0 : str.hashCode();
         int propHash = (prop == null) ? 0 : prop.hashCode();
         return strHash ^ propHash;
     }
 
+    /**
+     * Use {@link #getOriginalValue() original value} instead of 
+     * {@link #getString() current value} because they are one and the same 
+     * for non-dynamic Values and ensures that modifying dynamic Values do not
+     * impact equality or hashCode contract.   
+     */
     public boolean equals(Object other) {
         if (other == this)
             return true;
@@ -402,8 +454,11 @@ public abstract class Value implements Cloneable {
             return false;
 
         Value o = (Value) other;
-        return StringUtils.equals(prop, o.getProperty())
+        String thisStr = (isDynamic()) ? getOriginalValue() : getString();
-            && StringUtils.equals(getString(), o.getString());
+        String thatStr = (isDynamic()) ? o.getOriginalValue() : o.getString();
+        return (isDynamic() == o.isDynamic())
+            && StringUtils.equals(prop, o.getProperty())
+            && StringUtils.equals(thisStr, thatStr);
     }
 
     public Object clone() {