Support reading TG files with shared and non-shared value contexts

This is part of the fix for a nasty corner case in
TransferableGraphFileReader.readTG's deserialization of referable value
data types.

This regression was caused by commit
bf495713dbc9dec325f3929889466fa6cd58b541 over 1.5 years ago. The removal
of idContext.clear() was done without proper understanding of the real
issue.

The real issue is that TG's are currently written in two ways by
different codes: with shared and with non-shared value contexts.

Serializing TransferableGraph1 structures using the default Binding will
use shared context for serializing the values Variant array. Thus all TG
files produced by the graph compiler use a shared value context which
eans this class must be used with sharedValueContext set to true. As an
example, <code>true</code> must be used if the corresponding TG file is
written e.g. like this:

    DataContainers.writeFile(location
        new DataContainer(format,
                          version,
                          metadata,
                          new Variant(TransferableGraph1.BINDING, tg)));

On the other hand, any TG files serialized using more optimized methods
like ModelTransferableGraphSource do not use a shared value context when
writing the file. This means those files cannot be read safely using
standard {@link Binding} at all, and when using this class,
sharedValueContext must be set to false to prevent the import from
corrupting datatype values because the referable parts of datatypes may
get bound to the wrong existing types if the data is read using shared
context.

After this change clients should use readTG(File | InputStream, boolean)
methods instead of readTG to always specify the context sharing setting
explicitly according to how the TG's in question were written.

gitlab #409

Change-Id: Ic18fd1442987d4e740f729729b81cd3133f5d269

