CachedModel: use unmodifiable wrapper on Type.all (fixes #189)

Author: agarciadom

plugins/org.eclipse.epsilon.eol.engine/src/org/eclipse/epsilon/eol/models/CachedModel.java

@@ -10,12 +10,23 @@
  ******************************************************************************/
 package org.eclipse.epsilon.eol.models;
 
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Deque;
+import java.util.List;
+import java.util.Map;
+import java.util.NavigableSet;
+import java.util.Set;
+import java.util.SortedSet;
+
 import org.eclipse.epsilon.common.concurrent.ConcurrencyUtils;
 import org.eclipse.epsilon.common.util.Multimap;
 import org.eclipse.epsilon.common.util.StringProperties;
 import org.eclipse.epsilon.eol.exceptions.EolRuntimeException;
-import org.eclipse.epsilon.eol.exceptions.models.*;
+import org.eclipse.epsilon.eol.exceptions.models.EolModelElementTypeNotFoundException;
+import org.eclipse.epsilon.eol.exceptions.models.EolModelLoadingException;
+import org.eclipse.epsilon.eol.exceptions.models.EolNotInstantiableModelElementTypeException;
 
 /**
  * A model that performs memoization of allContents, getAllOfType
@@ -155,7 +166,22 @@ protected Collection<ModelElementType> wrap(Collection<ModelElementType> content
 		}
 		return result;
 	}
-	
+
+	@SuppressWarnings({ "unchecked", "rawtypes" })
+	protected <T> Collection<T> wrapUnmodifiable(Collection<T> original) {
+		if (original instanceof List) {
+			return Collections.unmodifiableList((List) original);
+		} else if (original instanceof NavigableSet) {
+			return Collections.unmodifiableNavigableSet((NavigableSet) original);
+		} else if (original instanceof SortedSet) {
+			return Collections.unmodifiableSortedSet((SortedSet) original);
+		} else if (original instanceof Set) {
+			return Collections.unmodifiableSet((Set) original);
+		} else {
+			return Collections.unmodifiableCollection(original);
+		}
+	}
+
 	protected void addToCache(String type, ModelElementType instance) throws EolModelElementTypeNotFoundException {
 		assert cachingEnabled;
 
@@ -214,11 +240,11 @@ public Collection<ModelElementType> allContents() {
 				if (allContentsCache == null) {
 					allContentsCache = wrap(allContentsFromModel());
 					if (allContentsCache == null) {
-						return wrap(new ArrayList<>(0));
+						return wrapUnmodifiable(wrap(new ArrayList<>(0)));
 					}
 				}
 			}
-			return allContentsCache;
+			return wrapUnmodifiable(allContentsCache);
 		}
 		else return wrap(allContentsFromModel());
 	}
@@ -262,7 +288,7 @@ else if (values == null) {
 			);
 		}
 
-		return values;
+		return wrapUnmodifiable(values);
 	}
 
 	@Override

tests/org.eclipse.epsilon.eol.engine.test.unit/src/org/eclipse/epsilon/eol/models/CachedModelTests.java

@@ -9,17 +9,20 @@
  ******************************************************************************/
 package org.eclipse.epsilon.eol.models;
 
-import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
+
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+
 import org.eclipse.epsilon.eol.exceptions.EolRuntimeException;
 import org.eclipse.epsilon.eol.exceptions.models.EolEnumerationValueNotFoundException;
 import org.eclipse.epsilon.eol.exceptions.models.EolModelElementTypeNotFoundException;
@@ -58,6 +61,14 @@ public void shouldNotComputeAllContentsOnSecondCall() throws Exception {
 
 			verify(model, times(1)).allContentsFromModel();
 		}
+
+		@SuppressWarnings("resource")
+		@Test
+		public void shouldNotAllowChanges() throws Exception {
+			final SimpleCachedModel model = new SimpleCachedModel();
+			assertThrows(UnsupportedOperationException.class,
+				() -> model.allContents().add("x"));
+		}
 	}
 
 	public static class AllOfTypeCachingTests {
@@ -113,6 +124,14 @@ public void shouldNotComputeAllOfTypeIfTypeIsEquivalentToACachedType() throws Ex
 
 			verify(model, never()).getAllOfTypeFromModel("org.example.Widget");
 		}
+
+		@SuppressWarnings("resource")
+		@Test
+		public void shouldNotAllowChanges() throws Exception {
+			final SimpleCachedModel model = new SimpleCachedModel();
+			assertThrows(UnsupportedOperationException.class,
+				() -> model.getAllOfType("Widget").add("x"));
+		}
 	}
 
 	public static class AllOfKindCachingTests {
@@ -168,6 +187,14 @@ public void shouldNotComputeAllOfKindIfKindIsEquivalentToACachedKind() throws Ex
 
 			verify(model, never()).getAllOfKindFromModel("org.example.Widget");
 		}
+
+		@SuppressWarnings("resource")
+		@Test
+		public void shouldNotAllowChanges() throws Exception {
+			final SimpleCachedModel model = new SimpleCachedModel();
+			assertThrows(UnsupportedOperationException.class,
+				() -> model.getAllOfKind("Widget").add("x"));
+		}
 	}
 
 	// TODO create / delete before caching has occured