MID-1951 in progress.

Evolveum · Oct 14, 2014 · 003b1ad · 003b1ad
1 parent 957bf85
commit 003b1ad
Show file tree

Hide file tree

Showing 10 changed files with 187 additions and 143 deletions.
diff --git a/infra/prism/src/main/java/com/evolveum/midpoint/prism/ConsistencyCheckScope.java b/infra/prism/src/main/java/com/evolveum/midpoint/prism/ConsistencyCheckScope.java
@@ -0,0 +1,16 @@
+package com.evolveum.midpoint.prism;
+
+/**
+ * Determines the scope of consistency checks.
+ * (Originally this was a boolean, but there are many methods with N boolean arguments, so it was
+ * too easy to mix them up.)
+ *
+ * @author mederly
+ */
+public enum ConsistencyCheckScope {
+    THOROUGH, MANDATORY_CHECKS_ONLY;
+
+    public boolean isThorough() {
+        return this == THOROUGH;
+    }
+}
diff --git a/infra/prism/src/main/java/com/evolveum/midpoint/prism/Item.java b/infra/prism/src/main/java/com/evolveum/midpoint/prism/Item.java
@@ -25,8 +25,6 @@
 import com.evolveum.midpoint.util.exception.SchemaException;
 import com.evolveum.midpoint.util.exception.SystemException;
 
-import org.w3c.dom.Element;
-
 import javax.xml.namespace.QName;
 
 import java.io.Serializable;
@@ -619,19 +617,23 @@ public static <T extends Item> T createNewDefinitionlessItem(QName name, Class<T
     	return item;
     }
 
