morphia icon indicating copy to clipboard operation
morphia copied to clipboard

Separate annotations into its own dependency

Open ctmay4 opened this issue 4 years ago • 21 comments

There was a pull request in 2015 (see #778) and I requested late binding of annotations in 2015 as well (#678).

Now that you have moved the codebase to 2.x I would like to request this again. I have separate projects which do not use Morphia but define entities that are used by other projects with Morphia. I am currently including the full Morphia dependency only to get the annotations but there are many user of the project that do not need that dependency.

Can that merge request be resurrected or recreated so that there is a dependency that only contains the annotations?

ctmay4 avatar May 04 '21 13:05 ctmay4

I'm not opposed to the idea of it but having a separate module would complicate documentation in ways that make me sad. Another possibility is a custom artifact jar containing only the annotations and attached to the build. So you'd have something like this:

<dependency>
    <groupId>dev.morphia.morphia</groupId>
    <artifactId>morphia-core</artifactId>
    <version>2.1.5</version>
    <classifier>annotations</classifier>
</dependency>

That seems pretty simple to do. (He says without actually trying...) Doing that would keep all the javadoc in one place but give a slimmed down dependency with just those annotations. Would something like that work?

evanchooly avatar May 04 '21 14:05 evanchooly

That would be great. I'm just trying to not expose the entire Morphia codebase to projects that do not need it.

ctmay4 avatar May 04 '21 14:05 ctmay4

Yep. I'll play with it. I'm trying to wrap up the 2.2 release so i'll see what i can do to get this sorted as part of that.

evanchooly avatar May 04 '21 14:05 evanchooly

Sounds great. Thanks.

ctmay4 avatar May 04 '21 14:05 ctmay4

That new jar should be available via a -SNAPSHOT of 2.2.0 if you want to take a look and make sure it's what you need. You can also find some brief documentation on the website

evanchooly avatar May 06 '21 03:05 evanchooly

I'm not sure it is working. I am using Gradle, so I believe this is the way to specify a classifier.

api 'dev.morphia.morphia:morphia-core:2.2.0-SNAPSHOT:annotations'

However when I look at the runtime classpath, I still see all the other parts of Morphia.

runtimeClasspath - Runtime classpath of source set 'main'.
+--- com.fasterxml.jackson.core:jackson-core:2.12.3
|    \--- com.fasterxml.jackson:jackson-bom:2.12.3
|         +--- com.fasterxml.jackson.core:jackson-annotations:2.12.3 (c)
|         +--- com.fasterxml.jackson.core:jackson-core:2.12.3 (c)
|         \--- com.fasterxml.jackson.core:jackson-databind:2.12.3 (c)
+--- com.fasterxml.jackson.core:jackson-annotations:2.12.3
|    \--- com.fasterxml.jackson:jackson-bom:2.12.3 (*)
+--- com.fasterxml.jackson.core:jackson-databind:2.12.3
|    +--- com.fasterxml.jackson.core:jackson-annotations:2.12.3 (*)
|    +--- com.fasterxml.jackson.core:jackson-core:2.12.3 (*)
|    \--- com.fasterxml.jackson:jackson-bom:2.12.3 (*)
+--- dev.morphia.morphia:morphia-core:2.2.0-SNAPSHOT
|    +--- org.mongodb:mongodb-driver-sync:4.2.2
|    |    +--- org.mongodb:bson:4.2.2
|    |    \--- org.mongodb:mongodb-driver-core:4.2.2
|    |         \--- org.mongodb:bson:4.2.2
|    +--- org.mongodb:mongodb-driver-legacy:4.2.2
|    |    +--- org.mongodb:bson:4.2.2
|    |    +--- org.mongodb:mongodb-driver-core:4.2.2 (*)
|    |    \--- org.mongodb:mongodb-driver-sync:4.2.2 (*)
|    +--- io.github.classgraph:classgraph:4.8.78
|    +--- net.bytebuddy:byte-buddy:1.10.2
|    +--- org.slf4j:slf4j-api:1.7.30
|    +--- org.jetbrains:annotations:13.0
|    +--- com.github.spotbugs:spotbugs-annotations:3.1.9
|    |    \--- com.google.code.findbugs:jsr305:3.0.2
|    \--- org.testng:testng:7.4.0
|         +--- com.beust:jcommander:1.78
|         \--- org.webjars:jquery:3.5.1
+--- org.cache2k:cache2k-api:2.0.0.Final
+--- org.apache.commons:commons-lang3:3.12.0
+--- org.ahocorasick:ahocorasick:0.4.0
+--- org.cache2k:cache2k-core:2.0.0.Final
|    \--- org.cache2k:cache2k-api:2.0.0.Final
\--- com.imsweb:seerapi-client-java:3.23
     +--- com.squareup.retrofit2:retrofit:2.9.0
     |    \--- com.squareup.okhttp3:okhttp:3.14.9
     |         \--- com.squareup.okio:okio:1.17.2
     +--- com.squareup.retrofit2:converter-jackson:2.9.0
     |    +--- com.squareup.retrofit2:retrofit:2.9.0 (*)
     |    \--- com.fasterxml.jackson.core:jackson-databind:2.10.1 -> 2.12.3 (*)
     \--- com.fasterxml.jackson.core:jackson-databind:2.10.5.1 -> 2.12.3 (*)

Maybe that is to be expected, so I was going to release a SNAPSHOT and try with the other classes, but when I build the project, it complains about missing a class. I assume that enum is used in the annotations somewhere and it is getting excluded? I checked and I do not have any reference to IndexDirection in my codebase.

> Task :processResources NO-SOURCE
> Task :classes
warning: unknown enum constant IndexDirection.ASC
  reason: class file for dev.morphia.utils.IndexDirection not found
1 warning

ctmay4 avatar May 06 '21 13:05 ctmay4

Looks like this class is the issue. It is including a class outside the annotations package.

package dev.morphia.annotations;


import dev.morphia.utils.IndexDirection;

import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
import java.lang.annotation.Inherited;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;


/**
 * Specified on fields that should be Indexed.
 *
 * @author Scott Hernandez
 */
@Documented
@Inherited
@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.FIELD})
public @interface Indexed {
    /**
     * @return Options to apply to the index. Use of this field will ignore any of the deprecated options defined on {@link Index} directly.
     */
    IndexOptions options() default @IndexOptions();

