Skip to content

[MODFQMMGR-1034] Handle entity type inheritance when migrating queries, custom entities - #1144

Merged
ncovercash merged 26 commits into
masterfrom
modfqmmgr-1034p2
Jan 30, 2026
Merged

[MODFQMMGR-1034] Handle entity type inheritance when migrating queries, custom entities#1144
ncovercash merged 26 commits into
masterfrom
modfqmmgr-1034p2

Conversation

@ncovercash

@ncovercash ncovercash commented Jan 28, 2026

Copy link
Copy Markdown
Member

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

  • Migration logic now takes a Map<UUID, Map<String, UUID>> of source mappings (basically et.getSources().filter(is entity source).toMap(alias, targetId)) which will be used to iteratively attempt migration on each level of nesting:
    • For example, an entity with a field like wrapping_entity.instance.id would 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; and
      • entityTypeId=simple-instance-id, fieldPrefix="wrapping_entity.instance.", field=id.
    • This unwrapping will continue until either something was done (field/op/value change, warning emitted, etc) or there is nothing left to unwrap;
    • This allows migration logic to only worry about the simplest entityTypeId and field themselves without worrying about parent structure.
  • These source mappings are defined as part of the migration (as, if we're defining that composite_instances has source instance pointing to simple_instances, there is no guarantee that this source will later have the same target, alias, or even exist altogether;
  • For custom entity types, these source mappings are generated based on their current state at the time of migration.
    • This does present the drawback that changes to custom entity types which modify the source entity's ID and/or alias` (either due to the user or FQM changes) will lose their ability to have old queries migrated, however, the cases of FQM causing this should be very limited;
      • IMO this is a reasonable drawback and far below the "best effort" I initially thought we'd have
    • These mappings are stored in the DB for optimization purposes (we don't want to recompute this for every migration) using a dedicated table with a single value (there's no need to use DB indexes/etc when we just grab the whole blob)
  • Custom entity types will be themselves migrated on tenant install, see the MD doc file for the most details about that.

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).

V23UserCreatedUpdatedDateFieldDeprecation defines it just for testing/example purposes.

@ncovercash ncovercash changed the title Modfqmmgr 1034p2 [MODFQMMGR-1034] Resolve composite entity type sources when migrating queries Jan 28, 2026
@ncovercash ncovercash changed the title [MODFQMMGR-1034] Resolve composite entity type sources when migrating queries [MODFQMMGR-1034] Handle entity type inheritance when migrating queries, custom fields Jan 28, 2026
@ncovercash ncovercash changed the title [MODFQMMGR-1034] Handle entity type inheritance when migrating queries, custom fields [MODFQMMGR-1034] Handle entity type inheritance when migrating queries, custom entities Jan 30, 2026
@CheckForNull String fqlQuery,
List<String> fields,
@Singular List<Warning> warnings,
String version,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was never used?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that I know of

Comment on lines -141 to +168
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
);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +187 to +200
/**
* 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!
*/

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This traversal is a little counter-intuitive at first, but handles every case I could think of

Comment on lines +280 to +282
migrateEntitySources(et, strategy, updatedMappings, warnings);
migrateEntityGroupByFields(et, strategy, updatedMappings, warnings);
migrateEntityDefaultSort(et, strategy, updatedMappings, warnings);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these methods are incredibly granular, however, it makes testing easier so 🤷

Comment on lines +156 to +172
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();
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

laziest way to test a traversal 🤣

@ncovercash
ncovercash marked this pull request as ready for review January 30, 2026 14:58
@ncovercash
ncovercash requested a review from a team as a code owner January 30, 2026 14:58
Comment thread docs/Migration.md Outdated
@ncovercash
ncovercash enabled auto-merge (squash) January 30, 2026 15:51
@ncovercash
ncovercash merged commit 6b50d17 into master Jan 30, 2026
15 checks passed
@ncovercash
ncovercash deleted the modfqmmgr-1034p2 branch January 30, 2026 15:54
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants