View Issue Details

IDProjectCategoryView StatusLast Update
0001529SymmetricDSBugpublic2014-02-05 10:08
Reporterboris_pavlovicAssigned Tochenson 
Priorityurgent 
Status closedResolutionfixed 
Product Version3.5.11 
Target Version3.5.16Fixed in Version3.5.16 
Summary0001529: Columns in old data are not transformed causing conflict detection
Descriptiontwo node groups:
 - corp
 - store

table with slightly different names:
 - corp: CORP_ITEM(ID_CORP_ITEM, CODE_CORP_ITEM)
 - store: STORE_ITEM(ID_STORE_ITEM, CODE_STORE_ITEM)

SYM_TRANSFORM_TABLE and SYM_TRANSFORM_COLUMN configured to support different naming at EXTRACTING

SYM_CONFLICT configured for both directions (corp -> store, store -> corp) with:
 - OLD_DATA
 - MANUAL resolution
 - OFF resolve type

initial load works fine replicating two rows from store to corp nodes:
 - (1, 'code_1')
 - (2, 'code_2')

on one store node update the value of CODE_STORE_ITEM to 'code_1_1'

the change will be successfully synchronized to corp

corp sends the update to all other nodes in store node group (sync_on_incoming_batch is set to 1)

then we have a conflict with the primary key ID_STORE_ITEM = 1
Steps To Reproducedebugging the symmetricDS it has been found that the OLD_DATA's columns on EXTRACT are not transformed (renamed) and the update statement on target nodes is returning 0 because there are missing values (in this case they are nulls) for the columns from OLD_VALUE that haven't been transformed
Tagsconflict manager, transformation

Activities

chenson

2014-01-14 08:02

administrator   ~0000415

This has already been fixed in 3.5.12

boris_pavlovic

2014-01-14 10:12

reporter   ~0000416

I have just tried with 3.5.13 and the problem persists

The update statement using OLD_VALUES as means for conflict detection misses the values of columns that are supposed to be transformed as declared in SYM_TRANSFORM_COLUMN. Update statement returns 0 and conflict has been detected

chenson

2014-01-14 20:03

administrator   ~0000417

Sorry, I did not read the issue close enough.

I can probably copy the old values over to the transformed old data if the transform type is 'copy'. Not sure what to do if the transform type is something else. In some cases the transform logic might not even apply to the old data.

Do you have ideas on how you would expect it to work?

boris_pavlovic

2014-01-15 03:19

reporter   ~0000418

Last edited: 2014-01-15 03:20

View 2 revisions

No problemo :)

I have observed that declarations from SYM_TRANSFORM_TABLE and SYM_TRANSFORM_COLUMN when configured on EXTRACTION time are applied only on current_values of CsvData. What it would need to be done is to have both current and old values transformed.

Let's say we have two tables:
 - corp: corp_table(id_corp_table, code_corp_table, status_corp, corp_only_col)
 - store: store_table(id_store_table, code_store_table, status_store, store_only_col)

and we have a SYM_TRANSFORM_TABLE:
 - from: corp
 - to: store
 - source: corp_table
 - target: store_table

and SYM_TRANSFORM_COLUMN:
 - from: corp
 - to: store
 - source table: corp_table
 - target table: store_table
 (
  - source column: id_corp_table
  - target column: id_store_table
 ),
 (
  - source column: code_corp_table
  - target column: code_store_table
 ),
 (
  - source column: status_corp_table
  - target column: status_store_table
 )

Columns corp_table.corp_only_col and store_table.store_only_col do not get transformed since they don't have their counterparts.

Let's say we define the SYM_CONFLICT:
 - source: corp
 - destination: store
 - type: USE_OLD_DATA
 - resolution: MANUAL

Step 1: initial load of all data from corp to stores
Step 2: @store_1: update store_table set status_store_table = status_store_table + 1 where id_store_table = 1;
Step 3: @corp: corp_table row with id_corp_table = 1 gets updated with the incremented status_corp_table
Step 4: @corp: at extract CsvData is created with
 OLD_DATA:
   - id_corp_table = 1
   - code_corp_table = 'code_1' (or whatever the value used to be)
   - status_corp_table = 1 (if we assume that the old value was 1)
 CURRENT_DATA:
   - id_store_table = 1
   - status_store_table = 2
 PK_DATA:
   - id_store_table = 1
