fix issues #62 & #61: improve resource unloading & attachment

atlanmod · Sep 20, 2017 · c253e64 · c253e64
1 parent f36ea34
commit c253e64
Show file tree

Hide file tree

Showing 12 changed files with 82 additions and 74 deletions.
diff --git a/...ks/core/src/main/java/fr/inria/atlanmod/neoemf/benchmarks/adapter/AbstractNeoAdapter.java b/...ks/core/src/main/java/fr/inria/atlanmod/neoemf/benchmarks/adapter/AbstractNeoAdapter.java
@@ -16,7 +16,6 @@
 import fr.inria.atlanmod.neoemf.data.mapping.DataMapper;
 import fr.inria.atlanmod.neoemf.data.store.StoreFactory;
 import fr.inria.atlanmod.neoemf.option.PersistenceOptions;
-import fr.inria.atlanmod.neoemf.resource.PersistentResource;
 import fr.inria.atlanmod.neoemf.util.UriBuilder;
 
 import org.eclipse.emf.common.util.URI;
@@ -93,12 +92,7 @@ public Resource load(File file, PersistenceOptions options) throws IOException {
     @Override
     public void unload(Resource resource) {
         if (resource.isLoaded()) {
-            if (PersistentResource.class.isInstance(resource)) {
-                PersistentResource.class.cast(resource).close();
-            }
-            else {
-                resource.unload();
-            }
+            resource.unload();
         }
     }
 }
