Skip to content

Commit 320c1d8

Browse files
committed
fix: fix use of EntityMatch and condition for multiple candidates
- DefaultSchemaMigration.entityReplacement now returns Optional<EntityMatch> (return type was mismatched with interface despite @CompileStatic due to Groovy generic erasure; would ClassCast at runtime on any call site using .map(EntityMatch::getMatch)) - UnmigratedCell returns Optional.empty() when lookup misses, rather than wrapping a null match inside EntityMatch.of() - AbstractMergeCellMigrator: fix two multipleCandidates flags that used size() > 0 (always true) instead of size() > 1 - minor fixes ING-5131
1 parent a1dc79d commit 320c1d8

8 files changed

Lines changed: 24 additions & 22 deletions

File tree

common/plugins/eu.esdihumboldt.hale.common.align.merge.test/src/eu/esdihumboldt/hale/common/align/merge/test/impl/DefaultMergeCellMigratorTest.groovy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,7 @@ class DefaultMergeCellMigratorTest extends AbstractMergeCellMigratorTest {
460460
private void filterCheckNull(Cell migrated) {
461461
JaxbAlignmentIO.printCell(migrated, System.out)
462462

463-
// the condition should be present on the source
463+
// the condition should be absent on the source
464464
def source = CellUtil.getFirstEntity(migrated.source).definition
465465
def filter = source.propertyPath.empty ? source.filter : source.propertyPath[0].condition?.filter
466466
assertNull(filter)

common/plugins/eu.esdihumboldt.hale.common.align.merge.test/src/eu/esdihumboldt/hale/common/align/merge/test/impl/JoinRetainConditionsTest.groovy

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,12 @@ class JoinRetainConditionsTest extends AbstractMergeCellMigratorTest {
6363
assertEquals(expectedFilterA, filter.filterTerm)
6464
}
6565
else if (e.definition.definition.displayName == 'B') {
66-
// expect filter part to have been propagated to A
66+
// expect filter part to have been propagated to B
6767
assertNotNull(filter)
6868
assertEquals(expectedFilterB, filter.filterTerm)
6969
}
7070
else if (e.definition.definition.displayName == 'C') {
71-
// expect filter part to have been propagated to A
71+
// expect filter part to have been propagated to C
7272
assertNotNull(filter)
7373
assertEquals(expectedFilterC, filter.filterTerm)
7474
}

common/plugins/eu.esdihumboldt.hale.common.align.merge/src/eu/esdihumboldt/hale/common/align/merge/impl/AbstractMergeCellMigrator.java

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -361,12 +361,12 @@ else if (newSource.size() == 1) {
361361
// try to transfer contexts
362362
Entity singleSource = CellUtil.getFirstEntity(newSource);
363363
if (singleSource != null) {
364-
EntityDefinition transferedSource = AbstractMigration.translateContexts(
364+
EntityDefinition transferredSource = AbstractMigration.translateContexts(
365365
original, EntityMatch.of(singleSource.getDefinition()), migration,
366366
null, log).getMatch();
367367
ListMultimap<String, Entity> s = ArrayListMultimap.create();
368368
s.put(newSource.keySet().iterator().next(),
369-
AlignmentUtil.createEntity(transferedSource));
369+
AlignmentUtil.createEntity(transferredSource));
370370
newCell.setSource(s);
371371
}
372372
}
@@ -440,14 +440,14 @@ public Entity apply(Entity input) {
440440
TypeDefinition inputType = input.getDefinition().getType();
441441
if (input.getDefinition().getPropertyPath().isEmpty()
442442
&& joinTypes.contains(inputType)) {
443-
EntityDefinition transferedSource = AbstractMigration
443+
EntityDefinition transferredSource = AbstractMigration
444444
.translateContexts(originalSource.getDefinition(),
445445
new EntityMatch(input.getDefinition(),
446-
joinTypes.size() > 0, true),
446+
joinTypes.size() > 1, true),
447447
migration, inputType, log)
448448
.getMatch();
449-
joinTypeFilters.put(inputType, transferedSource.getFilter());
450-
return AlignmentUtil.createEntity(transferedSource);
449+
joinTypeFilters.put(inputType, transferredSource.getFilter());
450+
return AlignmentUtil.createEntity(transferredSource);
451451
}
452452
else {
453453
return input;
@@ -539,7 +539,7 @@ private boolean applySourceContextsToJoinFocus(MutableCell newCell, Entity origi
539539
TypeEntityDefinition focus = joinConfig.getTypes().iterator().next();
540540
AtomicReference<Filter> focusFilter = new AtomicReference<>();
541541

542-
boolean multipleSources = newCell.getSource().size() > 0;
542+
boolean multipleSources = newCell.getSource().size() > 1;
543543

544544
// transfer context
545545
newCell.setSource(ArrayListMultimap.create(Multimaps.transformValues(newCell.getSource(),
@@ -549,14 +549,14 @@ private boolean applySourceContextsToJoinFocus(MutableCell newCell, Entity origi
549549
public Entity apply(Entity input) {
550550
if (input.getDefinition().getPropertyPath().isEmpty()
551551
&& input.getDefinition().getType().equals(focus.getType())) {
552-
EntityDefinition transferedSource = AbstractMigration
552+
EntityDefinition transferredSource = AbstractMigration
553553
.translateContexts(originalSource.getDefinition(),
554554
new EntityMatch(input.getDefinition(), multipleSources,
555555
true),
556556
migration, focus.getType(), log)
557557
.getMatch();
558-
focusFilter.set(transferedSource.getFilter());
559-
return AlignmentUtil.createEntity(transferedSource);
558+
focusFilter.set(transferredSource.getFilter());
559+
return AlignmentUtil.createEntity(transferredSource);
560560
}
561561
else {
562562
return input;

common/plugins/eu.esdihumboldt.hale.common.align.merge/src/eu/esdihumboldt/hale/common/align/merge/impl/AbstractMigration.groovy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ abstract class AbstractMigration implements AlignmentMigration {
5656
if (!matchedEntity.isPresent()) {
5757
matchedEntity = findParentMatch(defaultEntity, preferRoot)
5858
if (matchedEntity.present) {
59-
log.warn "Inaccurate match of $entity to ${matchedEntity.get()} via parent entity"
59+
log.warn "Inaccurate match of $entity to ${matchedEntity.get().match} via parent entity"
6060
}
6161
}
6262

common/plugins/eu.esdihumboldt.hale.common.align.merge/src/eu/esdihumboldt/hale/common/align/merge/impl/DefaultSchemaMigration.groovy

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import javax.xml.namespace.QName
2020

2121
import eu.esdihumboldt.hale.common.align.groovy.accessor.EntityAccessor
2222
import eu.esdihumboldt.hale.common.align.migrate.AlignmentMigration
23+
import eu.esdihumboldt.hale.common.align.migrate.EntityMatch
2324
import eu.esdihumboldt.hale.common.align.model.ChildContext
2425
import eu.esdihumboldt.hale.common.align.model.EntityDefinition
2526
import eu.esdihumboldt.hale.common.align.model.impl.PropertyEntityDefinition
@@ -44,7 +45,7 @@ class DefaultSchemaMigration implements AlignmentMigration {
4445
}
4546

4647
@Override
47-
public Optional<EntityDefinition> entityReplacement(EntityDefinition entity, TypeDefinition preferRoot, SimpleLog log) {
48+
public Optional<EntityMatch> entityReplacement(EntityDefinition entity, TypeDefinition preferRoot, SimpleLog log) {
4849

4950
// default behavior - try to find entity in new schema, based on names w/o namespace
5051

@@ -60,7 +61,7 @@ class DefaultSchemaMigration implements AlignmentMigration {
6061

6162
if (entity.propertyPath.empty) {
6263
// type entity definition - yield
63-
return Optional.<EntityDefinition>of(typeEntity)
64+
return Optional.of(EntityMatch.of(typeEntity))
6465
}
6566
else {
6667
// property entity definition -> follow the path
@@ -95,7 +96,8 @@ class DefaultSchemaMigration implements AlignmentMigration {
9596
return Optional.empty()
9697
}
9798
else {
98-
return Optional.ofNullable(applyContexts(candidate, entity))
99+
EntityDefinition resolved = applyContexts(candidate, entity)
100+
return resolved == null ? Optional.empty() : Optional.of(EntityMatch.of(resolved))
99101
}
100102
}
101103
}

common/plugins/eu.esdihumboldt.hale.common.align/src/eu/esdihumboldt/hale/common/align/migrate/impl/UnmigratedCell.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,8 @@ public MutableCell migrate(Map<EntityDefinition, EntityDefinition> additionalMap
8585
@Override
8686
public Optional<EntityMatch> entityReplacement(EntityDefinition entity,
8787
TypeDefinition preferRoot, SimpleLog log) {
88-
return Optional
89-
.ofNullable(new EntityMatch(joinedMappings.get(entity), false, false));
88+
EntityDefinition mapped = joinedMappings.get(entity);
89+
return mapped == null ? Optional.empty() : Optional.of(EntityMatch.of(mapped));
9090
}
9191

9292
@Override

common/plugins/eu.esdihumboldt.hale.common.filter/src/eu/esdihumboldt/hale/common/filter/AbstractGeotoolsFilter.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ public Optional<eu.esdihumboldt.hale.common.instance.model.Filter> migrateFilter
242242
messagePrefix, toFilterTerm(part), splitType);
243243
} catch (CQLException e) {
244244
log.error(
245-
"{0}A filter operand part of the filter's {1} condition was removed because no matches for the respective properties were found; error converting filter part to string",
245+
"{0}A filter operand part of the filter''s {1} condition was removed because no matches for the respective properties were found; error converting filter part to string",
246246
messagePrefix, splitType, e);
247247
}
248248
}
@@ -262,7 +262,7 @@ messagePrefix, toFilterTerm(part), focusType.getDisplayName(),
262262
splitType);
263263
} catch (CQLException e) {
264264
log.error(
265-
"{0}A filter operand part of the filter's {2} condition contains references related to other types than {1}; error converting filter part to string",
265+
"{0}A filter operand part of the filter''s {2} condition contains references related to other types than {1}; error converting filter part to string",
266266
messagePrefix, focusType.getDisplayName(), splitType, e);
267267
}
268268
}

common/plugins/eu.esdihumboldt.hale.common.filter/src/eu/esdihumboldt/hale/common/filter/internal/EntityReplacementVisitor.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ private String toPropertyName(EntityDefinition entityDefinition) {
112112
}
113113

114114
/**
115-
* @return if none the attempted replacements matched.
115+
* @return true if none of the attempted replacements matched.
116116
*/
117117
public boolean isAllMismatches() {
118118
return total > 0 && matched == 0;

0 commit comments

Comments
 (0)