Step 5: @store_2
   update store_table
    set status_store_table = 2
   where id_store_table = 1 (coming from PK_DATA)
   and code_store_table = null (OLD_DATA.code_store_table is null because there's only OLD_DATA.code_corp_table (equal to 'code_1'))
   and status_store_table = null (OLD_DATA.status_store_table is null because there's only OLD_DATA.status_corp_table (equal to 1))


CONCLUSION: at extract time not only the current values should be transformed (and serialized into CSV), but the old values, too, so we could use them for OLD_DATA_VALUES or CHANGED_VALUES conflict detection.

boris_pavlovic

2014-01-15 09:25

reporter  

bug_fix_for_http___www_symmetricds_org_issues_view_php_id=1529.patch (1,546 bytes)
Index: symmetric-io/src/main/java/org/jumpmind/symmetric/io/data/transform/TransformedData.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- symmetric-io/src/main/java/org/jumpmind/symmetric/io/data/transform/TransformedData.java	(revision 7859)
+++ symmetric-io/src/main/java/org/jumpmind/symmetric/io/data/transform/TransformedData.java	(revision )
@@ -282,11 +282,24 @@
                 if (oldSourceValues.containsKey(name)) {
                     values.add(oldSourceValues.get(name));
                 } else {
-                    /*
-                     * TODO copy transforms should probably copy the old value
-                     * to the new value. Currently, it will be null.
-                     */
+
+                    String oldSourceKey = null;
+
+                    for (TransformColumn transformColumn : transformation.getTransformColumns()) {
+
+                        if (transformColumn.getTargetColumnName().equals(name)) {
+                            oldSourceKey = transformColumn.getSourceColumnName();
+                            break;
+                        }
+                    }
+
+                    if (null == oldSourceKey) {
+
-                    values.add(null);
+                        values.add(null);
+                    } else {
+
+                        values.add(oldSourceValues.get(oldSourceKey));
+                    }
                 }
             }
         }

boris_pavlovic

2014-01-15 09:27

reporter   ~0000420

Please find the bug fix (actually TODO note) in the attached file: http://www.symmetricds.org/issues/file_download.php?file_id=38&type=bug

boris_pavlovic

2014-01-27 03:55

reporter   ~0000429

I haven't read your comment very closely, too. My patch works only for COPY column transformation mode. Here's an updated patch that will copy the value to the old column only when the transform type has been configured as COPY.

boris_pavlovic

2014-01-27 04:28

reporter  

bug_fix_for_http___www_symmetricds_org_issues_view_php_id=1530_taking_in_regard_transform_.patch (4,945 bytes)
Index: symmetric-io/src/main/java/org/jumpmind/symmetric/io/data/transform/TransformedData.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- symmetric-io/src/main/java/org/jumpmind/symmetric/io/data/transform/TransformedData.java	(revision 7859)
+++ symmetric-io/src/main/java/org/jumpmind/symmetric/io/data/transform/TransformedData.java	(revision )
@@ -20,11 +20,7 @@
  */
 package org.jumpmind.symmetric.io.data.transform;
 
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 
 import org.jumpmind.db.model.Column;
 import org.jumpmind.db.model.Table;
@@ -32,6 +28,9 @@
 import org.jumpmind.symmetric.io.data.DataEventType;
 import org.jumpmind.symmetric.io.data.transform.TransformColumn.IncludeOnType;
 
