Skip to content

feat(kotlin): kapt opt-out and default-stage dedup across modules (#921) #922

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dieppa wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat(kotlin): kapt opt-out and default-stage dedup across modules (#921) #922

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ plugins {

allprojects {
group = "io.flamingock"
val declaredVersion = "1.4.1-SNAPSHOT"
val declaredVersion = "1.4.2-SNAPSHOT"
version = VersionManager.resolveVersion(declaredVersion, project.hasProperty("release"))

extra["generalUtilVersion"] = "1.5.3"
Expand Down
87 changes: 56 additions & 31 deletions ...ore-commons/src/main/java/io/flamingock/internal/common/core/metadata/MetadataLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,16 @@ FlamingockMetadata aggregate(List<FlamingockMetadata> modules) {
static final class CompositePipelineBuilder {
PreviewPipeline buildFrom(List<FlamingockMetadata> modules) {
// Stage assembly preserves stage instances by reference — no defensive copies.
// Default stages are kept as-is (duplicate names across modules surface a warning
// because that's likely a configuration mistake). Legacy stages are merged
// group-by-name via collapseLegacyStagesByName; same-name legacy stages across
// modules are merged silently because that's the documented design.
// Stages with the same name across modules are collapsed group-by-name via
// collapseStagesByName for BOTH legacy and default stages. The collapse
// id-deduplicates the change set across the group; this is what makes the
// kapt + javac scenario in a mixed Java+Kotlin module produce one default stage
// with the union of changes rather than two side-by-side. Identity-field
// mismatches (different type / sourcesPackage / resourcesDir on stages that
// share a name) still surface a WARN for default stages so genuine multi-module
// misconfigurations stay visible; legacy stages stay silent because historical
// Mongock setups frequently have multiple modules legitimately producing the
// same legacy stage.
List<PreviewStage> defaultStages = new ArrayList<>();
List<PreviewStage> legacyStages = new ArrayList<>();
SystemPreviewStage systemStage = null;
Expand All @@ -209,22 +215,12 @@ PreviewPipeline buildFrom(List<FlamingockMetadata> modules) {
}
}

// Warn on duplicate names among DEFAULT stages only — for LEGACY, collapsing
// same-name stages is the intended outcome and would otherwise surface false
// positives in any project with multiple Mongock-using modules.
Set<String> seenDefaultNames = new HashSet<>();
for (PreviewStage s : defaultStages) {
if (!seenDefaultNames.add(s.getName())) {
logger.warn("Duplicate stage name '{}' across modules — proceeding with both.",
s.getName());
}
}

List<PreviewStage> collapsedLegacy = collapseLegacyStagesByName(legacyStages);
List<PreviewStage> collapsedLegacy = collapseStagesByName(legacyStages);
List<PreviewStage> collapsedDefault = collapseStagesByName(defaultStages);

List<PreviewStage> allStages = new ArrayList<>();
allStages.addAll(collapsedLegacy);
allStages.addAll(defaultStages);
allStages.addAll(collapsedDefault);

// Surface the rare case where the system stage name clashes with any
// resolved (post-legacy-collapse) stage name in the composite.
Expand Down Expand Up @@ -269,54 +265,83 @@ private static SystemPreviewStage mergeSystem(SystemPreviewStage soFar, PreviewS
}

/**
* Group legacy stages by name and merge each group independently. Returns one
* {@link PreviewStage} per distinct legacy-stage name, in first-seen order across
* modules (deterministic for a given module discovery order).
* Group stages by name and merge each group independently. Returns one
* {@link PreviewStage} per distinct stage name, in first-seen order across modules
* (deterministic for a given module discovery order).
*
* <p>Stages with different names stay separate — historically Mongock was the only
* producer (always {@code flamingock-legacy-stage}), but a future legacy source can
* declare its own stage name and is preserved as a peer alongside Mongock's.
* <p>Type-agnostic: handles both legacy and default stages with the same id-dedup
* semantics. Same-name groups across modules are merged silently for LEGACY stages
* (historical Mongock setups frequently have multiple modules producing the same
* legacy stage) and with an identity-field mismatch WARN for DEFAULT stages (so the
* "two modules declared 'mainStage' with different sourcesPackage" misconfiguration
* stays visible).
*/
private static List<PreviewStage> collapseLegacyStagesByName(List<PreviewStage> legacyStages) {
if (legacyStages.isEmpty()) return new ArrayList<>();
private static List<PreviewStage> collapseStagesByName(List<PreviewStage> stages) {
if (stages.isEmpty()) return new ArrayList<>();
LinkedHashMap<String, List<PreviewStage>> byName = new LinkedHashMap<>();
for (PreviewStage s : legacyStages) {
for (PreviewStage s : stages) {
byName.computeIfAbsent(s.getName(), k -> new ArrayList<>()).add(s);
}
List<PreviewStage> result = new ArrayList<>(byName.size());
for (List<PreviewStage> group : byName.values()) {
result.add(group.size() == 1 ? group.get(0) : mergeSameNameLegacyStages(group));
result.add(group.size() == 1 ? group.get(0) : mergeSameNameStages(group));
}
return result;
}

/**
* id-deduplicated union of changes for legacy stages that already share a name. The
* first stage's name/description/sourcesPackage/resourcesDir wins; subsequent stages
* id-deduplicated union of changes for stages that already share a name. The first
* stage's name/type/description/sourcesPackage/resourcesDir wins; subsequent stages
* contribute only changes whose ids haven't been seen yet.
*
* <p>Emits a WARN on identity-field mismatch (different {@code type},
* {@code sourcesPackage}, or {@code resourcesDir}) for DEFAULT stages — this is
* almost certainly a configuration mistake worth surfacing. LEGACY stages stay
* silent on mismatch by design: existing Mongock-using modules sometimes
* legitimately diverge on these fields without it being a real problem.
*/
private static PreviewStage mergeSameNameLegacyStages(List<PreviewStage> sameName) {
private static PreviewStage mergeSameNameStages(List<PreviewStage> sameName) {
PreviewStage first = sameName.get(0);
List<AbstractPreviewChange> merged = new ArrayList<>();
Set<String> seenIds = new HashSet<>();
if (first.getChanges() != null) {
first.getChanges().forEach(c -> { merged.add(c); seenIds.add(c.getId()); });
}
boolean identityMismatch = false;
for (int i = 1; i < sameName.size(); i++) {
PreviewStage extra = sameName.get(i);
if (!identityMismatch && hasIdentityMismatch(first, extra)) {
identityMismatch = true;
}
if (extra.getChanges() == null) continue;
for (AbstractPreviewChange c : extra.getChanges()) {
if (seenIds.add(c.getId())) {
merged.add(c);
} else {
logger.debug("Deduplicated legacy change id '{}' in stage '{}' (already contributed by an earlier module)",
logger.debug("Deduplicated change id '{}' in stage '{}' (already contributed by an earlier module)",
c.getId(), first.getName());
}
}
}
if (identityMismatch && first.getType() != StageType.LEGACY) {
logger.warn("Stage '{}' is declared by multiple modules with mismatched identity (type / sourcesPackage / resourcesDir). "
+ "Merging change sets; first-seen identity wins — verify your @EnableFlamingock declarations.",
first.getName());
}
return new PreviewStage(first.getName(), first.getType(), first.getDescription(),
first.getSourcesPackage(), first.getResourcesDir(), merged);
}

/**
* Two same-name stages have a real configuration mismatch when their type,
* sourcesPackage, or resourcesDir differ. Description is excluded — it's free-text
* and can reasonably differ across modules without indicating a problem.
*/
private static boolean hasIdentityMismatch(PreviewStage a, PreviewStage b) {
return a.getType() != b.getType()
|| !java.util.Objects.equals(a.getSourcesPackage(), b.getSourcesPackage())
|| !java.util.Objects.equals(a.getResourcesDir(), b.getResourcesDir());
}
}

/** Union map; later modules win on key clash. */
Expand Down
202 changes: 202 additions & 0 deletions ...c/test/java/io/flamingock/internal/common/core/metadata/CompositePipelineBuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
/*
* Copyright 2026 Flamingock (https://www.flamingock.io)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.flamingock.internal.common.core.metadata;

import io.flamingock.api.StageType;
import io.flamingock.internal.common.core.preview.AbstractPreviewChange;
import io.flamingock.internal.common.core.preview.CodePreviewChange;
import io.flamingock.internal.common.core.preview.PreviewPipeline;
import io.flamingock.internal.common.core.preview.PreviewStage;
import io.flamingock.internal.common.core.preview.SystemPreviewStage;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;

/**
* Unit tests for {@code MetadataLoader.CompositePipelineBuilder}, focused on the
* stage-collapse logic that was generalised to cover default stages (in addition to legacy
* stages) so that mixed Java + Kotlin modules — where kapt and javac each produce their own
* metadata provider with the same default stages — converge on a single default stage in the
* composite instead of two side-by-side entries.
*/
class CompositePipelineBuilderTest {

@Test
@DisplayName("same-name default stages across modules collapse into one with merged change list")
void defaultStagesWithSameNameAreCollapsed() {
// Mirrors the kapt + javac scenario: two providers, same default stage, disjoint
// changes (kapt contributed the Kotlin changes, javac contributed the Java ones).
FlamingockMetadata moduleKapt = metadataWithDefaultStage(
"main-stage", "com.example", changes("KotlinChangeA", "KotlinChangeB"));
FlamingockMetadata moduleJavac = metadataWithDefaultStage(
"main-stage", "com.example", changes("JavaChangeC"));

PreviewPipeline composite = new MetadataLoader.CompositePipelineBuilder()
.buildFrom(Arrays.asList(moduleKapt, moduleJavac));

List<PreviewStage> stages = new ArrayList<>(composite.getStages());
assertEquals(1, stages.size(),
"Two providers contributing the same default stage must yield exactly one stage in the composite");
PreviewStage merged = stages.get(0);
assertEquals("main-stage", merged.getName());
assertEquals(StageType.DEFAULT, merged.getType());

Set<String> changeIds = changeIds(merged);
assertEquals(setOf("KotlinChangeA", "KotlinChangeB", "JavaChangeC"), changeIds,
"Merged stage must contain the id-union of both providers' changes");
}

@Test
@DisplayName("same-name default stages with different sourcesPackage still collapse (first identity wins)")
void defaultStagesWithMismatchedIdentityStillCollapse() {
// The mismatch case: two modules declared a default stage with the same name but
// different sourcesPackage. We still merge — the alternative (keep both stages) was
// worse for any real use case — and first-seen identity wins. The behaviour change
// also emits a WARN at runtime; we don't assert on logging here (no in-module log
// capture infrastructure) and rely on code review for the WARN content.
FlamingockMetadata moduleA = metadataWithDefaultStage(
"shared-name", "com.example.a", changes("A1"));
FlamingockMetadata moduleB = metadataWithDefaultStage(
"shared-name", "com.example.b", changes("B1"));

PreviewPipeline composite = new MetadataLoader.CompositePipelineBuilder()
.buildFrom(Arrays.asList(moduleA, moduleB));

List<PreviewStage> stages = new ArrayList<>(composite.getStages());
assertEquals(1, stages.size(),
"Mismatched-identity same-name stages must still collapse to a single stage");
PreviewStage merged = stages.get(0);
assertEquals("com.example.a", merged.getSourcesPackage(),
"First-seen sourcesPackage must win on identity-mismatch");
assertEquals(setOf("A1", "B1"), changeIds(merged),
"Change sets must still be merged despite the identity mismatch");
}

@Test
@DisplayName("legacy stage collapse behaviour is preserved by the refactor")
void legacyStagesStillCollapse() {
// Regression guard: the legacy-stage collapse used to live in
// mergeSameNameLegacyStages; after generalisation it routes through the same
// mergeSameNameStages path. Verify same-name legacy stages still merge with id-dedup.
FlamingockMetadata m1 = metadataWithStage(StageType.LEGACY,
"flamingock-legacy-stage", null, changes("L1", "L2"));
FlamingockMetadata m2 = metadataWithStage(StageType.LEGACY,
"flamingock-legacy-stage", null, changes("L2", "L3"));

PreviewPipeline composite = new MetadataLoader.CompositePipelineBuilder()
.buildFrom(Arrays.asList(m1, m2));

List<PreviewStage> stages = new ArrayList<>(composite.getStages());
assertEquals(1, stages.size());
PreviewStage merged = stages.get(0);
assertEquals(StageType.LEGACY, merged.getType());
assertEquals(setOf("L1", "L2", "L3"), changeIds(merged),
"Duplicate legacy change ids across modules must be id-deduplicated");
}

@Test
@DisplayName("system stage id-dedup behaviour is preserved (separate code path, regression guard)")
void systemStageDedupUnchanged() {
// The system-stage merger (CompositePipelineBuilder#mergeSystem) is a different path
// from collapseStagesByName and is unchanged by this refactor. Sanity-check it.
SystemPreviewStage sysA = new SystemPreviewStage("system-stage", "desc", null, null,
new ArrayList<>(changes("S1", "S2")));
SystemPreviewStage sysB = new SystemPreviewStage("system-stage", "desc", null, null,
new ArrayList<>(changes("S2", "S3")));
FlamingockMetadata m1 = new FlamingockMetadata();
m1.setPipeline(new PreviewPipeline(sysA, Collections.emptyList()));
FlamingockMetadata m2 = new FlamingockMetadata();
m2.setPipeline(new PreviewPipeline(sysB, Collections.emptyList()));

PreviewPipeline composite = new MetadataLoader.CompositePipelineBuilder()
.buildFrom(Arrays.asList(m1, m2));

PreviewStage system = composite.getSystemStage();
assertNotNull(system, "Composite must carry a system stage when contributors had one");
assertEquals(setOf("S1", "S2", "S3"), changeIds(system),
"Duplicate system-stage change ids across modules must be id-deduplicated");
}

@Test
@DisplayName("default stages with distinct names remain distinct (sanity: not over-collapsing)")
void distinctNameDefaultStagesAreKeptSeparate() {
// Guard against the lazy implementation that would collapse everything into one
// group. Distinct names must stay distinct.
FlamingockMetadata m1 = metadataWithDefaultStage("alpha", "com.alpha", changes("a1"));
FlamingockMetadata m2 = metadataWithDefaultStage("beta", "com.beta", changes("b1"));

PreviewPipeline composite = new MetadataLoader.CompositePipelineBuilder()
.buildFrom(Arrays.asList(m1, m2));

List<PreviewStage> stages = new ArrayList<>(composite.getStages());
assertEquals(2, stages.size(), "Distinct-name stages must remain separate");
Set<String> names = stages.stream().map(PreviewStage::getName).collect(Collectors.toSet());
assertEquals(setOf("alpha", "beta"), names);
}

// ----------------------------------------------------------------------
// Fixture helpers
// ----------------------------------------------------------------------

private static FlamingockMetadata metadataWithDefaultStage(String name,
String sourcesPackage,
List<AbstractPreviewChange> changes) {
return metadataWithStage(StageType.DEFAULT, name, sourcesPackage, changes);
}

private static FlamingockMetadata metadataWithStage(StageType type,
String name,
String sourcesPackage,
List<AbstractPreviewChange> changes) {
PreviewStage stage = new PreviewStage(name, type, null, sourcesPackage, null, changes);
FlamingockMetadata md = new FlamingockMetadata();
md.setPipeline(new PreviewPipeline(Collections.singletonList(stage)));
return md;
}

private static List<AbstractPreviewChange> changes(String... ids) {
List<AbstractPreviewChange> result = new ArrayList<>(ids.length);
for (String id : ids) {
CodePreviewChange change = new CodePreviewChange();
change.setId(id);
result.add(change);
}
return result;
}

private static Set<String> changeIds(PreviewStage stage) {
if (stage.getChanges() == null) return Collections.emptySet();
Set<String> ids = new LinkedHashSet<>();
for (AbstractPreviewChange c : stage.getChanges()) {
ids.add(c.getId());
}
return ids;
}

private static Set<String> setOf(String... values) {
return new LinkedHashSet<>(Arrays.asList(values));
}
}
Loading
Loading