Skip to content

feat: Phase 8 cleanup and documentation#4241

Merged
evanchooly merged 12 commits intomasterfrom
phase-8-cleanup
May 1, 2026
Merged

feat: Phase 8 cleanup and documentation#4241
evanchooly merged 12 commits intomasterfrom
phase-8-cleanup

Conversation

@evanchooly
Copy link
Copy Markdown
Member

Summary

  • Removes the critter-integration-tests module — superseded by running core tests with -Dmorphia.mapper=critter
  • Updates CLAUDE.md to reflect critter's integration into morphia-core, adds morphia.mapper build commands, removes stale critter-core references
  • Documents the morphia.mapper config option as a comment in the test morphia-config.properties

Closes #4190

Test plan

  • Full build passes: ./mvnw clean install -Ddeploy.skip=true -Dinvoker.skip=true -DskipTests
  • Core tests green (1240 tests, 0 failures): ./mvnw test -pl :morphia-core -Ddeploy.skip=true

🤖 Generated with Claude Code

evanchooly and others added 10 commits April 29, 2026 22:37
- Remove critter-integration-tests module (superseded by -Dmorphia.mapper=critter on core tests)
- Update CLAUDE.md: reflect critter integration into morphia-core, add morphia.mapper build commands, remove stale critter-core references
- Document morphia.mapper config option in test morphia-config.properties

Closes #4190

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 1, 2026 01:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR finalizes “Phase 8” of the critter integration work by removing the now-redundant critter-integration-tests module and updating project documentation and plugin naming to reflect running core tests with -Dmorphia.mapper=critter.

Changes:

  • Removes critter-integration-tests from the critter build.
  • Updates Morphia docs to describe morphia.mapper selection and adds a new critter-maven plugin page.
  • Renames critter-maven goals (generategenerate-models, sourcesgenerate-criteria) and updates related tests/ITs/docs.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
docs/modules/ROOT/pages/maven-plugin.adoc Adds new documentation page for the critter-maven plugin and AOT model generation.
docs/modules/ROOT/pages/configuration.adoc Updates configuration docs and documents morphia.mapper selection.
docs/modules/ROOT/nav.adoc Adds the new Maven Plugin page to the docs navigation.
critter/pom.xml Removes critter-integration-tests from the critter subprojects list.
critter/critter-maven/src/test/kotlin/dev/morphia/critter/maven/CritterSourceProcessorTest.kt Updates tests for removed parameters and new config-based package selection.
critter/critter-maven/src/test/kotlin/dev/morphia/critter/maven/CritterProcessorTest.kt Updates tests for removed packages argument.
critter/critter-maven/src/main/kotlin/dev/morphia/critter/maven/CritterSourcesMojo.kt Renames the sources mojo goal and refactors source root/config loading behavior.
critter/critter-maven/src/main/kotlin/dev/morphia/critter/maven/CritterSourceProcessor.kt Removes explicit packages/criteriaPackage inputs and relies on MorphiaConfig.
critter/critter-maven/src/main/kotlin/dev/morphia/critter/maven/CritterProcessor.kt Removes explicit packages input and relies on MorphiaConfig for scanning.
critter/critter-maven/src/main/kotlin/dev/morphia/critter/maven/CritterMojo.kt Renames main mojo goal to generate-models and removes test/packages configuration.
critter/critter-maven/src/it/simple-test/pom.xml Updates invoker IT to call the renamed generate-models goal.
critter/critter-maven/src/it/generation-test/pom.xml Updates invoker IT to call the renamed generate-models goal.
critter/critter-maven/spec.md Renames documented mojo goal names (but still needs spec updates for removed parameters).
critter/critter-integration-tests/src/test/java/dev/morphia/critter/CritterCrudTest.java Deletes redundant integration test coverage module content.
critter/critter-integration-tests/pom.xml Deletes the redundant module POM.
core/src/test/resources/META-INF/morphia-config.properties Documents morphia.mapper option in test config via comment.
.claude/CLAUDE.md Updates build/test guidance to reflect critter integration into morphia-core and morphia.mapper usage.
Comments suppressed due to low confidence (3)