-    public void checkConsistence(boolean requireDefinitions) {
-    	checkConsistenceInternal(this, requireDefinitions, false);
+    public void checkConsistence(boolean requireDefinitions, ConsistencyCheckScope scope) {
+    	checkConsistenceInternal(this, requireDefinitions, false, scope);
     }
 
-    public void checkConsistence(boolean requireDefinitions, boolean prohibitRaw) {
-    	checkConsistenceInternal(this, requireDefinitions, prohibitRaw);
+    public void checkConsistence(boolean requireDefinitions, boolean prohibitRaw, ConsistencyCheckScope scope) {
+    	checkConsistenceInternal(this, requireDefinitions, prohibitRaw, scope);
     }
 
     public void checkConsistence() {
-    	checkConsistenceInternal(this, false, false);
+    	checkConsistenceInternal(this, false, false, ConsistencyCheckScope.THOROUGH);
     }
-
-    public void checkConsistenceInternal(Itemable rootItem, boolean requireDefinitions, boolean prohibitRaw) {
+
+    public void checkConsistenceMandatory() {
+        checkConsistenceInternal(this, false, false, ConsistencyCheckScope.MANDATORY_CHECKS_ONLY);
+    }
+
+    public void checkConsistenceInternal(Itemable rootItem, boolean requireDefinitions, boolean prohibitRaw, ConsistencyCheckScope scope) {
     	ItemPath path = getPath();
     	if (elementName == null) {
     		throw new IllegalStateException("Item "+this+" has no name ("+path+" in "+rootItem+")");
@@ -657,7 +659,7 @@ public void checkConsistenceInternal(Itemable rootItem, boolean requireDefinitio
     				throw new IllegalStateException("Wrong parent for value "+val+" in item "+this+" ("+path+" in "+rootItem+"), "+
     						"bad parent: " + val.getParent());
     			}
-    			val.checkConsistenceInternal(rootItem, requireDefinitions, prohibitRaw);
+    			val.checkConsistenceInternal(rootItem, requireDefinitions, prohibitRaw, scope);
     		}
     	}
     }

diff --git a/infra/prism/src/main/java/com/evolveum/midpoint/prism/PrismContainer.java b/infra/prism/src/main/java/com/evolveum/midpoint/prism/PrismContainer.java
@@ -615,17 +615,34 @@ protected void checkDefinition(ItemDefinition def) {
 
 	@Override
 	public void checkConsistenceInternal(Itemable rootItem, boolean requireDefinitions,
-			boolean prohibitRaw) {
-		// Containers that are from run-time schema cannot have compile-time class.
-		if (getDefinition() != null && !getDefinition().isRuntimeSchema()) {
-			if (getCompileTimeClass() == null) {
-				throw new IllegalStateException("No compile-time class in "+this+" ("+getPath()+" in "+rootItem+")");
-			}
-		}
-		super.checkConsistenceInternal(rootItem, requireDefinitions, prohibitRaw);
+			boolean prohibitRaw, ConsistencyCheckScope scope) {
+        checkIds();
+        if (scope.isThorough()) {
+            // Containers that are from run-time schema cannot have compile-time class.
+            if (getDefinition() != null && !getDefinition().isRuntimeSchema()) {
+                if (getCompileTimeClass() == null) {
+                    throw new IllegalStateException("No compile-time class in "+this+" ("+getPath()+" in "+rootItem+")");
+                }
+            }
+        }
+		super.checkConsistenceInternal(rootItem, requireDefinitions, prohibitRaw, scope);
 	}
 
-	@Override
+    private void checkIds() {
+        Set<Long> oidsUsed = new HashSet<>();
+        for (PrismContainerValue value : getValues()) {
+            Long id = value.getId();
+            if (id != null) {
+                if (oidsUsed.contains(id)) {
+                    throw new IllegalArgumentException("There are more container values with the id of " + id + " in " + getElementName());
+                } else {
+                    oidsUsed.add(id);
+                }
+            }
+        }
+    }
+
+    @Override
 	public void assertDefinitions(boolean tolarateRaw, String sourceDescription) throws SchemaException {
 		super.assertDefinitions(tolarateRaw, sourceDescription);
 		for (PrismContainerValue<V> val: getValues()) {

diff --git a/infra/prism/src/main/java/com/evolveum/midpoint/prism/PrismContainerValue.java b/infra/prism/src/main/java/com/evolveum/midpoint/prism/PrismContainerValue.java
@@ -1256,31 +1256,35 @@ public void normalize() {
 	}
 
 	@Override
-	public void checkConsistenceInternal(Itemable rootItem, boolean requireDefinitions, boolean prohibitRaw) {
-		ItemPath myPath = getPath();
-		if (prohibitRaw && isRaw()) {
-			throw new IllegalStateException("Raw elements in container value "+this+" ("+myPath+" in "+rootItem+")");
-		}
-		if (items == null && !isRaw()) {
-			// This is normal empty container, isn't it?
-//			throw new IllegalStateException("Neither items nor raw elements specified in container value "+this+" ("+myPath+" in "+rootItem+")");
-		}
-		if (items != null && isRaw()) {
-			throw new IllegalStateException("Both items and raw elements specified in container value "+this+" ("+myPath+" in "+rootItem+")");
-		}
+	public void checkConsistenceInternal(Itemable rootItem, boolean requireDefinitions, boolean prohibitRaw, ConsistencyCheckScope scope) {
+        ItemPath myPath = getPath();
+        if (scope.isThorough()) {
+            if (prohibitRaw && isRaw()) {
+                throw new IllegalStateException("Raw elements in container value "+this+" ("+myPath+" in "+rootItem+")");
+            }
+            if (items == null && !isRaw()) {
+                // This is normal empty container, isn't it?
+//			    throw new IllegalStateException("Neither items nor raw elements specified in container value "+this+" ("+myPath+" in "+rootItem+")");
+            }
+            if (items != null && isRaw()) {
+                throw new IllegalStateException("Both items and raw elements specified in container value "+this+" ("+myPath+" in "+rootItem+")");
+            }
+        }
 		if (items != null) {
 			for (Item<?> item: items) {
-				if (item == null) {
-					throw new IllegalStateException("Null item in container value "+this+" ("+myPath+" in "+rootItem+")");
-				}
-				if (item.getParent() == null) {
-					throw new IllegalStateException("No parent for item "+item+" in container value "+this+" ("+myPath+" in "+rootItem+")");
-				}
-				if (item.getParent() != this) {
-					throw new IllegalStateException("Wrong parent for item "+item+" in container value "+this+" ("+myPath+" in "+rootItem+"), " +
-							"bad parent: "+ item.getParent());
-				}
-				item.checkConsistenceInternal(rootItem, requireDefinitions, prohibitRaw);
+                if (scope.isThorough()) {
+                    if (item == null) {
+                        throw new IllegalStateException("Null item in container value "+this+" ("+myPath+" in "+rootItem+")");
+                    }
+                    if (item.getParent() == null) {
+                        throw new IllegalStateException("No parent for item "+item+" in container value "+this+" ("+myPath+" in "+rootItem+")");
+                    }
+                    if (item.getParent() != this) {
+                        throw new IllegalStateException("Wrong parent for item "+item+" in container value "+this+" ("+myPath+" in "+rootItem+"), " +
+                                "bad parent: "+ item.getParent());
+                    }
+                }
+				item.checkConsistenceInternal(rootItem, requireDefinitions, prohibitRaw, scope);
 			}
 		}
 	}

diff --git a/infra/prism/src/main/java/com/evolveum/midpoint/prism/PrismValue.java b/infra/prism/src/main/java/com/evolveum/midpoint/prism/PrismValue.java
@@ -163,7 +163,7 @@ public void accept(Visitor visitor, ItemPath path, boolean recursive) {
 		}
 	}
 
-	public abstract void checkConsistenceInternal(Itemable rootItem, boolean requireDefinitions, boolean prohibitRaw);
+    public abstract void checkConsistenceInternal(Itemable rootItem, boolean requireDefinitions, boolean prohibitRaw, ConsistencyCheckScope scope);
 
 	/**
 	 * Returns true if this and other value represent the same value.

diff --git a/infra/prism/src/main/java/com/evolveum/midpoint/prism/delta/ContainerDelta.java b/infra/prism/src/main/java/com/evolveum/midpoint/prism/delta/ContainerDelta.java
@@ -16,25 +16,11 @@
 
 package com.evolveum.midpoint.prism.delta;
 
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Iterator;
-import java.util.List;
+import java.util.*;
 
 import javax.xml.namespace.QName;
 
-import com.evolveum.midpoint.prism.Containerable;
-import com.evolveum.midpoint.prism.Item;
-import com.evolveum.midpoint.prism.ItemDefinition;
-import com.evolveum.midpoint.prism.Objectable;
-import com.evolveum.midpoint.prism.PrismContainer;
-import com.evolveum.midpoint.prism.PrismContainerDefinition;
-import com.evolveum.midpoint.prism.PrismContainerValue;
-import com.evolveum.midpoint.prism.PrismContainerable;
-import com.evolveum.midpoint.prism.PrismContext;
-import com.evolveum.midpoint.prism.PrismObject;
-import com.evolveum.midpoint.prism.PrismObjectDefinition;
-import com.evolveum.midpoint.prism.PrismValue;
+import com.evolveum.midpoint.prism.*;
 import com.evolveum.midpoint.prism.path.ItemPath;
 import com.evolveum.midpoint.util.MiscUtil;
 import com.evolveum.midpoint.util.exception.SchemaException;
@@ -216,7 +202,30 @@ public <O extends Objectable> void expand(PrismObject<O> object) throws SchemaEx
 		}
 	}
 
-	@Override
+    @Override
+    public void checkConsistence(boolean requireDefinition, boolean prohibitRaw, ConsistencyCheckScope scope) {
+        super.checkConsistence(requireDefinition, prohibitRaw, scope);
+        checkDuplicateId(valuesToAdd);
+    }
+
+    private void checkDuplicateId(Collection<PrismContainerValue<V>> valuesToAdd) {
+        if (valuesToAdd == null || valuesToAdd.isEmpty()) {
+            return;
+        }
+        Set<Long> idsToAdd = new HashSet<>();
+        for (PrismContainerValue<V> valueToAdd : valuesToAdd) {
+            Long id = valueToAdd.getId();
+            if (id != null) {
+                if (idsToAdd.contains(id)) {
+                    throw new IllegalArgumentException("Trying to add prism container value with id " + id + " multiple times");
+                } else {
+                    idsToAdd.add(id);
+                }
+            }
+        }
+    }
+
+    @Override
 	public ContainerDelta<V> clone() {
 		ContainerDelta<V> clone = new ContainerDelta<V>(getElementName(), getDefinition(), getPrismContext());
 		copyValues(clone);

diff --git a/infra/prism/src/main/java/com/evolveum/midpoint/prism/delta/ItemDelta.java b/infra/prism/src/main/java/com/evolveum/midpoint/prism/delta/ItemDelta.java
@@ -23,21 +23,7 @@
 
 import javax.xml.namespace.QName;
 
-import com.evolveum.midpoint.prism.Containerable;
-import com.evolveum.midpoint.prism.Item;
-import com.evolveum.midpoint.prism.ItemDefinition;
-import com.evolveum.midpoint.prism.Itemable;
-import com.evolveum.midpoint.prism.Objectable;
-import com.evolveum.midpoint.prism.PathVisitable;
-import com.evolveum.midpoint.prism.PrismContainer;
-import com.evolveum.midpoint.prism.PrismContainerValue;
-import com.evolveum.midpoint.prism.PrismContext;
-import com.evolveum.midpoint.prism.PrismObject;
-import com.evolveum.midpoint.prism.PrismObjectDefinition;
-import com.evolveum.midpoint.prism.PrismProperty;
-import com.evolveum.midpoint.prism.PrismValue;
-import com.evolveum.midpoint.prism.Visitable;
-import com.evolveum.midpoint.prism.Visitor;
+import com.evolveum.midpoint.prism.*;
 import com.evolveum.midpoint.prism.parser.XPathHolder;
 import com.evolveum.midpoint.prism.path.IdItemPathSegment;
 import com.evolveum.midpoint.prism.path.ItemPath;
@@ -49,9 +35,6 @@
 import com.evolveum.midpoint.util.MiscUtil;
 import com.evolveum.midpoint.util.PrettyPrinter;
 import com.evolveum.midpoint.util.exception.SchemaException;
-import com.evolveum.prism.xml.ns._public.types_3.ItemPathType;
-
-import org.apache.commons.lang.Validate;
 
 /**
  * @author Radovan Semancik
@@ -782,37 +765,41 @@ public void validate(String contextDescription) throws SchemaException {
     	}
     }
 
-	public static void checkConsistence(Collection<? extends ItemDelta> deltas) {
-		checkConsistence(deltas, false, false);
+	public static void checkConsistence(Collection<? extends ItemDelta> deltas, ConsistencyCheckScope scope) {
+		checkConsistence(deltas, false, false, scope);
 	}
 
-	public static void checkConsistence(Collection<? extends ItemDelta> deltas, boolean requireDefinition, boolean prohibitRaw) {
+	public static void checkConsistence(Collection<? extends ItemDelta> deltas, boolean requireDefinition, boolean prohibitRaw, ConsistencyCheckScope scope) {
 		for (ItemDelta<?> delta : deltas) {
-			delta.checkConsistence(requireDefinition, prohibitRaw);
+			delta.checkConsistence(requireDefinition, prohibitRaw, scope);
 		}
 	}
-
-	public void checkConsistence() {
-		checkConsistence(false, false);
+
+    public void checkConsistence() {
+        checkConsistence(ConsistencyCheckScope.THOROUGH);
+    }
+
+	public void checkConsistence(ConsistencyCheckScope scope) {
+		checkConsistence(false, false, scope);
 	}
 
-	public void checkConsistence(boolean requireDefinition, boolean prohibitRaw) {
-		if (parentPath == null) {
+	public void checkConsistence(boolean requireDefinition, boolean prohibitRaw, ConsistencyCheckScope scope) {
+		if (scope.isThorough() && parentPath == null) {
 			throw new IllegalStateException("Null parent path in " + this);
 		}
-		if (requireDefinition && definition == null) {
+		if (scope.isThorough() && requireDefinition && definition == null) {
 			throw new IllegalStateException("Null definition in "+this);
 		}
-		if (valuesToReplace != null && (valuesToAdd != null || valuesToDelete != null)) {
+		if (scope.isThorough() && valuesToReplace != null && (valuesToAdd != null || valuesToDelete != null)) {
 			throw new IllegalStateException(
 					"The delta cannot be both 'replace' and 'add/delete' at the same time");
 		}
-		assertSetConsistence(valuesToReplace, "replace", requireDefinition, prohibitRaw);
-		assertSetConsistence(valuesToAdd, "add", requireDefinition, prohibitRaw);
-		assertSetConsistence(valuesToDelete, "delete", requireDefinition, prohibitRaw);
+		assertSetConsistence(valuesToReplace, "replace", requireDefinition, prohibitRaw, scope);
+		assertSetConsistence(valuesToAdd, "add", requireDefinition, prohibitRaw, scope);
+		assertSetConsistence(valuesToDelete, "delete", requireDefinition, prohibitRaw, scope);
 	}
 
-	private void assertSetConsistence(Collection<V> values, String type, boolean requireDefinitions, boolean prohibitRaw) {
+	private void assertSetConsistence(Collection<V> values, String type, boolean requireDefinitions, boolean prohibitRaw, ConsistencyCheckScope scope) {
 		if (values == null) {
 			return;
 		}
@@ -822,13 +809,15 @@ private void assertSetConsistence(Collection<V> values, String type, boolean req
 		// IllegalStateException("The "+type+" values set in "+this+" is not-null but it is empty");
 		// }
 		for (V val : values) {
-			if (val == null) {
-				throw new IllegalStateException("Null value in the " + type + " values set in " + this);
-			}
-			if (val.getParent() != this) {
-				throw new IllegalStateException("Wrong parent for " + val + " in " + type + " values set in " + this + ": " + val.getParent());
-			}
-			val.checkConsistenceInternal(this, requireDefinitions, prohibitRaw);
+            if (scope.isThorough()) {
+                if (val == null) {
+                    throw new IllegalStateException("Null value in the " + type + " values set in " + this);
+                }
+                if (val.getParent() != this) {
+                    throw new IllegalStateException("Wrong parent for " + val + " in " + type + " values set in " + this + ": " + val.getParent());
+                }
+            }
+            val.checkConsistenceInternal(this, requireDefinitions, prohibitRaw, scope);
 		}
 	}