[MODFQMMGR-1034] Handle entity type inheritance when migrating queries, custom entities - #1144
Conversation
…er into modfqmmgr-1034p2
| @CheckForNull String fqlQuery, | ||
| List<String> fields, | ||
| @Singular List<Warning> warnings, | ||
| String version, |
| BiPredicate<F, SingleFieldMigrationResult<F>> didModify | ||
| BiPredicate<F, SingleFieldMigrationResult<F>> didModify, | ||
| Map<UUID, Map<String, UUID>> sourceMappings | ||
| ) { | ||
| SingleFieldMigrationResult<F> transformed = handler.apply(original); | ||
| int fieldDelimiterIndex = original.field().indexOf('.'); | ||
|
|
||
| // stub for follow-up ticket | ||
| didModify.test(original, transformed); | ||
| if (didModify.test(original, transformed) || fieldDelimiterIndex == -1) { | ||
| return transformed; | ||
| } | ||
|
|
||
| return transformed; | ||
| Map<String, UUID> sourceMap = sourceMappings.get(original.entityTypeId()); | ||
| String source = original.field().substring(0, fieldDelimiterIndex); | ||
| String remainder = original.field().substring(fieldDelimiterIndex + 1); | ||
| if (sourceMap == null || sourceMap.get(source) == null) { | ||
| return transformed; | ||
| } | ||
|
|
||
| return handleSingleFieldWithNesting( | ||
| original.dereferenced(sourceMap.get(source), source, remainder), | ||
| handler, | ||
| didModify, | ||
| sourceMappings | ||
| ); |
There was a problem hiding this comment.
glad I did all the migration refactoring in that previous PR — this is nearly all the logic necessary for the actual composite traversal
|
|
||
| @Repository | ||
| @Log4j2 | ||
| public class CustomEntityTypeMigrationMappingRepository { |
There was a problem hiding this comment.
Basically a key/value store with a single row and the only key mapping, mirrors the idea of mod-lists's latest_migrated_version. We might update this later with more stuff, but for now it's just this.
| /** | ||
| * We must traverse the custom entity types from the outermost-in, to ensure source | ||
| * references work as we traverse. If A -> B -> simple_instance, both A and B may refer | ||
| * to fields inside simple_instance. However, if simple_instance changes (e.g. entity ID change) | ||
| * and B is migrated before A, B's migration will change how simple_instance is referred to, | ||
| * breaking the ability for A's migration to "know" that simple_instance is being referred to. | ||
| * | ||
| * By doing this from the outermost entities inwards, we can ensure that earlier migrations will | ||
| * not break future ones. | ||
| * | ||
| * Traversal as a DFS is a bit easier, though, so we do that to build a queue inside-out; | ||
| * since we know children appear before parents in this queue, we can also know parents will | ||
| * appear after children. Flip it et voilà, we've got a parent-first queue without too much complexity! | ||
| */ |
There was a problem hiding this comment.
This traversal is a little counter-intuitive at first, but handles every case I could think of
| migrateEntitySources(et, strategy, updatedMappings, warnings); | ||
| migrateEntityGroupByFields(et, strategy, updatedMappings, warnings); | ||
| migrateEntityDefaultSort(et, strategy, updatedMappings, warnings); |
There was a problem hiding this comment.
All of these methods are incredibly granular, however, it makes testing easier so 🤷
| static List<Arguments> migrationOrderTestCases() { | ||
| // ensure we try every possible case here (only 24). for relationships between these, | ||
| // see comments in consuming test. | ||
| Iterator<List<String>> permutations = new PermutationIterator<>( | ||
| List.of( | ||
| "aaaaaaaa-fb77-5995-8ead-8b1efd81fd10", | ||
| "bbbbbbbb-7b59-522f-9b85-7208291def17", | ||
| "cccccccc-56d1-573d-9639-3b006ed8f953", | ||
| "dddddddd-5523-5c2b-8372-3c3787ed9c23" | ||
| ) | ||
| ); | ||
|
|
||
| return StreamSupport | ||
| .stream(Spliterators.spliteratorUnknownSize(permutations, Spliterator.ORDERED), false) | ||
| .map(Arguments::of) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
laziest way to test a traversal 🤣
|



Purpose
Custom entity types need love, too! (And this explicitly breaks the assumption used for earlier migrations that we'd only need to define changes in terms of the public, composite entities).
Approach
Map<UUID, Map<String, UUID>>of source mappings (basicallyet.getSources().filter(is entity source).toMap(alias, targetId)) which will be used to iteratively attempt migration on each level of nesting:wrapping_entity.instance.idwould result in migration being attempted on each of:entityTypeId=base,fieldPrefix="",field=wrapping_entity.instance.id;entityTypeId=wrapping-id,fieldPrefix="wrapping_entity.",field=instance.id; andentityTypeId=simple-instance-id,fieldPrefix="wrapping_entity.instance.",field=id.entityTypeIdandfieldthemselves without worrying about parent structure.composite_instanceshas sourceinstancepointing tosimple_instances, there is no guarantee that this source will later have the same target, alias, or even exist altogether;Why don't we need to update old migrations to support composites like this?
Custom entity types will debut in Trillium, so there's no chance (outside of dev envs) that custom entities would need to be migrated (and we've been providing simples on a "use at your own risk" philosophy).
V23UserCreatedUpdatedDateFieldDeprecationdefines it just for testing/example purposes.