    /**
     * @return the type of the index (ascending, descending, geo2d); default is ascending
     * @see IndexDirection
     */
    IndexDirection value() default IndexDirection.ASC;
}

ctmay4 avatar May 06 '21 14:05 ctmay4

OK. The enum is easy enough to fix. If you can run mvn dependency:tree and post that output I can dig in to that. I have some vague ideas and none of them excite me...

evanchooly avatar May 06 '21 15:05 evanchooly

I am not using Maven. I am using Gradle. I posted the dependency tree above from gradle dependencies.

ctmay4 avatar May 06 '21 15:05 ctmay4

right. duh. sorry. bouncing back and forth with $dayjob. :)

evanchooly avatar May 06 '21 15:05 evanchooly

No problem. Let me know if there is anything I can do to help.

ctmay4 avatar May 06 '21 15:05 ctmay4

I've been playing with this all night and running in to some wrinkles. There are a number of references to items outside the annotations package that make it very difficult to extract out just the annotations. I'm still playing with options and haven't quite given up just yet but it's getting messier and messier which says to me I'm on the wrong path.

While I play with this, is there any reasons you can't just use morphia-core as a provided scope dependency so that you can build with those annotations but the projects that don't need morphia don't get it and those that do can simply import morphia directly?

evanchooly avatar May 08 '21 02:05 evanchooly

Honestly, I think that would be the cleanest solution. You're likely going to have to declare this new dependency as provided anyway for the same reasons.

evanchooly avatar May 08 '21 02:05 evanchooly

I guess I don't totally understand. If I say that Morphia is a provided dependency, then a user of the library, which doesn't want or care about MongoDB, would need to add that provided dependency to their build.gradle. That kind of defeats the purpose or maybe I am missing the point.

Sorry you had to spend so much time one this. I can definitely live with the extra dependency if it is too difficult to restructure. In reality, even if you got this to work, it is still not ideal. The "perfect" solution is to implement what Jackson does for this.

https://github.com/FasterXML/jackson-docs/wiki/JacksonMixInAnnotations

If that were implemented, I could remove all references to Morphia in my library project. The vast majority of people who use the library would not have any MongoDB-related dependencies in their projects. The one project I want to use the library and persist those entities would create Mixin abstract classes with the annotations and tell Morphia that at initialization.

ctmay4 avatar May 08 '21 14:05 ctmay4

Actually, now that I think more about it, even Mixin annotations wouldn't totally work. For example, here is one of the entities in the library.

https://github.com/imsweb/staging-client-java/blob/master/src/main/java/com/imsweb/staging/entities/StagingSchema.java

One of the fields on this class is an ObjectID so having externally defined annotations would help many cases but not completely handle this specific one.

ctmay4 avatar May 08 '21 14:05 ctmay4

There are references in the annotation to classes outside the annotations (reference related mostly). Those classes, obviously, have references to other classes and so on. So simply extracting out the annotation types is not sufficient. I'll have to rework how some of that works to make that happen and I'd rather not do that right on the cusp of 2.2 releasing. Mixins would likely render this whole thing moot either way.

I will definitely take a look at mixins for 2.3. I have a partial solution in place but it's limited to applying class level annotations and mixins would round that out.

As for the ObjectId dep, that's in the org.mongodb:bson dep so you can include just that jar as an optional dep and limit the dependencies

evanchooly avatar May 08 '21 16:05 evanchooly

That sounds good. Definitely no rush and I don't want to add any more work to your plate.

I've been thinking this morning about some things and I am going to try to refactor my library so that I won't need any changes to Morphia. I've tried it before but I am going to try something different this time.

Thanks for all your help.

ctmay4 avatar May 08 '21 16:05 ctmay4

Any time. If nothing else, that reminder about jackson's mixins (which I'd forgotten about) is a good thing to consider for the next version. 2.3 should be a big one.

evanchooly avatar May 08 '21 16:05 evanchooly

1.6 -> 2.x was a big change for me. It took a while to convert and test. Fingers crossed not too much changes in 2.3!

ctmay4 avatar May 08 '21 17:05 ctmay4

Feature wise. No API changes like 1.6->2.0. i knew 2.0 was going to be a huge shift and was meant to be. it tried to clean up years of cruft. that's all behind us. 🤞🏻

evanchooly avatar May 08 '21 18:05 evanchooly

OK. so morphia.next should have this. I know i've been kicking this can down the road but things are in better shape for a clean separation. I'm not sure if .next is 2.4 or 3.0 but we'll get it done.

evanchooly avatar Mar 14 '22 15:03 evanchooly