diff --git a/bundles/org.simantics.db.layer0/src/org/simantics/db/layer0/migration/MigrationStateImpl.java b/bundles/org.simantics.db.layer0/src/org/simantics/db/layer0/migration/MigrationStateImpl.java
index f50bae78aa2e8c8ebf702efd86c472c436ce2bea..fdd6f9132fe4b78d45d6874275778ccdc4481465 100644 (file)
--- a/bundles/org.simantics.db.layer0/src/org/simantics/db/layer0/migration/MigrationStateImpl.java
+++ b/bundles/org.simantics.db.layer0/src/org/simantics/db/layer0/migration/MigrationStateImpl.java
@@ -109,7 +109,7 @@ public class MigrationStateImpl implements MigrationState {
             try {
                 File modelFile = getProperty(MigrationStateKeys.MODEL_FILE);
                 TimeLogger.log(MigrationStateImpl.class, "reading TG into memory from " + modelFile);
-                TransferableGraph1 tg = TransferableGraphFileReader.read(modelFile);
+                TransferableGraph1 tg = TransferableGraphFileReader.read(modelFile, false);
                 TimeLogger.log(MigrationStateImpl.class, "read TG into memory from " + modelFile);
                 setProperty(MigrationStateKeys.CURRENT_TG, tg);
                 return (T)tg;

diff --git a/bundles/org.simantics.graph/src/org/simantics/graph/representation/TransferableGraphFileReader.java b/bundles/org.simantics.graph/src/org/simantics/graph/representation/TransferableGraphFileReader.java
index 6fc7ac5516f5cc9efb7da5f8cd3a4f588565fb54..c7cc415045c490c7cb2f496a1db7af387a1c8035 100644 (file)
--- a/bundles/org.simantics.graph/src/org/simantics/graph/representation/TransferableGraphFileReader.java
+++ b/bundles/org.simantics.graph/src/org/simantics/graph/representation/TransferableGraphFileReader.java
@@ -5,13 +5,13 @@ import java.io.DataInputStream;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
-import java.nio.ByteBuffer;
 import java.nio.channels.ReadableByteChannel;
 import java.util.ArrayList;
 import java.util.List;
 
 import org.simantics.databoard.Bindings;
 import org.simantics.databoard.Datatypes;
+import org.simantics.databoard.binding.Binding;
 import org.simantics.databoard.binding.error.RuntimeDatatypeConstructionException;
 import org.simantics.databoard.binding.mutable.Variant;
 import org.simantics.databoard.container.DataContainer;
@@ -26,9 +26,45 @@ import org.slf4j.LoggerFactory;
 /**
  * It is recommended to use {@link #read(File)} and {@link #read(InputStream)}
  * for reading to ensure proper resource handling.
+ * 
+ * <p>
+ * It is important to use the correct setting for
+ * {@link #sharedValueContext(boolean)} which depends on how the TG was
+ * serialized to begin with. See {@link #DEFAULT_SHARED_VALUE_CONTEXT} for more
+ * information on this.
  */
 final public class TransferableGraphFileReader extends ByteFileReader {
 
+    /**
+     * Serializing TransferableGraph1 structures using the default {@link Binding}
+     * will use shared context for serializing the values Variant array. Thus all TG
+     * files produced by the graph compiler use a shared value context which means
+     * this class must be used with {@link #sharedValueContext(boolean)} set to
+     * true. As an example, <code>true</code> must be used if the corresponding TG
+     * file is written e.g. like this:
+     * <pre>
+     * DataContainers.writeFile(location 
+     *     new DataContainer(format,
+     *                       version,
+     *                       metadata,
+     *                       new Variant(TransferableGraph1.BINDING, tg)));
+     * </pre>
+     * 
+     * <p>
+     * On the other hand, any TG files serialized using more optimized methods like
+     * <code>ModelTransferableGraphSource</code> do not use a shared value context
+     * when writing the file. This means those files cannot be read safely using
+     * standard {@link Binding} at all, and when using this class,
+     * {@link #sharedValueContext(boolean)} must be set to false to prevent the
+     * import from corrupting datatype values because the referable parts of
+     * datatypes may get bound to the wrong existing types if the data is read using
+     * shared context.
+     * 
+     * <p>
+     * <code>true</code> is the default setting.
+     */
+    public static final boolean DEFAULT_SHARED_VALUE_CONTEXT = true;
+
     private static final Logger LOGGER = LoggerFactory.getLogger(TransferableGraphFileReader.class);
 
        InputStream in = new InputStream() {
@@ -59,6 +95,7 @@ final public class TransferableGraphFileReader extends ByteFileReader {
        final private static int SIZE = 1<<18;
        final private static int HEADER = headerSize();
        final private int header;
+       private boolean shareValueContext = true;
 
        /**
         * Reads a {@link DataContainer} containing a {@link TransferableGraph1}
@@ -69,8 +106,34 @@ final public class TransferableGraphFileReader extends ByteFileReader {
         * @throws IOException
         */
        public static TransferableGraph1 read(File file) throws IOException {
+               return read(file, DEFAULT_SHARED_VALUE_CONTEXT);
+       }
+
+       /**
+        * Reads a {@link DataContainer} containing a {@link TransferableGraph1}
+        * structure from the specified InputStream. Note that this implementation does
+        * not close the specified <code>input</code> stream, it is expected to be
+        * closed by the caller.
+        * 
+        * @param input the input stream to read from
+        * @return the TG contained by the stream
+        * @throws IOException
+        */
+       public static TransferableGraph1 read(InputStream input) throws IOException {
+               return read(input, DEFAULT_SHARED_VALUE_CONTEXT);
+       }
+
+       /**
+        * Reads a {@link DataContainer} containing a {@link TransferableGraph1}
+        * structure from the specified {@link File}.
+        * 
+        * @param file the file to read from
+        * @return the TG contained by the file
+        * @throws IOException
+        */
+       public static TransferableGraph1 read(File file, boolean sharedValueContext) throws IOException {
                try (TransferableGraphFileReader reader = new TransferableGraphFileReader(file)) {
-                       return reader.readTG();
+                       return reader.sharedValueContext(sharedValueContext).readTG();
                }
        }
 
@@ -84,9 +147,9 @@ final public class TransferableGraphFileReader extends ByteFileReader {
         * @return the TG contained by the stream
         * @throws IOException
         */
-       public static TransferableGraph1 read(InputStream input) throws IOException {
+       public static TransferableGraph1 read(InputStream input, boolean sharedValueContext) throws IOException {
                try (TransferableGraphFileReader reader = new TransferableGraphFileReader(input)) {
-                       return reader.readTG();
+                       return reader.sharedValueContext(sharedValueContext).readTG();
                }
        }
 
@@ -135,6 +198,11 @@ final public class TransferableGraphFileReader extends ByteFileReader {
                this.header = HEADER;
        }
 
+       public TransferableGraphFileReader sharedValueContext(boolean share) {
+               this.shareValueContext = share;
+               return this;
+       }
+
        private static int headerSize() {
                try {
                        return Bindings.getSerializerUnchecked(Datatype.class).serialize(Datatypes.getDatatypeUnchecked(TransferableGraph1.class)).length;
@@ -146,7 +214,7 @@ final public class TransferableGraphFileReader extends ByteFileReader {
                        throw new Error("Failed to determine TransferableGraph1 header size. ", e);
                }               
        }
-       
+
        public TransferableGraph1 readTG() throws IOException {
 
                if(getSize() == 0) return null;
@@ -269,7 +337,8 @@ final public class TransferableGraphFileReader extends ByteFileReader {
                
                for(int i=0;i<valueLength;i++) {
                        int resource = safeInt();
-                       //idcontext.clear();
+                       if (!shareValueContext)
+                               idcontext.clear();
                        Variant value = (Variant)variantSerializer
                                .deserialize((DataInput)dis, idcontext);
                        values[i] = new Value(resource, value);