+import static java.util.Arrays.asList;
+import static org.jumpmind.symmetric.io.data.transform.CopyColumnTransform.NAME;
+
 public class TransformedData implements Cloneable {
 
     protected boolean generatedIdentityNeeded = false;
@@ -271,25 +270,90 @@
         List<String> keyNames = retrieve(keysBy, true);
         List<String> keyValues = retrieve(keysBy, false);
         List<String> values = new ArrayList<String>();
-        for (String name : names) {
+
+        for (String name : names)
+            addValue(keyNames, keyValues, values, name);
+
+        return values.toArray(new String[values.size()]);
+    }
+
+    private void addValue(List<String> keyNames, List<String> keyValues, List<String> values, String name) {
+
-            if (keyNames.contains(name)) {
-                /*
-                 * Must always use the transformed value for the key else
-                 * deletes and updates won't work.
-                 */
-                values.add(keyValues.get(keyNames.indexOf(name)));
+        if (keyNames.contains(name)) {
+            /*
+             * Must always use the transformed value for the key else
+             * deletes and updates won't work.
+             */
+            values.add(keyValues.get(keyNames.indexOf(name)));
-            } else {
+            return;
+        }
+
-                if (oldSourceValues.containsKey(name)) {
+        if (oldSourceValues.containsKey(name)) {
+
-                    values.add(oldSourceValues.get(name));
+            values.add(oldSourceValues.get(name));
-                } else {
-                    /*
-                     * TODO copy transforms should probably copy the old value
-                     * to the new value. Currently, it will be null.
-                     */
+            return;
+        }
+
+        TransformColumn transformColumn = null;
+
+        for (TransformColumn tc : transformation.getTransformColumns())
+            if (tc.getTargetColumnName().equals(name)) {
+                transformColumn = tc;
+                break;
+            }
+
+        if (null == transformColumn) {
+
-                    values.add(null);
+            values.add(null);
+            return;
-                }
+        }
+
+        String transformType = transformColumn.getTransformType();
+
+        if (NAME.equals(transformType)) {
+            values.add(oldSourceValues.get(transformColumn.getSourceColumnName()));
+            return;
-            }
+        }
+
+        if ("remove".equalsIgnoreCase(transformType)) {
+            values.add(null);
+            return;
         }
-        return values.toArray(new String[values.size()]);
+
+        if ("const".equalsIgnoreCase(transformType)) {
+
+            values.add(transformColumn.getTransformExpression());
+            return;
-    }
+        }
+
+        if ("substr".equalsIgnoreCase(transformType)) {
+
+            String value = oldSourceValues.get(transformColumn.getSourceColumnName());
+
+            try {
+                values.add(new SubstrColumnTransform().transform(
+                        null, // IDatabasePlatform
+                        null, // DataContext
+                        transformColumn,
+                        null, // TransformedData
+                        null, // Map<String, String> sourceValue
+                        value, // String newValue
+                        null // String oldValue
+                ));
+            } catch (IgnoreColumnException e) {
+                throw new RuntimeException(e);
+            } catch (IgnoreRowException e) {
+                throw new RuntimeException(e);
+            }
+
+            return;
+        }
+
+        if (asList("variable", "additive", "multiply", "lookup", "bsh", "identity").
+                contains(transformType.toLowerCase()))
+            throw new UnsupportedOperationException(
+                    "'"+ transformType +"' transformation of OLD values hasn't been implemented"
+            );
+    }
+
 }

chenson

2014-01-27 06:56

administrator   ~0000430

Awesome! Have you sign the CLA? http://www.symmetricds.org/developer/contributor. I'll get this reviewed and committed for the next patch release after you have.

boris_pavlovic

2014-01-27 10:57

reporter   ~0000431

Last edited: 2014-01-27 11:24

View 2 revisions

I've just realized that the second patch is throwing UnsupportedOperationException for transformation types: variable, additive, multiply, lookup, bsh and identity. It should be throwing it when there's such transformation configured with a conflict detection of type OLD_DATA or CHANGED_DATA. Otherwise it can be ignored.

P.S. CLA signed and sent by email

chenson

2014-01-29 18:41

administrator   ~0000433

Yeah. I was going to comment on that. Not sure the best way to detect that. I'll have to think about that.

Do you think that:

        if (oldSourceValues.containsKey(name)) {
            values.add(oldSourceValues.get(name));
            return;
        }

Should be done, only if a transformation can't be found? Seems like the transformation type would be ignored if there is an old source value that matches ...

boris_pavlovic

2014-01-30 07:49

reporter  

20140130_bug_fix_for_http___www_symmetricds_org_issues_view_php_id=1530.patch (19,453 bytes)
Index: symmetric-io/pom.xml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- symmetric-io/pom.xml	(revision 7859)
+++ symmetric-io/pom.xml	(revision )
@@ -26,7 +26,7 @@
             <groupId>org.jumpmind.symmetric</groupId>
             <artifactId>symmetric-jdbc</artifactId>
             <scope>test</scope>
-        </dependency>   		
+        </dependency>
 		<dependency>
 			<groupId>org.jumpmind.symmetric</groupId>
 			<artifactId>symmetric-jdbc</artifactId>
@@ -50,7 +50,7 @@
             <groupId>log4j</groupId>
             <artifactId>log4j</artifactId>
             <scope>provided</scope>
-        </dependency>		
+        </dependency>
 		<!-- Databases -->
 		<dependency>
 			<groupId>org.apache.derby</groupId>
@@ -66,7 +66,7 @@
 			<groupId>mysql</groupId>
 			<artifactId>mysql-connector-java</artifactId>
 			<scope>provided</scope>