critter/critter-maven/spec.md:49

  • After renaming the mojo to generate-criteria, this spec section still lists configuration parameters/behavior that were removed from the implementation (critter.packages, includeTestSources, criteriaPackage, configurable format). Please bring the spec in sync with the current CritterSourcesMojo/CritterSourceProcessor implementation.
## Sources Mojo (`critter:generate-criteria`)

* create another mojo called "generate-criteria" that collects all the source files from the project's source directories and passes them to a
  CritterSourceProcessor class for processing.
  * The mojo should be bound to the "generate-sources" phase of the Maven build lifecycle.
  * The processed source files should be stored in the target/generated-sources/critter directory.
  * The mojo should also be implemented in kotlin and use the Maven Plugin API.
  * this mojo can also be configured to optionally process the test source files as well.
  * this mojo should also add the generated-sources/critter directory to the project's compile source roots so that the generated
    sources are included in the compilation.

critter/critter-maven/src/main/kotlin/dev/morphia/critter/maven/CritterSourcesMojo.kt:105

  • loadMorphiaConfig() is looking for resources/morphia-config.properties, but Morphia’s default config file is META-INF/morphia-config.properties. Also, adding the properties file URL itself to a URLClassLoader won’t make that resource discoverable via MorphiaConfig.load() (it expects a classpath root or an explicit location). Consider either (1) locating src/main/resources/META-INF/morphia-config.properties correctly and calling MorphiaConfig.load(configFile.toUri().toString()), or (2) adding the resources directory (classpath root) to the temporary classloader.
    private fun loadMorphiaConfig(): MorphiaConfig {
        val configFile =
            project
                .getEnabledSourceRoots(MAIN, null)
                .asSequence()
                .mapNotNull { it.directory().resolve("resources/morphia-config.properties") }
                .firstOrNull { path: Path -> path.exists() }

        return if (configFile?.exists() == true) {
            logger.info("Loading morphia config from: ${configFile.toFile().absolutePath}")
            // Load config by adding the resources dir to classpath temporarily
            val urls = arrayOf(configFile.toUri().toURL())
            val tempClassLoader = URLClassLoader(urls, Thread.currentThread().contextClassLoader)
            val originalClassLoader = Thread.currentThread().contextClassLoader
            try {
                Thread.currentThread().contextClassLoader = tempClassLoader
                MorphiaConfig.load()
            } finally {
                Thread.currentThread().contextClassLoader = originalClassLoader
            }
        } else {
            logger.info("No morphia-config.properties found, using default configuration")
            MorphiaConfig.load() // Returns default config when file not found
        }

critter/critter-maven/spec.md:23

  • After renaming the goal to generate-models, this spec section still documents configuration parameters/behavior that no longer exist in the code (e.g., packages, includeTestClasses). Please update the spec to match the current mojo surface area so it remains the “source of truth”.
## Generate Mojo (`critter:generate-models`)

* create a maven mojo called named "generate-models" examines the compiled .class files in the project's output directory as defined in the
  MavenProject passed to the mojo.
  * These classes should be passed to a CritterProcessor class that performs some processing on them.
  * The mojo should be bound to the "process-classes" phase of the Maven build lifecycle.
  * The mojo should be implemented in kotlin and use the Maven Plugin API.
  * The results of the processing should be stored in the target (or the equivalent output directory as defined in the MavenProject) under
    generated-classes/critter
  * The mojo can be configured to optionally process the test classes as well.

Comment on lines +3 to +4
Starting in Morphia 3.0, configuration will be driven via a configuration file. However programmatic configuration is still supportedfor those who prefer, or need, a more flexible approach. The file should live in your build's resources folder and be named `META-INF/morphia-config.properties`. Reference documentation for the configuration file can be found in the link:++javadoc/dev/morphia/config/MorphiaConfig.html++[javadoc] but
we'll go in depth on a few items here. An example configuration file is included below. All possible configuration elements are listed. However, if the default values are acceptable that element can be safely omitted from your file.
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

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

There’s a missing space in “supportedfor”, which reads as a typo and makes the opening paragraph hard to parse. Please change to “supported for” (and consider reflowing the sentence for readability).

Suggested change
Starting in Morphia 3.0, configuration will be driven via a configuration file. However programmatic configuration is still supportedfor those who prefer, or need, a more flexible approach. The file should live in your build's resources folder and be named `META-INF/morphia-config.properties`. Reference documentation for the configuration file can be found in the link:++javadoc/dev/morphia/config/MorphiaConfig.html++[javadoc] but
we'll go in depth on a few items here. An example configuration file is included below. All possible configuration elements are listed. However, if the default values are acceptable that element can be safely omitted from your file.
Starting in Morphia 3.0, configuration will be driven via a configuration file.
However, programmatic configuration is still supported for those who prefer,
or need, a more flexible approach. The file should live in your build's
resources folder and be named `META-INF/morphia-config.properties`.
Reference documentation for the configuration file can be found in the
link:++javadoc/dev/morphia/config/MorphiaConfig.html++[javadoc], but we'll go
in depth on a few items here. An example configuration file is included
below. All possible configuration elements are listed. However, if the
default values are acceptable that element can be safely omitted from your
file.

Copilot uses AI. Check for mistakes.
@Parameter(property = "critter.criteriaPackage") private var criteriaPackage: String? = null

@Parameter(property = "critter.format", defaultValue = "true")
// @Parameter(property = "critter.format", defaultValue = "true")
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

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

The critter.format parameter annotation is commented out, so format is no longer configurable even though it appears intended to be. Either restore the @Parameter annotation (if this is meant to remain user-configurable) or remove the commented annotation to avoid misleading future maintainers.

Suggested change
// @Parameter(property = "critter.format", defaultValue = "true")
@Parameter(property = "critter.format", defaultValue = "true")

Copilot uses AI. Check for mistakes.
Comment on lines +35 to +42
readonly = true,
)
private lateinit var sourceOutputDirectory: File

