diff --git a/build.gradle.kts b/build.gradle.kts index ddabb0e83..a95e21751 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -19,7 +19,7 @@ plugins { allprojects { group = "io.flamingock" - val declaredVersion = "1.4.0-SNAPSHOT" + val declaredVersion = "1.4.2-SNAPSHOT" version = VersionManager.resolveVersion(declaredVersion, project.hasProperty("release")) extra["generalUtilVersion"] = "1.5.3" diff --git a/buildSrc/src/main/kotlin/flamingock.release-management.gradle.kts b/buildSrc/src/main/kotlin/flamingock.release-management.gradle.kts index 1f55366de..3be2211cb 100644 --- a/buildSrc/src/main/kotlin/flamingock.release-management.gradle.kts +++ b/buildSrc/src/main/kotlin/flamingock.release-management.gradle.kts @@ -148,6 +148,18 @@ if (isReleasing && projectsToRelease.contains(project.name)) { } } +// Defensive gate against unqualified `./gradlew jreleaserDeploy` fan-out. The org.jreleaser +// plugin is applied to every subproject that consumes this convention plugin, so every one of +// them registers a jreleaserDeploy task. Without this onlyIf, Gradle 8.5's strict input +// validation fails the task on subprojects whose build/jreleaser directory was never primed +// (e.g. flamingock-gradle-plugin, which is published via publishPlugins and has no deploy +// config). Skipping cleanly preserves the script-level qualification fix as belt-and-suspenders. +tasks.matching { it.name == "jreleaserDeploy" }.configureEach { + onlyIf("project not in projectsToRelease") { + project == rootProject || projectsToRelease.contains(project.name) + } +} + // Utility functions fun Project.getIfAlreadyReleasedFromCentralPortal(): Boolean { val mavenUsername: String? = System.getenv("JRELEASER_MAVENCENTRAL_USERNAME") diff --git a/core/flamingock-core-commons/src/main/java/io/flamingock/internal/common/core/metadata/MetadataLoader.java b/core/flamingock-core-commons/src/main/java/io/flamingock/internal/common/core/metadata/MetadataLoader.java index 6f4aaded6..a6b8bb816 100644 --- a/core/flamingock-core-commons/src/main/java/io/flamingock/internal/common/core/metadata/MetadataLoader.java +++ b/core/flamingock-core-commons/src/main/java/io/flamingock/internal/common/core/metadata/MetadataLoader.java @@ -182,10 +182,16 @@ FlamingockMetadata aggregate(List modules) { static final class CompositePipelineBuilder { PreviewPipeline buildFrom(List 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 defaultStages = new ArrayList<>(); List legacyStages = new ArrayList<>(); SystemPreviewStage systemStage = null; @@ -209,22 +215,12 @@ PreviewPipeline buildFrom(List 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 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 collapsedLegacy = collapseLegacyStagesByName(legacyStages); + List collapsedLegacy = collapseStagesByName(legacyStages); + List collapsedDefault = collapseStagesByName(defaultStages); List 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. @@ -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). * - *

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. + *

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 collapseLegacyStagesByName(List legacyStages) { - if (legacyStages.isEmpty()) return new ArrayList<>(); + private static List collapseStagesByName(List stages) { + if (stages.isEmpty()) return new ArrayList<>(); LinkedHashMap> byName = new LinkedHashMap<>(); - for (PreviewStage s : legacyStages) { + for (PreviewStage s : stages) { byName.computeIfAbsent(s.getName(), k -> new ArrayList<>()).add(s); } List result = new ArrayList<>(byName.size()); for (List 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. + * + *

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 sameName) { + private static PreviewStage mergeSameNameStages(List sameName) { PreviewStage first = sameName.get(0); List merged = new ArrayList<>(); Set 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. */ diff --git a/core/flamingock-core-commons/src/test/java/io/flamingock/internal/common/core/metadata/CompositePipelineBuilderTest.java b/core/flamingock-core-commons/src/test/java/io/flamingock/internal/common/core/metadata/CompositePipelineBuilderTest.java new file mode 100644 index 000000000..e3083161b --- /dev/null +++ b/core/flamingock-core-commons/src/test/java/io/flamingock/internal/common/core/metadata/CompositePipelineBuilderTest.java @@ -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 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 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 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 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 stages = new ArrayList<>(composite.getStages()); + assertEquals(2, stages.size(), "Distinct-name stages must remain separate"); + Set 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 changes) { + return metadataWithStage(StageType.DEFAULT, name, sourcesPackage, changes); + } + + private static FlamingockMetadata metadataWithStage(StageType type, + String name, + String sourcesPackage, + List 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 changes(String... ids) { + List result = new ArrayList<>(ids.length); + for (String id : ids) { + CodePreviewChange change = new CodePreviewChange(); + change.setId(id); + result.add(change); + } + return result; + } + + private static Set changeIds(PreviewStage stage) { + if (stage.getChanges() == null) return Collections.emptySet(); + Set ids = new LinkedHashSet<>(); + for (AbstractPreviewChange c : stage.getChanges()) { + ids.add(c.getId()); + } + return ids; + } + + private static Set setOf(String... values) { + return new LinkedHashSet<>(Arrays.asList(values)); + } +} diff --git a/core/flamingock-core/src/main/java/io/flamingock/internal/core/builder/AbstractChangeRunnerBuilder.java b/core/flamingock-core/src/main/java/io/flamingock/internal/core/builder/AbstractChangeRunnerBuilder.java index 08257f822..b0292eb7e 100644 --- a/core/flamingock-core/src/main/java/io/flamingock/internal/core/builder/AbstractChangeRunnerBuilder.java +++ b/core/flamingock-core/src/main/java/io/flamingock/internal/core/builder/AbstractChangeRunnerBuilder.java @@ -51,6 +51,7 @@ import io.flamingock.internal.core.plugin.Plugin; import io.flamingock.internal.core.plugin.PluginManager; import io.flamingock.internal.core.builder.args.FlamingockArguments; +import io.flamingock.internal.core.builder.runner.DisabledRunner; import io.flamingock.internal.core.builder.runner.Runner; import io.flamingock.internal.core.builder.runner.RunnerBuilder; import io.flamingock.internal.core.builder.runner.RunnerFactory; @@ -191,6 +192,15 @@ public HOLDER setApplicationArguments(String[] args) { */ @Override public final Runner build() { + FlamingockArguments flamingockArgs = FlamingockArguments.parse(applicationArgs); + + // Kill switch for the auto-execution path: when `flamingock.enabled=false` and we're + // not in CLI mode, return a no-op runner before any side-effectful setup runs (no + // template scan, no plugin init, no audit-store connection, no lock attempt, no + // pipeline load). Explicitly-invoked CLI commands ignore the flag — user intent wins. + if (!flamingockArgs.isCliMode() && !coreConfiguration.isEnabled()) { + return new DisabledRunner(); + } ChangeTemplateManager.loadTemplates(); pluginManager.initialize(context); @@ -221,8 +231,6 @@ public final Runner build() { pipeline.validate(); pipeline.contributeToContext(hierarchicalContext); - FlamingockArguments flamingockArgs = FlamingockArguments.parse(applicationArgs); - OperationResolver operationResolver = new OperationResolver( runnerId, flamingockArgs, diff --git a/core/flamingock-core/src/main/java/io/flamingock/internal/core/builder/runner/DisabledRunner.java b/core/flamingock-core/src/main/java/io/flamingock/internal/core/builder/runner/DisabledRunner.java new file mode 100644 index 000000000..9f26d2384 --- /dev/null +++ b/core/flamingock-core/src/main/java/io/flamingock/internal/core/builder/runner/DisabledRunner.java @@ -0,0 +1,38 @@ +/* + * 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.core.builder.runner; + +import io.flamingock.internal.util.log.FlamingockLoggerFactory; +import org.slf4j.Logger; + +/** + * No-op {@link Runner} returned by {@code AbstractChangeRunnerBuilder.build()} when + * {@code flamingock.enabled=false} and the build is not in CLI mode. Emits a single + * INFO line and exits. + * + *

By the time the builder returns this runner, none of the change-application + * machinery has been initialised — no template scan, no plugin initialisation, no + * audit-store connection, no lock attempt, no pipeline load. Hence no finalizer is + * needed: nothing was opened, nothing to close. + */ +public final class DisabledRunner implements Runner { + private static final Logger logger = FlamingockLoggerFactory.getLogger("flamingock.runner"); + + @Override + public void run() { + logger.info("Flamingock disabled (flamingock.enabled=false); skipping execution."); + } +} diff --git a/core/flamingock-core/src/test/java/io/flamingock/internal/core/builder/runner/DisabledRunnerTest.java b/core/flamingock-core/src/test/java/io/flamingock/internal/core/builder/runner/DisabledRunnerTest.java new file mode 100644 index 000000000..8073992f4 --- /dev/null +++ b/core/flamingock-core/src/test/java/io/flamingock/internal/core/builder/runner/DisabledRunnerTest.java @@ -0,0 +1,53 @@ +/* + * 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.core.builder.runner; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Unit tests for {@link DisabledRunner}. The runner is returned by + * {@code AbstractChangeRunnerBuilder.build()} when {@code flamingock.enabled=false} and the + * build is not in CLI mode; its sole contract is to log a single line and exit without side + * effects. + */ +class DisabledRunnerTest { + + @Test + @DisplayName("run() returns cleanly without throwing") + void runDoesNotThrow() { + DisabledRunner runner = new DisabledRunner(); + assertDoesNotThrow((org.junit.jupiter.api.function.Executable) runner::run); + } + + @Test + @DisplayName("run() is repeatable (idempotent no-op)") + void runIsIdempotent() { + DisabledRunner runner = new DisabledRunner(); + assertDoesNotThrow((org.junit.jupiter.api.function.Executable) runner::run); + assertDoesNotThrow((org.junit.jupiter.api.function.Executable) runner::run); + } + + @Test + @DisplayName("DisabledRunner is a Runner") + void isARunner() { + assertTrue(new DisabledRunner() instanceof Runner, + "DisabledRunner must implement the Runner contract so AbstractChangeRunnerBuilder.build() can return it"); + } +} diff --git a/flamingock-gradle-plugin/docs/KOTLIN_SUPPORT_FOLLOWUPS.md b/flamingock-gradle-plugin/docs/KOTLIN_SUPPORT_FOLLOWUPS.md new file mode 100644 index 000000000..eeba5c833 --- /dev/null +++ b/flamingock-gradle-plugin/docs/KOTLIN_SUPPORT_FOLLOWUPS.md @@ -0,0 +1,200 @@ +# Kotlin support — deferred follow-ups + +**Status:** Section 1(a) and Section 2 are **implemented** (1.4.2 release). Section 1(b) — the +KSP port of `flamingock-processor` — remains deferred and awaits a scoping issue before +implementation. The original analysis is preserved below as context for future work. + +Surfaced during review of the +[#918](https://github.com/flamingock/flamingock-java/issues/918) Kotlin-support fix (auto-apply +`org.jetbrains.kotlin.kapt` when Kotlin JVM is detected, register `flamingock-processor` under +the `kapt` configuration, harden the reflective Kotlin-compile-task arg wiring). + +Two related concerns were intentionally **not** addressed in the original bug-fix PR so the +patch release stayed focused. They are described here in enough detail that follow-up work +doesn't have to re-derive the analysis. + +--- + +## 1. Kapt auto-apply: opt-out and KSP roadmap + +**Status — item (a) opt-out: implemented in 1.4.2.** Exposed as the Gradle property +`flamingock.autoApplyKapt` (default `true`; opt out with literal case-insensitive `false`). +Implemented as a Gradle property rather than a `flamingock { … }` DSL setting because the +`plugins { }` block evaluates before the `flamingock { }` block — a DSL setter would always +fire too late to influence the auto-apply that already happened during `plugins { }`. See +`docs/get-started/gradle-plugin.md` (Kotlin support section) for the user-facing documentation. + +**Status — item (b) KSP port: still deferred.** Needs its own scoping issue. + +### Current behaviour + +`FlamingockPlugin.apply()` reacts to `org.jetbrains.kotlin.jvm` being present and +unconditionally applies `org.jetbrains.kotlin.kapt` (via `FlamingockConstants.KAPT_PLUGIN_ID`) +unless it is already applied. There is no opt-out. `DependencyConfigurator.configure()` then +adds `flamingock-processor` (and `mongock-support` when Mongock support is enabled) under the +`kapt` configuration alongside `annotationProcessor`. + +### Why this is fine for now + +The plugin's stated positioning is zero-boilerplate (`"id 'io.flamingock'"` and the user is +done). Kotlin users hitting #918 were dropping into raw kapt configuration and getting it +wrong; auto-apply removes that failure mode. As long as `flamingock-processor` ships only as a +`javax.annotation.processing.Processor`, **kapt is the only path** for running it against +Kotlin sources — KSP cannot run a `javax.annotation.processing.Processor` directly. + +### Why it deserves an opt-out + +- **Build cost.** Kapt generates Java stubs from Kotlin sources and runs the AP against the + stubs. This is slower than KSP and historically blocks some Kotlin language features (e.g. + certain inline-class scenarios) from being visible to the AP. +- **Long-term direction.** KSP is Google/JetBrains' recommended replacement for kapt for new + annotation processing in Kotlin projects. Teams that have moved their stack to KSP will + resent kapt being silently force-applied. +- **Composability.** Users who deliberately don't want kapt — for instance, they have their + own kapt setup, or they want to register the processor under `ksp` once that's supported — + have no way to disable the auto-apply today. + +### Deferred items + +**a) Add an extension toggle.** Shape suggestion: + +```kotlin +flamingock { + // Default: "kapt" (current behaviour, no breaking change). + // Future values: "ksp" (once the processor is ported), "none" (caller wires it themselves). + kotlinAnnotationProcessor = "kapt" +} +``` + +A simpler `autoApplyKapt: Boolean` is acceptable if KSP support is genuinely far off; the +three-valued enum is preferred because it makes the eventual KSP migration a value change +rather than an option deprecation. + +Default must remain the current behaviour (`"kapt"` or `autoApplyKapt = true`) so existing +users see no change. + +Implementation touches `FlamingockExtension`, `FlamingockPlugin.apply()` (gate the +`project.plugins.withId(KOTLIN_JVM_PLUGIN_ID) { … }` block on the new value), and +`DependencyConfigurator.configure()` (skip the `kapt` dependency registration when the toggle +is `"none"`). + +**b) KSP port of `flamingock-processor`.** Larger, separate piece of work touching +`core/flamingock-processor/`. Not every javax.annotation.processing API has a direct KSP +equivalent — `RoundDiscovery`, the `Filer`-based incremental cache in `FlamingockMetadataStore`, +and the SPI-file roundtrip in `MetadataModuleIdentity` all need careful porting. Worth a +scoping issue before implementation to enumerate the API gaps. The auto-apply toggle above +should land first so the KSP work has somewhere to plug in. + +--- + +## 2. Mixed Java+Kotlin modules: duplicate same-name default stages + +**Status: implemented in 1.4.2 via the recommended merge-layer dedup.** +`MetadataLoader.CompositePipelineBuilder` now applies the same id-deduplicated same-name +collapse to **default** stages that has always been applied to **legacy** stages. The two +helpers were generalised into `collapseStagesByName` + `mergeSameNameStages`. Identity-field +mismatches (different `type` / `sourcesPackage` / `resourcesDir` on stages sharing a name) +still emit a WARN for default stages so genuine multi-module misconfigurations stay visible; +legacy stages remain silent on mismatch by design. The analysis below is preserved for +context. + +### Mechanism + +When a single module has **both** Java and Kotlin sources annotated with Flamingock, the +processor runs **twice** per build — once under `compileJava` (via the `annotationProcessor` +configuration) and once under `kaptKotlin` (via the `kapt` configuration). The two invocations +happen with **different `CLASS_OUTPUT` directories**: + +- kapt: `build/tmp/kapt3/classes//` +- javac: `build/classes/java//` + +`MetadataModuleIdentity.resolve()` +(`core/flamingock-processor/src/main/java/io/flamingock/core/processor/util/MetadataModuleIdentity.java`, +the `readExistingProviderFqn` path) reads the SPI registration from `CLASS_OUTPUT` to discover +or generate a per-module suffix. Because the two `CLASS_OUTPUT` dirs are disjoint, the two +invocations each find no existing SPI file and each generate their own random 8-hex suffix. +End result: + +- kapt writes `META-INF/services/io.flamingock.internal.common.core.metadata.FlamingockMetadataProvider` + pointing at `…Impl_aaaa1111`, plus `META-INF/flamingock/metadata_aaaa1111.json`. +- javac writes the same SPI filename pointing at `…Impl_bbbb2222`, plus + `META-INF/flamingock/metadata_bbbb2222.json`. + +Both end up in the packaged JAR. At runtime, +`MetadataLoader.loadAggregated()` +(`core/flamingock-core-commons/src/main/java/io/flamingock/internal/common/core/metadata/MetadataLoader.java`) +uses `ServiceLoader` to discover **both** providers. + +### Runtime aggregation behaviour + +In `MetadataLoader.CompositePipelineBuilder.buildFrom()`: + +- **System stages**: `mergeSystem()` id-deduplicates and logs at DEBUG. +- **Legacy stages**: `collapseLegacyStagesByName()` + `mergeSameNameLegacyStages()` group by + name and id-deduplicate the change set per group. +- **Default stages**: the loop in `buildFrom()` adds every default stage from every provider + into `defaultStages`. The duplicate-name detection logs a **WARN** (`"Duplicate stage name + '{}' across modules — proceeding with both."`) but **keeps both copies**. There is no + id-dedup at the default-stage level. + +For the kapt + javac case in a single module, both metadata files contain the **same** +default-stage structure (both derive it from the same `@EnableFlamingock`). Within those +stages, each AP only contributes elements visible to its own compiler (kapt: Kotlin sources; +javac: Java sources). So the composite ends up with the same default stage appearing **twice** +— one populated with the module's Kotlin changes, the other with its Java changes. + +### Why this is messy but not catastrophic + +- **No double-execution of the same change.** kapt's metadata has Kotlin changes, javac's has + Java changes; they don't overlap. +- **Audit and reporting are messy.** The execution report and the audit log will reference the + same stage name twice with different change subsets each time, which diverges from what the + user declared in `@EnableFlamingock` and is confusing to read. +- **A WARN fires every startup.** Currently the WARN is genuinely useful for catching + multi-module configuration mistakes; a fix should keep that signal alive while suppressing + it for the legitimate kapt+javac case. + +### Solution directions + +**(a) Merge-layer dedup (recommended).** Extend `MetadataLoader.CompositePipelineBuilder` to +id-union same-name default stages, mirroring `mergeSameNameLegacyStages`. Concretely: refactor +`mergeSameNameLegacyStages` and `collapseLegacyStagesByName` into a stage-type-agnostic helper +(`collapseStagesByName(List)` returning one collapsed stage per name); call it +for both `defaultStages` and `legacyStages`. Preserve the duplicate-name WARN for the +deduplicated-but-still-suspicious case where the contents weren't fully aligned across modules +(e.g. different `sourcesPackage` on stages of the same name) so genuine misconfigurations are +still surfaced. + +Pros: one fix lives in one place (commons), reuses an existing pattern, robust to any future +multi-provider scenario (not just kapt+javac). Cons: weakens the current invariant that +"default stages with the same name across modules is suspicious" — mitigated by keeping the +WARN for content mismatches. + +**(b) Shared `CLASS_OUTPUT` / shared module identity (rejected).** Make kapt and javac in the +same module reuse the same suffix. Conceptually cleaner — one metadata file end-to-end — but +the build-side mechanics are fragile: Gradle's task ordering across compilers, incremental +recompilation scenarios, and clean-build orderings all become surface area for this to drift. +The merge-layer fix achieves the same end result without that fragility. + +### Verification fixture + +A test fixture under `flamingock-gradle-plugin/src/test/resources/` (or as an integration test +under `core/flamingock-core-commons/`) exercising a module with at least one `@Change` in Java +and one in Kotlin, both under the same `@EnableFlamingock` stage. Assertions: + +1. Build succeeds, producing two `META-INF/flamingock/metadata_*.json` files in the JAR (this + is expected — the fix is at the aggregator, not the build). +2. After `MetadataLoader.loadAggregated()`, the composite pipeline contains exactly one + default stage with that name, and its change list contains both changes (one from each + language). +3. No `"Duplicate stage name … proceeding with both."` WARN is emitted when the contents are + compatible. + +--- + +## Tracking + +- Section 1(a) and Section 2 — implemented in 1.4.2. No further tracking issues needed. +- Section 1(b) — KSP port of `flamingock-processor`. Still requires a scoping issue before + implementation; not every `javax.annotation.processing` API has a direct KSP equivalent + and `RoundDiscovery` / the `Filer`-based incremental cache need substantive rework. diff --git a/flamingock-gradle-plugin/src/main/kotlin/io/flamingock/gradle/FlamingockPlugin.kt b/flamingock-gradle-plugin/src/main/kotlin/io/flamingock/gradle/FlamingockPlugin.kt index def6d48f8..70e7f6c12 100644 --- a/flamingock-gradle-plugin/src/main/kotlin/io/flamingock/gradle/FlamingockPlugin.kt +++ b/flamingock-gradle-plugin/src/main/kotlin/io/flamingock/gradle/FlamingockPlugin.kt @@ -45,12 +45,30 @@ import org.gradle.api.Project */ class FlamingockPlugin : Plugin { + companion object { + private const val KOTLIN_JVM_PLUGIN_ID = "org.jetbrains.kotlin.jvm" + } + override fun apply(project: Project) { // Apply java plugin if not already applied if (!project.plugins.hasPlugin("java")) { project.plugins.apply("java") } + // Kotlin projects need KAPT so the Flamingock annotation processor runs on Kotlin + // sources. Apply it automatically to preserve the plugin's zero-boilerplate intent. + // Users who want to manage kapt themselves (or who plan to migrate to KSP) can opt + // out via -Pflamingock.autoApplyKapt=false. The opt-out has to be a Gradle property + // and not a `flamingock { … }` DSL setting because the `plugins { }` block (where + // the user applies kotlin.jvm) evaluates before the `flamingock { }` block, so any + // DSL setter would fire too late to influence the auto-apply that already happened. + project.plugins.withId(KOTLIN_JVM_PLUGIN_ID) { + if (shouldAutoApplyKapt(project) + && !project.plugins.hasPlugin(FlamingockConstants.KAPT_PLUGIN_ID)) { + project.pluginManager.apply(FlamingockConstants.KAPT_PLUGIN_ID) + } + } + // Create and register the extension val extension = project.extensions.create( FlamingockConstants.EXTENSION_NAME, @@ -74,6 +92,19 @@ class FlamingockPlugin : Plugin { } } + /** + * Decides whether the plugin should auto-apply `org.jetbrains.kotlin.kapt`. Defaults to + * `true`; users opt out by setting [FlamingockConstants.AUTO_APPLY_KAPT_PROPERTY] to a + * value equal (case-insensitively) to `"false"`. Any other value — including unset — + * preserves the auto-apply behaviour. Read at `withId`-fire time so the decision is + * available before kapt would otherwise be applied. Visible to the same module so tests + * can exercise the property handling directly. + */ + internal fun shouldAutoApplyKapt(project: Project): Boolean { + val raw = project.findProperty(FlamingockConstants.AUTO_APPLY_KAPT_PROPERTY) ?: return true + return !raw.toString().equals("false", ignoreCase = true) + } + private fun validateConfiguration(extension: FlamingockExtension) { if (extension.isCommunityEnabled && extension.isCloudEnabled) { throw GradleException( diff --git a/flamingock-gradle-plugin/src/main/kotlin/io/flamingock/gradle/internal/CompilerArgsConfigurator.kt b/flamingock-gradle-plugin/src/main/kotlin/io/flamingock/gradle/internal/CompilerArgsConfigurator.kt index e944b9150..26e249bec 100644 --- a/flamingock-gradle-plugin/src/main/kotlin/io/flamingock/gradle/internal/CompilerArgsConfigurator.kt +++ b/flamingock-gradle-plugin/src/main/kotlin/io/flamingock/gradle/internal/CompilerArgsConfigurator.kt @@ -23,6 +23,8 @@ import org.gradle.api.tasks.SourceSet import org.gradle.api.tasks.SourceSetContainer import org.gradle.api.tasks.compile.JavaCompile import java.io.File +import java.lang.reflect.Array +import java.lang.reflect.Method /** * Passes Gradle's authoritative source/resource roots to the Flamingock annotation processor @@ -52,11 +54,7 @@ import java.io.File */ internal object CompilerArgsConfigurator { - private const val SOURCES_OPTION = "flamingock.sources" - private const val RESOURCES_OPTION = "flamingock.resources" - private const val KAPT_PLUGIN_ID = "org.jetbrains.kotlin.kapt" - private const val KSP_PLUGIN_ID = "com.google.devtools.ksp" fun configure(project: Project) { val sourceSets = project.extensions.findByType(SourceSetContainer::class.java) ?: return @@ -70,9 +68,9 @@ internal object CompilerArgsConfigurator { project.tasks.named(ss.compileJavaTaskName, JavaCompile::class.java) .configure(Action { - options.compilerArgs.add("-A$SOURCES_OPTION=$sourcesArg") + options.compilerArgs.add("-A${FlamingockConstants.SOURCES_OPTION}=$sourcesArg") if (resourcesArg != null) { - options.compilerArgs.add("-A$RESOURCES_OPTION=$resourcesArg") + options.compilerArgs.add("-A${FlamingockConstants.RESOURCES_OPTION}=$resourcesArg") } }) @@ -81,10 +79,10 @@ internal object CompilerArgsConfigurator { // plugins.withId so the configuration kicks in regardless of plugin-application // order; if a plugin is never applied, the callback never fires. if (ss.name == SourceSet.MAIN_SOURCE_SET_NAME) { - project.plugins.withId(KAPT_PLUGIN_ID) { + project.plugins.withId(FlamingockConstants.KAPT_PLUGIN_ID) { configureKapt(project, ss) } - project.plugins.withId(KSP_PLUGIN_ID) { + project.plugins.withId(FlamingockConstants.KSP_PLUGIN_ID) { configureKsp(project, ss) } } @@ -126,10 +124,11 @@ internal object CompilerArgsConfigurator { && it.parameterCount == 1 && Function1::class.java.isAssignableFrom(it.parameterTypes[0]) } ?: return + argumentsMethod.ensureAccessible() val action: (Any) -> Unit = { argsObj -> - invokeArg(argsObj, SOURCES_OPTION, sourcesArg) - if (resourcesArg != null) invokeArg(argsObj, RESOURCES_OPTION, resourcesArg) + invokeArg(argsObj, FlamingockConstants.SOURCES_OPTION, sourcesArg) + if (resourcesArg != null) invokeArg(argsObj, FlamingockConstants.RESOURCES_OPTION, resourcesArg) } argumentsMethod.invoke(kaptExt, action) } @@ -143,8 +142,8 @@ internal object CompilerArgsConfigurator { val resourcesArg = ss.resources.srcDirs.firstOrNull()?.absolutePath val kspExt = project.extensions.findByName("ksp") ?: return - invokeArg(kspExt, SOURCES_OPTION, sourcesArg) - if (resourcesArg != null) invokeArg(kspExt, RESOURCES_OPTION, resourcesArg) + invokeArg(kspExt, FlamingockConstants.SOURCES_OPTION, sourcesArg) + if (resourcesArg != null) invokeArg(kspExt, FlamingockConstants.RESOURCES_OPTION, resourcesArg) } private fun sourcesArgFor(ss: SourceSet): String? { @@ -166,6 +165,7 @@ internal object CompilerArgsConfigurator { && it.parameterTypes[1] == String::class.java } if (stringString != null) { + stringString.ensureAccessible() stringString.invoke(target, name, value) return } @@ -174,7 +174,23 @@ internal object CompilerArgsConfigurator { && it.parameterTypes[1].isArray } if (vararg != null) { - vararg.invoke(target, name, arrayOf(value)) + vararg.ensureAccessible() + vararg.invoke(target, name, singleElementArray(vararg.parameterTypes[1], value)) + } + } + + private fun Method.ensureAccessible() { + isAccessible = true + } + + // Visible for testing: the helper bridges Kotlin's `arrayOf(value)` (which produces + // `Object[]`) and the reflective `vararg.invoke(...)` call against a method whose array + // parameter has a more specific component type (typically `String[]`). Wrong array type + // here is exactly the silent regression the test guards against. + internal fun singleElementArray(arrayType: Class<*>, value: String): Any { + val componentType = arrayType.componentType ?: return arrayOf(value) + return Array.newInstance(componentType, 1).also { array -> + Array.set(array, 0, value) } } } diff --git a/flamingock-gradle-plugin/src/main/kotlin/io/flamingock/gradle/internal/DependencyConfigurator.kt b/flamingock-gradle-plugin/src/main/kotlin/io/flamingock/gradle/internal/DependencyConfigurator.kt index f5e9a3307..529770c84 100644 --- a/flamingock-gradle-plugin/src/main/kotlin/io/flamingock/gradle/internal/DependencyConfigurator.kt +++ b/flamingock-gradle-plugin/src/main/kotlin/io/flamingock/gradle/internal/DependencyConfigurator.kt @@ -26,12 +26,20 @@ internal object DependencyConfigurator { fun configure(project: Project, extension: FlamingockExtension, version: String) { val group = FlamingockConstants.GROUP val dependencies = project.dependencies + val kaptEnabled = project.plugins.hasPlugin(FlamingockConstants.KAPT_PLUGIN_ID) + || project.configurations.findByName("kapt") != null // Always add the annotation processor dependencies.add( "annotationProcessor", "$group:flamingock-processor:$version" ) + if (kaptEnabled) { + dependencies.add( + "kapt", + "$group:flamingock-processor:$version" + ) + } // Always add BOM for version management dependencies.add( @@ -62,6 +70,12 @@ internal object DependencyConfigurator { "annotationProcessor", "$group:mongock-support:$version" ) + if (kaptEnabled) { + dependencies.add( + "kapt", + "$group:mongock-support:$version" + ) + } } // Spring Boot integration diff --git a/flamingock-gradle-plugin/src/main/kotlin/io/flamingock/gradle/internal/FlamingockConstants.kt b/flamingock-gradle-plugin/src/main/kotlin/io/flamingock/gradle/internal/FlamingockConstants.kt index 5e1f546f6..58e229141 100644 --- a/flamingock-gradle-plugin/src/main/kotlin/io/flamingock/gradle/internal/FlamingockConstants.kt +++ b/flamingock-gradle-plugin/src/main/kotlin/io/flamingock/gradle/internal/FlamingockConstants.kt @@ -22,6 +22,20 @@ internal object FlamingockConstants { const val GROUP = "io.flamingock" const val EXTENSION_NAME = "flamingock" + const val SOURCES_OPTION = "flamingock.sources" + const val RESOURCES_OPTION = "flamingock.resources" + + const val KAPT_PLUGIN_ID = "org.jetbrains.kotlin.kapt" + const val KSP_PLUGIN_ID = "com.google.devtools.ksp" + + /** + * Gradle property that, when set to `false`, suppresses the auto-application of + * `org.jetbrains.kotlin.kapt` in Kotlin projects. Default behaviour (property unset or + * any value other than literal `false`, case-insensitive) is to auto-apply kapt so the + * Flamingock annotation processor runs against Kotlin sources. + */ + const val AUTO_APPLY_KAPT_PROPERTY = "flamingock.autoApplyKapt" + val FLAMINGOCK_VERSION: String by lazy { FlamingockConstants::class.java.classLoader .getResourceAsStream("flamingock-plugin.properties") diff --git a/flamingock-gradle-plugin/src/test/kotlin/io/flamingock/gradle/FlamingockPluginTest.kt b/flamingock-gradle-plugin/src/test/kotlin/io/flamingock/gradle/FlamingockPluginTest.kt new file mode 100644 index 000000000..7736c9838 --- /dev/null +++ b/flamingock-gradle-plugin/src/test/kotlin/io/flamingock/gradle/FlamingockPluginTest.kt @@ -0,0 +1,179 @@ +/* + * 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.gradle + +import io.flamingock.gradle.internal.FlamingockConstants +import org.gradle.api.internal.project.ProjectInternal +import org.gradle.testfixtures.ProjectBuilder +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertNotNull +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir +import java.nio.file.Path + +/** + * Unit tests for [FlamingockPlugin] using Gradle's `ProjectBuilder` test fixture. + * + *

Scope intentionally avoids pulling the Kotlin Gradle Plugin onto the test classpath + * (the existing {@code CompilerArgsConfiguratorTest} avoids it for the same reason). Tests + * therefore verify the wiring through {@link DependencyConfigurator}'s fallback path — + * `project.configurations.findByName("kapt") != null` — which is the same branch the real + * `org.jetbrains.kotlin.kapt` plugin triggers at apply time. The positive + * "kotlin-jvm auto-applies kapt" case is best covered by a Gradle TestKit functional test + * if/when stronger guarantees are needed; visual review of the five-line + * {@code plugins.withId(KOTLIN_JVM_PLUGIN_ID) { ... }} block in + * {@link FlamingockPlugin#apply} suffices for now. + */ +class FlamingockPluginTest { + + @TempDir + lateinit var projectDir: Path + + @Test + fun `pure-Java project does not auto-apply kotlin kapt`() { + val project = ProjectBuilder.builder().withProjectDir(projectDir.toFile()).build() + + project.plugins.apply(FlamingockPlugin::class.java) + (project as ProjectInternal).evaluate() + + assertFalse( + project.plugins.hasPlugin(FlamingockConstants.KAPT_PLUGIN_ID), + "Java-only project must not auto-apply org.jetbrains.kotlin.kapt" + ) + assertNull( + project.configurations.findByName("kapt"), + "Java-only project must not have a kapt configuration" + ) + } + + @Test + fun `pure-Java project registers flamingock-processor under annotationProcessor only`() { + val project = ProjectBuilder.builder().withProjectDir(projectDir.toFile()).build() + + project.plugins.apply(FlamingockPlugin::class.java) + (project as ProjectInternal).evaluate() + + val ap = project.configurations.getByName("annotationProcessor").allDependencies + .map { "${it.group}:${it.name}" }.toSet() + assertTrue( + ap.contains("io.flamingock:flamingock-processor"), + "annotationProcessor must include flamingock-processor. Actual: $ap" + ) + // No kapt configuration means no kapt-side registration. Already asserted above; this + // test is the positive complement to keep the failure mode explicit. + assertNull( + project.configurations.findByName("kapt"), + "Java-only project must not have a kapt configuration" + ) + } + + @Test + fun `when a kapt configuration is present, DependencyConfigurator also registers flamingock-processor under kapt`() { + // Simulate a project where kapt is "enabled" without pulling the Kotlin Gradle Plugin + // onto the test classpath: pre-create the `kapt` configuration. DependencyConfigurator + // then takes its fallback `findByName("kapt") != null` branch — the same branch the + // real kotlin.kapt plugin's apply triggers. + val project = ProjectBuilder.builder().withProjectDir(projectDir.toFile()).build() + project.configurations.create("kapt") + + project.plugins.apply(FlamingockPlugin::class.java) + (project as ProjectInternal).evaluate() + + val kapt = project.configurations.getByName("kapt").allDependencies + .map { "${it.group}:${it.name}" }.toSet() + assertTrue( + kapt.contains("io.flamingock:flamingock-processor"), + "kapt configuration must include flamingock-processor when kapt is enabled. Actual: $kapt" + ) + // annotationProcessor still receives it too — the fix adds to kapt, doesn't remove + // from annotationProcessor. + val ap = project.configurations.getByName("annotationProcessor").allDependencies + .map { "${it.group}:${it.name}" }.toSet() + assertTrue( + ap.contains("io.flamingock:flamingock-processor"), + "annotationProcessor must still include flamingock-processor even when kapt is wired. Actual: $ap" + ) + } + + @Test + fun `shouldAutoApplyKapt returns true when no property is set`() { + // The opt-out is read at withId-fire time via Gradle's findProperty. With no value + // anywhere (gradle.properties, -P, or env), the helper returns true so kapt is + // auto-applied — preserving the 1.4.1 default behaviour. + val project = ProjectBuilder.builder().withProjectDir(projectDir.toFile()).build() + val plugin = FlamingockPlugin() + + assertTrue( + plugin.shouldAutoApplyKapt(project), + "Default (property unset) must auto-apply kapt" + ) + } + + @Test + fun `shouldAutoApplyKapt returns false when flamingock autoApplyKapt is set to false`() { + // Simulates a user setting flamingock.autoApplyKapt=false in gradle.properties or + // via -Pflamingock.autoApplyKapt=false. ProjectBuilder lets us seed extra properties + // via the project's ExtraPropertiesExtension; the same lookup path Gradle uses for + // -P / gradle.properties resolves through findProperty(). + val project = ProjectBuilder.builder().withProjectDir(projectDir.toFile()).build() + project.extensions.extraProperties.set(FlamingockConstants.AUTO_APPLY_KAPT_PROPERTY, "false") + val plugin = FlamingockPlugin() + + assertFalse( + plugin.shouldAutoApplyKapt(project), + "Explicit 'false' must opt out of kapt auto-apply" + ) + } + + @Test + fun `shouldAutoApplyKapt treats false-string case-insensitively, other values default to true`() { + // Case variants of "false" all opt out; anything else (including "no", "0", "FALSE!", + // arbitrary strings) preserves the default. This avoids surprising the user with + // values that look like opt-out but aren't (e.g. "no" stays on so the user sees the + // auto-apply happen and learns the canonical property name from docs). + val plugin = FlamingockPlugin() + + listOf("false", "False", "FALSE").forEach { v -> + val p = ProjectBuilder.builder().withProjectDir(projectDir.toFile()).build() + p.extensions.extraProperties.set(FlamingockConstants.AUTO_APPLY_KAPT_PROPERTY, v) + assertFalse(plugin.shouldAutoApplyKapt(p), "'$v' must opt out") + } + listOf("true", "True", "TRUE", "yes", "no", "0", "1", "").forEach { v -> + val p = ProjectBuilder.builder().withProjectDir(projectDir.toFile()).build() + p.extensions.extraProperties.set(FlamingockConstants.AUTO_APPLY_KAPT_PROPERTY, v) + assertTrue( + plugin.shouldAutoApplyKapt(p), + "'$v' must not opt out — only literal case-insensitive 'false' suppresses auto-apply" + ) + } + } + + @Test + fun `applying the plugin creates the flamingock extension`() { + val project = ProjectBuilder.builder().withProjectDir(projectDir.toFile()).build() + + project.plugins.apply(FlamingockPlugin::class.java) + + // Extension is registered at apply-time (not afterEvaluate); checked without + // triggering evaluate. + assertNotNull( + project.extensions.findByName(FlamingockConstants.EXTENSION_NAME), + "Applying the plugin must register the '${FlamingockConstants.EXTENSION_NAME}' extension" + ) + } +} diff --git a/flamingock-gradle-plugin/src/test/kotlin/io/flamingock/gradle/internal/CompilerArgsConfiguratorTest.kt b/flamingock-gradle-plugin/src/test/kotlin/io/flamingock/gradle/internal/CompilerArgsConfiguratorTest.kt index 7af07b3bc..978ddb0ed 100644 --- a/flamingock-gradle-plugin/src/test/kotlin/io/flamingock/gradle/internal/CompilerArgsConfiguratorTest.kt +++ b/flamingock-gradle-plugin/src/test/kotlin/io/flamingock/gradle/internal/CompilerArgsConfiguratorTest.kt @@ -198,6 +198,53 @@ class CompilerArgsConfiguratorTest { args.firstOrNull { it.startsWith("-Aflamingock.sources=") } ?.removePrefix("-Aflamingock.sources=") + @Test + fun `singleElementArray returns a typed array matching the requested component type`() { + // The original bug: `arrayOf(value)` produces `Object[]`, which fails reflection + // invocation against a `String[]`-typed vararg parameter (the typical case in real + // Kotlin/Gradle plugin signatures). The helper must build an array whose component + // type matches the parameter so reflection can dispatch. + val stringArray = CompilerArgsConfigurator.singleElementArray(Array::class.java, "v") + assertTrue(stringArray is Array<*>, "Helper must return an array") + assertEquals(String::class.java, stringArray.javaClass.componentType, + "Component type must be String, got ${stringArray.javaClass.componentType}") + assertEquals(1, (stringArray as Array<*>).size, "Array must contain exactly one element") + assertEquals("v", stringArray[0]) + } + + @Test + fun `singleElementArray falls back to Object array when componentType is null`() { + // Defensive path for the rare case where the reflected parameter type does not report + // a componentType (e.g. `Object` declared as the array slot). Keeps the helper safe + // against degenerate signatures rather than NPE'ing. + val fallback = CompilerArgsConfigurator.singleElementArray(Object::class.java, "v") + assertTrue(fallback is Array<*>, "Fallback must still produce an array") + assertEquals(1, (fallback as Array<*>).size) + assertEquals("v", fallback[0]) + } + + @Test + fun `configureKapt reflectively dispatches against a String-typed vararg parameter`() { + // Regression guard for the precise signature shape that motivated the fix: a stub + // KaptArguments whose `arg(name, vararg values: String)` declares the values as + // `String[]` at the JVM level — which is what real Kotlin libraries typically expose. + // Before the fix, `arrayOf(value)` would throw IllegalArgumentException on the + // reflective invocation here because the actual argument was an `Object[]`. + val project = ProjectBuilder.builder().withProjectDir(projectDir.toFile()).build() + project.plugins.apply("java") + + val kapt = StubKaptExtensionWithStringVararg() + project.extensions.add(StubKaptExtensionWithStringVararg::class.java, "kapt", kapt) + + val main = project.extensions.getByType(SourceSetContainer::class.java).getByName("main") + CompilerArgsConfigurator.configureKapt(project, main) + + assertEquals(srcDir(project, "main", "java"), + kapt.arguments.collected["flamingock.sources"]) + assertEquals(srcDir(project, "main", "resources"), + kapt.arguments.collected["flamingock.resources"]) + } + private fun resourcesArg(args: List): String? = args.firstOrNull { it.startsWith("-Aflamingock.resources=") } ?.removePrefix("-Aflamingock.resources=") @@ -243,4 +290,24 @@ class CompilerArgsConfiguratorTest { collected[k] = v } } + + /** + * Like [StubKaptArguments] but with the vararg parameter typed `String...` (i.e. JVM + * signature `String[]`) instead of `Object...`. This is the more demanding shape that the + * production reflective dispatch has to support — it's the precise reason the + * `singleElementArray` helper exists. + */ + open class StubKaptArgumentsWithStringVararg { + val collected = mutableMapOf() + fun arg(name: Any, vararg values: String) { + collected[name.toString()] = values.joinToString(" ") + } + } + + open class StubKaptExtensionWithStringVararg { + val arguments = StubKaptArgumentsWithStringVararg() + fun arguments(action: StubKaptArgumentsWithStringVararg.() -> Unit) { + arguments.action() + } + } } diff --git a/flamingock-gradle-plugin/src/test/kotlin/io/flamingock/gradle/internal/DependencyConfiguratorTest.kt b/flamingock-gradle-plugin/src/test/kotlin/io/flamingock/gradle/internal/DependencyConfiguratorTest.kt new file mode 100644 index 000000000..5025103b6 --- /dev/null +++ b/flamingock-gradle-plugin/src/test/kotlin/io/flamingock/gradle/internal/DependencyConfiguratorTest.kt @@ -0,0 +1,89 @@ +/* + * 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.gradle.internal + +import io.flamingock.gradle.FlamingockExtension +import org.gradle.testfixtures.ProjectBuilder +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir +import java.nio.file.Path + +/** + * Unit tests for [DependencyConfigurator] focused on the `kaptEnabled` branch — specifically + * the `mongock-support` registration under the `kapt` configuration that the [#918] fix added. + * + *

The Kotlin Gradle Plugin is intentionally not on the test classpath. `kaptEnabled` is + * driven via the fallback `findByName("kapt") != null` path, by pre-creating the configuration + * — the same observable state the real `org.jetbrains.kotlin.kapt` plugin produces. + */ +class DependencyConfiguratorTest { + + @TempDir + lateinit var projectDir: Path + + private val version = "TEST-VERSION" + private val mongockSupport = "io.flamingock:mongock-support" + + @Test + fun `mongock-support is added to kapt when both kapt is enabled and mongock support is requested`() { + val project = ProjectBuilder.builder().withProjectDir(projectDir.toFile()).build() + project.plugins.apply("java") + project.configurations.create("kapt") + val extension = FlamingockExtension().apply { mongock() } + + DependencyConfigurator.configure(project, extension, version) + + val kapt = project.configurations.getByName("kapt").allDependencies + .map { "${it.group}:${it.name}" }.toSet() + assertTrue( + kapt.contains(mongockSupport), + "kapt configuration must include mongock-support when kapt + mongock support are both on. Actual: $kapt" + ) + // The annotationProcessor side also gets it; verifying both keeps the contract explicit. + val ap = project.configurations.getByName("annotationProcessor").allDependencies + .map { "${it.group}:${it.name}" }.toSet() + assertTrue( + ap.contains(mongockSupport), + "annotationProcessor must still include mongock-support alongside the kapt registration. Actual: $ap" + ) + } + + @Test + fun `mongock-support is NOT added to kapt when kapt is enabled but mongock support is not requested`() { + val project = ProjectBuilder.builder().withProjectDir(projectDir.toFile()).build() + project.plugins.apply("java") + project.configurations.create("kapt") + val extension = FlamingockExtension() // mongock() NOT called + + DependencyConfigurator.configure(project, extension, version) + + val kapt = project.configurations.getByName("kapt").allDependencies + .map { "${it.group}:${it.name}" }.toSet() + assertFalse( + kapt.contains(mongockSupport), + "kapt configuration must NOT include mongock-support when mongock is disabled. Actual: $kapt" + ) + // mongock-support also must not appear on annotationProcessor when mongock is disabled. + val ap = project.configurations.getByName("annotationProcessor").allDependencies + .map { "${it.group}:${it.name}" }.toSet() + assertFalse( + ap.contains(mongockSupport), + "annotationProcessor must NOT include mongock-support when mongock is disabled. Actual: $ap" + ) + } +} diff --git a/infra/bundle-release-with-retry.sh b/infra/bundle-release-with-retry.sh index e065b06fb..74e4c14a0 100755 --- a/infra/bundle-release-with-retry.sh +++ b/infra/bundle-release-with-retry.sh @@ -20,7 +20,11 @@ validate_bundle "$1" echo "Releasing bundle[$1] to Central Portal with max attempts[$maxAttempts] and $waitingSeconds seconds delay" for (( i=1; i<=maxAttempts; i++ )); do - if ./gradlew jreleaserFullRelease -Pmodule="$1" --no-daemon --stacktrace; then + # A bundle spans multiple projects, so we deliberately use the unqualified task name and + # let the release-management convention plugin's projectsToRelease (driven by -PreleaseBundle) + # decide which projects participate. The onlyIf gate inside that plugin skips projects + # outside the bundle cleanly, so the unqualified fan-out is safe. + if ./gradlew jreleaserFullRelease -PreleaseBundle="$1" --no-daemon --stacktrace; then exit 0 fi if [ "$i" -eq "$maxAttempts" ]; then diff --git a/infra/module-release-with-retry.sh b/infra/module-release-with-retry.sh index 87646ea5a..8cd3acddc 100755 --- a/infra/module-release-with-retry.sh +++ b/infra/module-release-with-retry.sh @@ -13,6 +13,9 @@ else MODULE_FLAG="" echo "Releasing bundle to Central Portal with max attempts[$maxAttempts] and $waitingSeconds seconds delay" fi +# The unqualified jreleaserDeploy fans out to every subproject that has the org.jreleaser +# plugin applied (it's applied broadly by flamingock.release-management). The release-management +# convention plugin's onlyIf gate skips projects outside projectsToRelease so the fan-out is safe. for (( i=1; i<=maxAttempts; i++ )); do if ./gradlew jreleaserDeploy $MODULE_FLAG $extraFlags --no-daemon --stacktrace; then exit 0