-		</dependency>		
+		</dependency>
 		<dependency>
             <groupId>org.jumpmind.symmetric.jdbc</groupId>
             <artifactId>mariadb-java-client</artifactId>
@@ -108,6 +108,30 @@
             <groupId>org.apache.geronimo.specs</groupId>
             <artifactId>geronimo-j2ee-connector_1.6_spec</artifactId>
             <scope>provided</scope>
-        </dependency>		
+        </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-all</artifactId>
+            <version>1.9.5</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.powermock</groupId>
+            <artifactId>powermock-api-mockito</artifactId>
+            <version>1.5.3</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.powermock</groupId>
+            <artifactId>powermock-module-junit4</artifactId>
+            <version>1.5.3</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.powermock</groupId>
+            <artifactId>powermock-core</artifactId>
+            <version>1.5.3</version>
+            <scope>test</scope>
+        </dependency>
 	</dependencies>
 </project>
Index: symmetric-io/src/main/java/org/jumpmind/symmetric/io/data/transform/TransformedData.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- symmetric-io/src/main/java/org/jumpmind/symmetric/io/data/transform/TransformedData.java	(revision 7859)
+++ symmetric-io/src/main/java/org/jumpmind/symmetric/io/data/transform/TransformedData.java	(revision )
@@ -20,18 +20,17 @@
  */
 package org.jumpmind.symmetric.io.data.transform;
 
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
-
 import org.jumpmind.db.model.Column;
 import org.jumpmind.db.model.Table;
 import org.jumpmind.symmetric.io.data.CsvData;
 import org.jumpmind.symmetric.io.data.DataEventType;
 import org.jumpmind.symmetric.io.data.transform.TransformColumn.IncludeOnType;
 