@Parameter(
property = "critter.resourceOutputDirectory",
defaultValue = "\${project.build.directory}/generated-resources/critter",
readonly = true,
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

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

sourceOutputDirectory/resourceOutputDirectory are declared with Maven properties but marked readonly = true, which prevents users (and integration tests) from overriding them via plugin configuration. If these are meant to be configurable, remove readonly = true; if they are not, consider removing the property = ... attributes to avoid advertising unsupported configuration.

Suggested change
readonly = true,
)
private lateinit var sourceOutputDirectory: File
@Parameter(
property = "critter.resourceOutputDirectory",
defaultValue = "\${project.build.directory}/generated-resources/critter",
readonly = true,
)
private lateinit var sourceOutputDirectory: File
@Parameter(
property = "critter.resourceOutputDirectory",
defaultValue = "\${project.build.directory}/generated-resources/critter",

Copilot uses AI. Check for mistakes.
@Parameter(
property = "critter.outputDirectory",
defaultValue = "\${project.build.directory}/generated-classes/critter",
readonly = true,
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

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

outputDirectory is marked readonly = true, which is a behavioral change for plugin consumers because it can no longer be overridden via configuration despite advertising critter.outputDirectory. If overrides should still be supported, drop readonly = true; otherwise remove the property attribute to reflect that it’s fixed.

Suggested change
readonly = true,

Copilot uses AI. Check for mistakes.
name = "generate",
name = "generate-models",
defaultPhase = LifecyclePhase.PROCESS_CLASSES,
requiresDependencyResolution = ResolutionScope.TEST,
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

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

requiresDependencyResolution is still ResolutionScope.TEST, but the mojo now only processes main output (project.build.outputDirectory) and builds its classloader from project.compileClasspathElements. Consider lowering this to COMPILE (or RUNTIME if needed) to avoid resolving test-scoped deps unnecessarily during process-classes.

Suggested change
requiresDependencyResolution = ResolutionScope.TEST,
requiresDependencyResolution = ResolutionScope.COMPILE,

Copilot uses AI. Check for mistakes.
# Conflicts:
#	critter/critter-integration-tests/pom.xml
@evanchooly evanchooly merged commit d93ab12 into master May 1, 2026
29 of 32 checks passed
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.

[100%] Phase 8: Cleanup and documentation

2 participants