View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0001529 | SymmetricDS | Bug | public | 2014-01-14 08:45 | 2014-02-05 15:08 |
Reporter | boris_pavlovic | Assigned To | chenson | ||
Priority | urgent | ||||
Status | closed | Resolution | fixed | ||
Product Version | 3.5.11 | ||||
Target Version | 3.5.16 | Fixed in Version | 3.5.16 | ||
Summary | 0001529: Columns in old data are not transformed causing conflict detection | ||||
Description | two 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 Reproduce | debugging 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 | ||||
Tags | conflict manager, transformation | ||||
|
This has already been fixed in 3.5.12 |
|
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 |
|
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? |
|
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. |
|
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)); + } } } } |
|
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 |
|
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. |
|
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" + ); + } + } |
|
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. |
|
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 |
|
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 ... |
|
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)); + } +} |
|
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. |
|
I like your changes. I made some slight modifications and checked in. |
|
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). |
SymmetricDS: master a97e1746 2014-01-30 09:54:54 Details Diff |
0001529: Columns in old data are not transformed causing conflict detection |
Affected Issues 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 Details Diff |
0001529: Columns in old data are not transformed causing conflict detection Removed the setting of old data for constants and substr values |
Affected Issues 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 |
Date Modified | Username | Field | Change |
---|---|---|---|
2014-01-14 08:45 | boris_pavlovic | New Issue | |
2014-01-14 08:58 | boris_pavlovic | Tag Attached: column transformation | |
2014-01-14 08:58 | boris_pavlovic | Tag Attached: conflict detection | |
2014-01-14 13:02 | chenson | Note Added: 0000415 | |
2014-01-14 15:12 | boris_pavlovic | Note Added: 0000416 | |
2014-01-15 01:03 | chenson | Note Added: 0000417 | |
2014-01-15 08:19 | boris_pavlovic | Note Added: 0000418 | |
2014-01-15 08:20 | boris_pavlovic | Note Edited: 0000418 | View Revisions |
2014-01-15 14:25 | boris_pavlovic | File Added: bug_fix_for_http___www_symmetricds_org_issues_view_php_id=1529.patch | |
2014-01-15 14:27 | boris_pavlovic | Note Added: 0000420 | |
2014-01-24 12:54 | chenson | Assigned To | => chenson |
2014-01-24 12:54 | chenson | Status | new => assigned |
2014-01-27 08:55 | boris_pavlovic | Note Added: 0000429 | |
2014-01-27 09: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 11:56 | chenson | Note Added: 0000430 | |
2014-01-27 15:57 | boris_pavlovic | Note Added: 0000431 | |
2014-01-27 16:24 | boris_pavlovic | Note Edited: 0000431 | View Revisions |
2014-01-29 02:18 | chenson | Target Version | => 3.5.16 |
2014-01-29 23:41 | chenson | Note Added: 0000433 | |
2014-01-30 12:49 | boris_pavlovic | File Added: 20140130_bug_fix_for_http___www_symmetricds_org_issues_view_php_id=1530.patch | |
2014-01-30 12:56 | boris_pavlovic | Note Added: 0000435 | |
2014-01-30 12:56 | boris_pavlovic | Note Edited: 0000435 | View Revisions |
2014-01-30 14:55 | chenson | Note Added: 0000436 | |
2014-02-03 01:53 | Changeset attached | => SymmetricDS trunk r7898 | |
2014-02-03 22:46 | chenson | Note Added: 0000438 | |
2014-02-03 23:00 | Changeset attached | => SymmetricDS trunk r7908 | |
2014-02-05 15:07 | chenson | Status | assigned => resolved |
2014-02-05 15:07 | chenson | Fixed in Version | => 3.5.16 |
2014-02-05 15:07 | chenson | Resolution | open => fixed |
2014-02-05 15:08 | chenson | Status | resolved => closed |
2015-07-31 01:49 | chenson | Changeset attached | => SymmetricDS master 277da044 |
2015-07-31 01:49 | chenson | Changeset attached | => SymmetricDS master a97e1746 |
2019-04-12 12:50 | admin | Tag Renamed | column transformation => transformation |
2019-04-12 16:37 | admin | Tag Renamed | conflict detection => conflict manager |