+import java.util.*;
+
+import static java.util.Arrays.asList;
+import static org.jumpmind.symmetric.io.data.transform.CopyColumnTransform.NAME;
+
 public class TransformedData implements Cloneable {
 
     protected boolean generatedIdentityNeeded = false;
@@ -53,8 +52,9 @@
     protected Map<String, String> sourceValues;
 
     public TransformedData(TransformTable transformation, DataEventType sourceDmlType,
-            Map<String, String> sourceKeyValues, Map<String, String> oldSourceValues,
-            Map<String, String> sourceValues) {
+                           Map<String, String> sourceKeyValues, Map<String, String> oldSourceValues,
+                           Map<String, String> sourceValues) {
+
         this.transformation = transformation;
         this.targetDmlType = sourceDmlType;
         this.sourceDmlType = sourceDmlType;
@@ -64,30 +64,37 @@
     }
 
     public String getFullyQualifiedTableName() {
+
         return transformation.getFullyQualifiedTargetTableName();
     }
 
     public DataEventType getTargetDmlType() {
+
         return targetDmlType;
     }
 
     public void setTargetDmlType(DataEventType dmlType) {
+
         this.targetDmlType = dmlType;
     }
 
     public String getTableName() {
+
         return transformation.getTargetTableName();
     }
 
     public String getCatalogName() {
+
         return transformation.getTargetCatalogName();
     }
 
     public String getSchemaName() {
+
         return transformation.getTargetSchemaName();
     }
 
     public void put(TransformColumn column, String columnValue, boolean recordAsKey) {
+
         if (recordAsKey) {
             if (keysBy == null) {
                 keysBy = new HashMap<TransformColumn.IncludeOnType, LinkedHashMap<String, String>>(
@@ -114,6 +121,7 @@
     protected List<String> retrieve(
             Map<TransformColumn.IncludeOnType, LinkedHashMap<String, String>> source,
             boolean getColumnNames) {
+
         List<String> list = new ArrayList<String>(source == null ? 0 : source.size());
         if (source != null) {
             LinkedHashMap<String, String> values = source.get(IncludeOnType.ALL);
@@ -145,30 +153,36 @@
     }
 
     public String[] getKeyNames() {
+
         List<String> list = retrieve(keysBy, true);
         return list.toArray(new String[list.size()]);
     }
 
     public String[] getKeyValues() {
+
         List<String> list = retrieve(keysBy, false);
         return list.toArray(new String[list.size()]);
     }
 
     public String[] getColumnNames() {
+
         List<String> list = retrieve(columnsBy, true);
         return list.toArray(new String[list.size()]);
     }
 
     public String[] getColumnValues() {
+
         List<String> list = retrieve(columnsBy, false);
         return list.toArray(new String[list.size()]);
     }
 
     public DataEventType getSourceDmlType() {
+
         return sourceDmlType;
     }
 
     public TransformedData copy() {
+
         try {
             TransformedData clone = (TransformedData) this.clone();
             clone.columnsBy = copy(columnsBy);
@@ -180,19 +194,23 @@
     }
 
     public TransformTable getTransformation() {
+
         return transformation;
     }
 
     public void setGeneratedIdentityNeeded(boolean generatedIdentityNeeded) {
+
         this.generatedIdentityNeeded = generatedIdentityNeeded;
     }
 
     public boolean isGeneratedIdentityNeeded() {
+
         return generatedIdentityNeeded;
     }
 
     protected Map<TransformColumn.IncludeOnType, LinkedHashMap<String, String>> copy(
             Map<TransformColumn.IncludeOnType, LinkedHashMap<String, String>> toCopy) {
+
         Map<TransformColumn.IncludeOnType, LinkedHashMap<String, String>> newMap = new HashMap<TransformColumn.IncludeOnType, LinkedHashMap<String, String>>(
                 toCopy.size());
         for (TransformColumn.IncludeOnType key : toCopy.keySet()) {
@@ -203,18 +221,22 @@
     }
 
     public Map<String, String> getSourceKeyValues() {
+
         return sourceKeyValues;
     }
 
     public Map<String, String> getOldSourceValues() {
+
         return oldSourceValues;
     }
 
     public Map<String, String> getSourceValues() {
+
         return sourceValues;
     }
 
     public boolean hasSameKeyValues(String[] otherKeyValues) {
+
         String[] keyValues = getKeyValues();
         if (otherKeyValues != null) {
             if (keyValues != null) {
@@ -236,6 +258,7 @@
     }
 
     public Table buildTargetTable() {
+
         Table table = null;
         String[] columnNames = getColumnNames();
         String[] keyNames = getKeyNames();
@@ -258,6 +281,7 @@
     }
 
     public CsvData buildTargetCsvData() {
+
         CsvData data = new CsvData(this.targetDmlType);
         data.putParsedData(CsvData.OLD_DATA, getOldColumnValues());
         data.putParsedData(CsvData.ROW_DATA, getColumnValues());
@@ -267,29 +291,110 @@
     }
 
     public String[] getOldColumnValues() {
+
         List<String> names = retrieve(columnsBy, true);
         List<String> keyNames = retrieve(keysBy, true);
         List<String> keyValues = retrieve(keysBy, false);
         List<String> values = new ArrayList<String>();
-        for (String name : names) {
+
+        for (String name : names)
+            addValue(keyNames, keyValues, values, name);
+
+        return values.toArray(new String[values.size()]);
+    }
+
+    private void addValue(List<String> keyNames, List<String> keyValues, List<String> values, String name) {
+
-            if (keyNames.contains(name)) {
-                /*
-                 * Must always use the transformed value for the key else
-                 * deletes and updates won't work.
-                 */
-                values.add(keyValues.get(keyNames.indexOf(name)));
+        if (keyNames.contains(name)) {
+            /*
+             * Must always use the transformed value for the key else
+             * deletes and updates won't work.
+             */
+            values.add(keyValues.get(keyNames.indexOf(name)));
-            } else {
+            return;
+        }
+
+        TransformColumn transformColumn = findTransformColumn(name);
+
+        if (null == transformColumn) {
+
-                if (oldSourceValues.containsKey(name)) {
+            if (oldSourceValues.containsKey(name)) {
+
-                    values.add(oldSourceValues.get(name));
+                values.add(oldSourceValues.get(name));
-                } else {
-                    /*
-                     * TODO copy transforms should probably copy the old value
-                     * to the new value. Currently, it will be null.
-                     */
+                return;
+            }
+
-                    values.add(null);
+            values.add(null);
+            return;
-                }
+        }
+
+        String transformType = transformColumn.getTransformType();
+
+        if (NAME.equals(transformType)) {
+            values.add(oldSourceValues.get(transformColumn.getSourceColumnName()));
+            return;
-            }
+        }
+
+        if ("remove".equalsIgnoreCase(transformType)) {
+            values.add(null);
+            return;
         }
-        return values.toArray(new String[values.size()]);
+
+        if ("const".equalsIgnoreCase(transformType)) {
+
+            values.add(transformColumn.getTransformExpression());
+            return;
-    }
+        }
+
+        if ("substr".equalsIgnoreCase(transformType)) {
+
+            String value = oldSourceValues.get(transformColumn.getSourceColumnName());
+
+            try {
+                values.add(new SubstrColumnTransform().transform(
+                        null, // IDatabasePlatform
+                        null, // DataContext
+                        transformColumn,
+                        null, // TransformedData
+                        null, // Map<String, String> sourceValue
+                        value, // String newValue
+                        null // String oldValue
+                ));
+            } catch (IgnoreColumnException e) {
+                throw new RuntimeException(e);
+            } catch (IgnoreRowException e) {
+                throw new RuntimeException(e);
+            }
+
+            return;
+        }
+
+        if (asList("variable", "additive", "multiply", "lookup", "bsh", "identity").
+                contains(transformType.toLowerCase()) && isConflictDetected())
+            throw new UnsupportedOperationException(
+                    "'" + transformType + "' transformation of OLD values hasn't been implemented"
+            );
+
+        values.add(null);
+    }
+
+    private boolean isConflictDetected() {
+
+        // TODO return true if USE_OLD_DATA or USE_CHANGED_DATA
+        //      needed to inject DatabaseWriterSettings and invoke the method Conflict pickConflict(Table table, Batch batch)
+
+        // Conflict.DetectConflict type = DatabaseWriterSettings#pickConflict(table, batch).getDetectType();
+        // return type == Conflict.DetectConflict.USE_OLD_DATA || type == Conflict.DetectConflict.USE_CHANGED_DATA;
+
+        return false;
+    }
+
+    private TransformColumn findTransformColumn(String name) {
+
+        for (TransformColumn tc : transformation.getTransformColumns())
+            if (tc.getTargetColumnName().equals(name))
+                return tc;
+        return null;
+    }
+
 }
Index: symmetric-io/src/test/java/org/jumpmind/symmetric/io/data/transform/TransformedDataTest.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- symmetric-io/src/test/java/org/jumpmind/symmetric/io/data/transform/TransformedDataTest.java	(revision )
+++ symmetric-io/src/test/java/org/jumpmind/symmetric/io/data/transform/TransformedDataTest.java	(revision )
@@ -0,0 +1,176 @@
+package org.jumpmind.symmetric.io.data.transform;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+import java.util.*;
+
+import static java.util.Arrays.asList;
+import static org.hamcrest.core.IsEqual.equalTo;
+import static org.hamcrest.core.IsNull.nullValue;
+import static org.jumpmind.symmetric.io.data.DataEventType.UPDATE;
+import static org.jumpmind.symmetric.io.data.transform.CopyColumnTransform.NAME;
+import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import static org.powermock.api.mockito.PowerMockito.whenNew;
+import static org.powermock.reflect.Whitebox.invokeMethod;
+
+@RunWith(PowerMockRunner.class)
+@PrepareForTest({TransformedData.class})
+public class TransformedDataTest {
+
+    public static final String ADD_VALUE_METHOD = "addValue";
+    private TransformedData transformedData;
+    private TransformTable transformation;
+    private HashMap<String, String> sourceKeyValues;
+    private HashMap<String, String> oldSourceValues;
+    private HashMap<String, String> sourceValues;
+    private List<String> keyNames;
+    private List<String> keyValues;
+    private List<String> values;
+    public static final String COLUMN_VALUE = String.valueOf(Integer.MAX_VALUE);
+    private TransformColumn transformColumn;
+    public static final String TARGET_NAME = "code_store_table";
+    public static final String SOURCE_NAME = "code_corp_table";
+
+    @Before
+    public void setUp() throws Exception {
+
+        keyNames = new ArrayList<String>();
+        keyValues = new ArrayList<String>();
+        values = new ArrayList<String>();
+
+        transformedData = new TransformedData(
+                transformation = mock(TransformTable.class),
+                UPDATE,
+                sourceKeyValues = new HashMap<String, String>(),
+                oldSourceValues = new HashMap<String, String>(),
+                sourceValues = new HashMap<String, String>()
+        );
+
+        transformColumn = mock(TransformColumn.class);
+    }
+
+    @After
+    public void tearDown() throws Exception {
+
+        for (Collection c : new Collection[]{keyNames, keyValues, values})
+            c.clear();
+
+        for (Map m : new Map[]{sourceKeyValues, oldSourceValues, sourceValues})
+            m.clear();
+    }
+
+    @Test
+    public void addKeyValue() throws Exception {
+
+        String name = "id_store_table";
+        String value = String.valueOf(Integer.MAX_VALUE);
+
+        keyNames.add(name);
+        keyValues.add(value);
+
+        invokeMethod(transformedData, ADD_VALUE_METHOD, keyNames, keyValues, values, name);
+
+        assertThat(values.iterator().next(), equalTo(value));
+    }
+
+    @Test
+    public void addValueWithoutTransformation() throws Exception {
+
+        String name = "column_same_name_on_source_and_target_nodes";
+
+        oldSourceValues.put(name, COLUMN_VALUE);
+
+        invokeMethod(transformedData, ADD_VALUE_METHOD, keyNames, keyValues, values, name);
+
+        assertThat(values.iterator().next(), equalTo(COLUMN_VALUE));
+    }
+
+    @Test
+    public void addNoValueWithoutTransformation() throws Exception {
+
+        String name = "column_same_name_on_source_and_target_nodes";
+        invokeMethod(transformedData, ADD_VALUE_METHOD, keyNames, keyValues, values, name);
+
+        assertThat(values.iterator().next(), nullValue());
+    }
+
+    @Test
+    public void addValueCopy() throws Exception {
+
+        setTransformColumn(TARGET_NAME, SOURCE_NAME, NAME);
+
+        oldSourceValues.put(SOURCE_NAME, COLUMN_VALUE);
+
+        invokeMethod(transformedData, ADD_VALUE_METHOD, keyNames, keyValues, values, TARGET_NAME);
+
+        assertThat(values.iterator().next(), equalTo(COLUMN_VALUE));
+    }
+
+    @Test
+    public void removeTransformedValue() throws Exception {
+
+        setTransformColumn(TARGET_NAME, SOURCE_NAME, "remove");
+
+        oldSourceValues.put(SOURCE_NAME, COLUMN_VALUE);
+
+        invokeMethod(transformedData, ADD_VALUE_METHOD, keyNames, keyValues, values, TARGET_NAME);
+
+        assertThat(values.iterator().next(), nullValue());
+    }
+
+    @Test
+    public void addTransformedConstant() throws Exception {
+
+        String someConstantValue = "some constant value";
+        when(transformColumn.getTransformExpression()).thenReturn(someConstantValue);
+        setTransformColumn(TARGET_NAME, SOURCE_NAME, "const");
+
+        oldSourceValues.put(SOURCE_NAME, COLUMN_VALUE);
+
+        invokeMethod(transformedData, ADD_VALUE_METHOD, keyNames, keyValues, values, TARGET_NAME);
+
+        assertThat(values.iterator().next(), equalTo(someConstantValue));
+    }
+
+    @Test
+    public void addTransformedSubstringValue() throws Exception {
+
+        setTransformColumn(TARGET_NAME, SOURCE_NAME, "substr");
+
+        oldSourceValues.put(SOURCE_NAME, COLUMN_VALUE);
+
+        SubstrColumnTransform substrColumnTransform = mock(SubstrColumnTransform.class);
+
+        String substring = COLUMN_VALUE.substring(1, 2);
+        when(substrColumnTransform.transform(
+                null, // IDatabasePlatform
+                null, // DataContext
+                transformColumn,
+                null, // TransformedData
+                null, // Map<String, String> sourceValue
+                COLUMN_VALUE, // String newValue
+                null // String oldValue
+        )).thenReturn(substring);
+
+        whenNew(SubstrColumnTransform.class).withNoArguments().thenReturn(substrColumnTransform);
+
+        invokeMethod(transformedData, ADD_VALUE_METHOD, keyNames, keyValues, values, TARGET_NAME);
+
+        assertThat(values.iterator().next(), equalTo(substring));
+    }
+
+    private void setTransformColumn(String targetName, String sourceName, String transformType) {
+
+        when(transformColumn.getTargetColumnName()).thenReturn(targetName);
+        when(transformColumn.getTransformType()).thenReturn(transformType);
+        when(transformColumn.getSourceColumnName()).thenReturn(sourceName);
+        when(transformation.getTransformColumns()).thenReturn(asList(transformColumn));
+    }
+}

boris_pavlovic

2014-01-30 07:56

reporter   ~0000435

Last edited: 2014-01-30 07:56

View 2 revisions

Yes, you are right. I have moved it under

   if (null == transformColumn) {

     if (oldSourceValues.containsKey(name)) {

       values.add(oldSourceValues.get(name));
       return;
     }

     values.add(null);
     return;
   }


There's an addition to the last check of unsupported transformations (variable, additive, multiply, lookup, bsh and identity) not to throw the exception until conflict detection type is fetched and compared to USE_OLD_DATA or USE_CHANGED_DATA.

There are few tests added.

chenson

2014-01-30 09:55

administrator   ~0000436

I like your changes. I made some slight modifications and checked in.

chenson

2014-02-03 17:46

administrator   ~0000438

I ended up removing the setting of old values for const and substr transforms. We were running into issues with the values not being applied for updates when we wanted them to (because the old and new values were the same in the data).

Related Changesets

SymmetricDS: master a97e1746

2014-01-30 09:54:54

chenson

Details Diff
0001529: Columns in old data are not transformed causing conflict detection
0001529
mod - symmetric-io/pom.xml Diff File
mod - symmetric-io/src/main/java/org/jumpmind/symmetric/io/data/transform/TransformedData.java Diff File
add - symmetric-io/src/test/java/org/jumpmind/symmetric/io/data/transform/TransformedDataTest.java Diff File
mod - symmetric-parent/pom.xml Diff File

SymmetricDS: master 277da044

2014-02-03 17:44:32

chenson

Details Diff
0001529: Columns in old data are not transformed causing conflict detection

Removed the setting of old data for constants and substr values

0001529
mod - symmetric-io/src/main/java/org/jumpmind/symmetric/io/data/transform/TransformedData.java Diff File
mod - symmetric-io/src/test/java/org/jumpmind/symmetric/io/data/transform/TransformedDataTest.java Diff File

Issue History

Date Modified Username Field Change
2014-01-14 03:45 boris_pavlovic New Issue
2014-01-14 03:58 boris_pavlovic Tag Attached: column transformation
2014-01-14 03:58 boris_pavlovic Tag Attached: conflict detection
2014-01-14 08:02 chenson Note Added: 0000415
2014-01-14 10:12 boris_pavlovic Note Added: 0000416
2014-01-14 20:03 chenson Note Added: 0000417
2014-01-15 03:19 boris_pavlovic Note Added: 0000418
2014-01-15 03:20 boris_pavlovic Note Edited: 0000418 View Revisions
2014-01-15 09:25 boris_pavlovic File Added: bug_fix_for_http___www_symmetricds_org_issues_view_php_id=1529.patch
2014-01-15 09:27 boris_pavlovic Note Added: 0000420
2014-01-24 07:54 chenson Assigned To => chenson
2014-01-24 07:54 chenson Status new => assigned
2014-01-27 03:55 boris_pavlovic Note Added: 0000429
2014-01-27 04:28 boris_pavlovic File Added: bug_fix_for_http___www_symmetricds_org_issues_view_php_id=1530_taking_in_regard_transform_.patch
2014-01-27 06:56 chenson Note Added: 0000430
2014-01-27 10:57 boris_pavlovic Note Added: 0000431
2014-01-27 11:24 boris_pavlovic Note Edited: 0000431 View Revisions
2014-01-28 21:18 chenson Target Version => 3.5.16
2014-01-29 18:41 chenson Note Added: 0000433
2014-01-30 07:49 boris_pavlovic File Added: 20140130_bug_fix_for_http___www_symmetricds_org_issues_view_php_id=1530.patch
2014-01-30 07:56 boris_pavlovic Note Added: 0000435
2014-01-30 07:56 boris_pavlovic Note Edited: 0000435 View Revisions
2014-01-30 09:55 chenson Note Added: 0000436
2014-02-02 20:53 Changeset attached => SymmetricDS trunk r7898
2014-02-03 17:46 chenson Note Added: 0000438
2014-02-03 18:00 Changeset attached => SymmetricDS trunk r7908
2014-02-05 10:07 chenson Status assigned => resolved
2014-02-05 10:07 chenson Fixed in Version => 3.5.16
2014-02-05 10:07 chenson Resolution open => fixed
2014-02-05 10:08 chenson Status resolved => closed
2015-07-30 21:49 chenson Changeset attached => SymmetricDS master 277da044
2015-07-30 21:49 chenson Changeset attached => SymmetricDS master a97e1746
2019-04-12 08:50 admin Tag Renamed column transformation => transformation
2019-04-12 12:37 admin Tag Renamed conflict detection => conflict manager