diff --git a/...s/core/src/main/java/fr/inria/atlanmod/neoemf/benchmarks/resource/DirectStoreCreator.java b/...s/core/src/main/java/fr/inria/atlanmod/neoemf/benchmarks/resource/DirectStoreCreator.java
@@ -32,7 +32,7 @@ public File getOrCreateStore(File file, PersistenceOptions options, Adapter.Inte
 
         adapter.initAndGetEPackage();
 
-        Log.info("Importing resource content...");
+        Log.info("Migrating resource content...");
 
         try (DataMapper mapper = adapter.createMapper(targetFile, options)) {
             Migrator.fromXmi(file)

diff --git a/...core/src/main/java/fr/inria/atlanmod/neoemf/benchmarks/resource/StandardStoreCreator.java b/...core/src/main/java/fr/inria/atlanmod/neoemf/benchmarks/resource/StandardStoreCreator.java
@@ -54,21 +54,17 @@ public File getOrCreateStore(File file, PersistenceOptions options, Adapter.Inte
         }
         sourceResource.load(loadOpts);
 
-        Log.info("Copying resource content...");
-
-        Collection<EObject> targetRoot = EcoreUtil.copyAll(sourceResource.getContents());
-        sourceResource.unload();
-
         Log.info("Migrating resource content...");
 
         Resource targetResource = adapter.createResource(targetFile, resourceSet);
         adapter.save(targetResource, options);
 
-        targetResource.getContents().addAll(targetRoot);
+        targetResource.getContents().addAll(sourceResource.getContents());
 
         Log.info("Saving resource to: {0}", targetResource.getURI());
         adapter.save(targetResource, options);
 
+        sourceResource.unload();
         adapter.unload(targetResource);
 
         return targetFile;

diff --git a/neoemf-core/src/main/java/fr/inria/atlanmod/neoemf/core/DefaultPersistentEObject.java b/neoemf-core/src/main/java/fr/inria/atlanmod/neoemf/core/DefaultPersistentEObject.java
@@ -62,11 +62,17 @@
 @ParametersAreNonnullByDefault
 public class DefaultPersistentEObject extends MinimalEStoreEObjectImpl implements PersistentEObject {
 
+    /**
+     * The {@link StoreAdapter} where this object is stored.
+     */
+    @Nonnull
+    private final LazyObject<StoreAdapter> lazyStore = LazyObject.with(() -> getOrCreateStore(resource()));
+
     /**
      * The cached container of this object.
      */
     @Nonnull
-    private final LazyReference<InternalEObject> lazyContainer = LazyReference.soft(super::eInternalContainer);
+    private final LazyReference<PersistentEObject> lazyContainer = LazyReference.soft(() -> eStore().getContainer(this));
 
     /**
      * The identifier of this object.
@@ -81,10 +87,14 @@ public class DefaultPersistentEObject extends MinimalEStoreEObjectImpl implement
     private Resource.Internal resource;
 
     /**
-     * The {@link StoreAdapter} where this object is stored.
+     * {@code true} if this object is being attached to a resource. This avoids an infinite loop when copying a fully
+     * transient backend to a persistent one.
+     * <p>
+     * When {@code true}, all calls to {@link #resource(Resource.Internal)} will be rejected.
+     *
+     * @see #resource(Resource.Internal)
      */
-    @Nonnull
-    private final LazyObject<StoreAdapter> lazyStore = LazyObject.with(() -> getOrCreateStore(resource()));
+    private boolean locked;
 
     /**
      * Constructs a new {@code DefaultPersistentEObject} with an undefined {@link Id}.
@@ -129,22 +139,23 @@ public Resource.Internal resource() {
 
     @Override
     public void resource(@Nullable Resource.Internal newResource) {
-        StoreAdapter newStore = null;
-
-        if (PersistentResource.class.isInstance(newResource)) {
-            // The resource store may have been changed (persistent <-> transient)
-            newStore = PersistentResource.class.cast(newResource).eStore();
-        }
-        else if (resource != newResource) {
-            newStore = getOrCreateStore(newResource);
+        if (locked || resource == newResource) {
+            return;
         }
 
+        locked = true;
         resource = newResource;
 
-        // The store may have been modified
-        if (nonNull(newStore)) {
-            eStore(newStore);
+        // Refresh the container if necessary
+        if (lazyStore.isLoaded()) {
+            lazyContainer.get();
         }
+
+        // Define and copy the store if necessary
+        StoreAdapter newStore = getOrCreateStore(newResource);
+        eStore(newStore);
+
+        locked = false;
     }
 
     @Nullable
@@ -254,7 +265,7 @@ public void dynamicUnset(int dynamicFeatureId) {
     }
 
     @Override
-    public InternalEObject eInternalContainer() {
+    public PersistentEObject eInternalContainer() {
         return lazyContainer.get();
     }
 
@@ -270,16 +281,22 @@ public InternalEObject eInternalContainer() {
      */
     @Nonnull
     private StoreAdapter getOrCreateStore(@Nullable Resource.Internal resource) {
-        if (!lazyStore.isLoaded() || PersistentResource.class.isInstance(lazyStore.get().resource())) {
-            TransientBackend backend = new BoundTransientBackend(id());
-            Store baseStore = StoreFactory.getInstance().createStore(backend, CommonOptions.noOption());
-            return new TransientStoreAdapter(baseStore, resource);
+        // Use the store of the resource
+        if (PersistentResource.class.isInstance(resource)) {
+            return PersistentResource.class.cast(resource).eStore();
         }
-        else {
+        // Adapt the current store
+        else if (lazyStore.isLoaded()) {
             StoreAdapter currentStore = lazyStore.get();
             currentStore.resource(resource);
             return currentStore;
         }
+        // Create a new transient store
+        else {
+            TransientBackend backend = new BoundTransientBackend(id());
+            Store baseStore = StoreFactory.getInstance().createStore(backend, CommonOptions.noOption());
+            return new TransientStoreAdapter(baseStore, resource);
+        }
     }
 
     /**

diff --git a/neoemf-core/src/main/java/fr/inria/atlanmod/neoemf/core/PersistentEObject.java b/neoemf-core/src/main/java/fr/inria/atlanmod/neoemf/core/PersistentEObject.java
@@ -72,4 +72,7 @@ static PersistentEObject from(Object object) {
      * @param newResource the containing resource
      */
     void resource(@Nullable Resource.Internal newResource);
+
+    @Override
+    PersistentEObject eInternalContainer();
 }
diff --git a/neoemf-core/src/main/java/fr/inria/atlanmod/neoemf/core/internal/ContentsCopier.java b/neoemf-core/src/main/java/fr/inria/atlanmod/neoemf/core/internal/ContentsCopier.java
@@ -40,33 +40,33 @@ public ContentsCopier(PersistentEObject object) {
 
     @Override
     public void copy(StoreAdapter source, StoreAdapter target) {
-        PersistentEObject container = source.getContainer(object);
+        PersistentEObject container = object.eInternalContainer();
         if (nonNull(container)) {
             target.updateContainment(object, source.getContainingFeature(object), container);
         }
         else {
             target.removeContainment(object);
         }
 
-        object.eClass().getEAllStructuralFeatures().forEach(feature -> {
-            if (!feature.isMany()) {
-                Object value = source.get(object, feature, InternalEObject.EStore.NO_INDEX);
+        object.eClass().getEAllStructuralFeatures().forEach(f -> {
+            if (!f.isMany()) {
+                Object value = source.get(object, f, InternalEObject.EStore.NO_INDEX);
                 if (nonNull(value)) {
-                    if (requireAttachment(feature)) {
+                    if (requireAttachment(f)) {
                         value = attach(value);
                     }
 
-                    target.set(object, feature, InternalEObject.EStore.NO_INDEX, value);
+                    target.set(object, f, InternalEObject.EStore.NO_INDEX, value);
                 }
             }
             else {
-                List<Object> values = source.getAll(object, feature);
+                List<Object> values = source.getAll(object, f);
                 if (!values.isEmpty()) {
-                    if (requireAttachment(feature)) {
+                    if (requireAttachment(f)) {
                         values = values.stream().map(this::attach).collect(Collectors.toList());
                     }
 
-                    target.setAll(object, feature, values);
+                    target.setAll(object, f, values);
                 }
             }
         });

diff --git a/...core/src/main/java/fr/inria/atlanmod/neoemf/data/store/adapter/TransientStoreAdapter.java b/...core/src/main/java/fr/inria/atlanmod/neoemf/data/store/adapter/TransientStoreAdapter.java
@@ -16,6 +16,7 @@
 import fr.inria.atlanmod.neoemf.core.Id;
 import fr.inria.atlanmod.neoemf.core.PersistentEObject;
 import fr.inria.atlanmod.neoemf.data.store.Store;
+import fr.inria.atlanmod.neoemf.resource.PersistentResource;
 
 import org.eclipse.emf.ecore.resource.Resource;
 
@@ -69,6 +70,11 @@ public void close() {
 
     @Override
     public void resource(@Nullable Resource.Internal resource) {
+        if (holder.resource == resource || PersistentResource.class.isInstance(resource)) {
+            // A transient store cannot be attached to a persistent resource
+            return;
+        }
+
         this.holder.close();
 
         this.holder = SharedHolder.forResource(resource);

diff --git a/neoemf-core/src/main/java/fr/inria/atlanmod/neoemf/resource/DefaultPersistentResource.java b/neoemf-core/src/main/java/fr/inria/atlanmod/neoemf/resource/DefaultPersistentResource.java
@@ -116,14 +116,14 @@ public class DefaultPersistentResource extends ResourceImpl implements Persisten
     public DefaultPersistentResource(URI uri) {
         super(uri);
 
-        rootObject = new RootObject(this);
-
         factory = BackendFactoryRegistry.getInstance().getFactoryProvider(uri.scheme());
 
         Backend backend = factory.createTransientBackend();
         Store baseStore = StoreFactory.getInstance().createStore(backend, CommonOptions.noOption());
         eStore = new PersistentStoreAdapter(baseStore, this);
 
+        rootObject = new RootObject(this);
+
         Log.info("PersistentResource created: {0}", uri);
     }
 
@@ -150,27 +150,29 @@ public EObject getEObject(String uriFragment) {
     @Override
     @SuppressWarnings("unchecked")
     public void save(Map<?, ?> options) throws IOException {
-        innerSave((Map<String, Object>) options);
+        doSave((Map<String, Object>) options);
     }
 
     @Override
     @SuppressWarnings("unchecked")
     public void load(Map<?, ?> options) throws IOException {
-        innerLoad((Map<String, Object>) options);
+        doLoad((Map<String, Object>) options);
     }
 
     @Override
-    protected void doUnload() {
-        Iterable<EObject> allContents = () -> getAllProperContents(unloadingContents);
+    protected List<EObject> getUnloadingContents() {
+        // Content is created on demand
+        return Collections.emptyList();
+    }
 
+    @Override
+    protected void doUnload() {
         getErrors().clear();
         getWarnings().clear();
 
-        MoreIterables.stream(allContents)
-                .map(InternalEObject.class::cast)
-                .forEach(this::unloaded);
+        eStore.close();
 
-        close();
+        Log.info("PersistentResource closed:  {0}", uri);
     }
 
     /**
@@ -183,7 +185,7 @@ protected void doUnload() {
      *
      * @throws IOException if an I/O error occurs during the save
      */
-    protected void innerSave(Map<String, Object> options) throws IOException {
+    protected void doSave(Map<String, Object> options) throws IOException {
         checkOptions(options);
 
         if (!isLoaded || !eStore.store().backend().isPersistent()) {
@@ -216,7 +218,7 @@ protected void innerSave(Map<String, Object> options) throws IOException {
      *
      * @throws IOException if an I/O error occurs during the load
      */
-    protected void innerLoad(Map<String, Object> options) throws IOException {
+    protected void doLoad(Map<String, Object> options) throws IOException {
         try {
             isLoading = true;
 
@@ -244,11 +246,7 @@ protected void innerLoad(Map<String, Object> options) throws IOException {
 
     @Override
     public void close() {
-        eStore.close();
-
-        isLoaded = false;
-
-        Log.info("PersistentResource closed:  {0}", uri);
+        unload();
     }
 
     @Nonnull

diff --git a/neoemf-tests/src/test/java/fr/inria/atlanmod/neoemf/tests/AbstractBackendTest.java b/neoemf-tests/src/test/java/fr/inria/atlanmod/neoemf/tests/AbstractBackendTest.java
@@ -24,6 +24,7 @@
 import fr.inria.atlanmod.neoemf.tests.sample.SampleFactory;
 import fr.inria.atlanmod.neoemf.tests.sample.SamplePackage;
 
+import org.eclipse.emf.ecore.resource.Resource;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.runner.RunWith;
@@ -62,7 +63,7 @@ public abstract class AbstractBackendTest extends AbstractTest implements Contex
      * A list that holds the resources that should be closed at the end of the tests.
      */
     @Nonnull
-    private final List<PersistentResource> loadedResources = new ArrayList<>();
+    private final List<Resource> loadedResources = new ArrayList<>();
 
     /**
      * The current context.
@@ -205,7 +206,7 @@ public final void createWorkspace() throws IOException {
     @After
     public final void cleanWorkspace() {
         if (context.isInitialized()) {
-            loadedResources.forEach(PersistentResource::close);
+            loadedResources.forEach(Resource::unload);
             loadedResources.clear();
         }
     }

diff --git a/neoemf-tests/src/test/java/fr/inria/atlanmod/neoemf/tests/AllInstancesTest.java b/neoemf-tests/src/test/java/fr/inria/atlanmod/neoemf/tests/AllInstancesTest.java
@@ -103,7 +103,7 @@ public void testAllInstancesPersistentLoaded() throws IOException {
         fillResource(resource);
 
         resource.save(context().optionsBuilder().asMap());
-        resource.close();
+        resource.unload();
         resource.load(context().optionsBuilder().asMap());
 
         assertAllInstancesHas(resource, false, NODE_COUNT, PHYSICAL_NODE_COUNT);
@@ -122,7 +122,7 @@ public void testAllInstancesStrictPersistentLoaded() throws IOException {
         fillResource(resource);
 
         resource.save(context().optionsBuilder().asMap());
-        resource.close();
+        resource.unload();
         resource.load(context().optionsBuilder().asMap());
 
         assertAllInstancesHas(resource, true, NODE_STRICT_COUNT, PHYSICAL_NODE_STRICT_COUNT);

diff --git a/neoemf-tests/src/test/java/fr/inria/atlanmod/neoemf/tests/LoadResourceTest.java b/neoemf-tests/src/test/java/fr/inria/atlanmod/neoemf/tests/LoadResourceTest.java
@@ -123,7 +123,7 @@ private PersistentResource fillResource(PersistentResource resource) throws IOEx
         resource.getContents().add(primary);
 
         resource.save(context().optionsBuilder().asMap());
-        resource.close();
+        resource.unload();
 
         PersistentResource newResource = context().loadResource(EPACKAGE, file());
         return closeAtExit(newResource);

diff --git a/plugins/eclipse/ui/src/main/java/fr/inria/atlanmod/neoemf/eclipse/ui/editor/NeoEditor.java b/plugins/eclipse/ui/src/main/java/fr/inria/atlanmod/neoemf/eclipse/ui/editor/NeoEditor.java
@@ -149,14 +149,7 @@ public void dispose() {
      * Closes all opened resources.
      */
     private void closeAll() {
-        for (Resource resource : getEditingDomain().getResourceSet().getResources()) {
-            if (PersistentResource.class.isInstance(resource)) {
-                PersistentResource.class.cast(resource).close();
-            }
-            else {
-                resource.unload();
-            }
-        }
+        getEditingDomain().getResourceSet().getResources().forEach(Resource::unload);
     }
 
     /**