spring-data-mongodb icon indicating copy to clipboard operation
spring-data-mongodb copied to clipboard

`@Meta` annotation not working properly with `MongoRepository` and custom repositories

Open malaquf opened this issue 1 year ago • 8 comments

Environment: Kotlin with coroutines 2.1.0 Spring Boot 3.3.1

Problem When adding a @Meta annotation to my queries in a given repository, I get empty lists as result.

Unfortunately, I could not find the root cause yet, but I hope the following description helps to reproduce it.

CrudMethodMetadataPopulatingMethodInterceptor checks if a method is a query using isQueryMethod(). Because @Meta adds @QueryAnnotation, the method is not added to Set<Method> implementations.

Surprisingly, query methods defined in repositories by default are added to Set<Method> implementations from CrudMethodMetadataPopulatingMethodInterceptor, and everything works fine when not adding the @Meta annotation.

The difference I see so far is in the execution path for the query in the following method:

@Override
		public Object invoke(MethodInvocation invocation) throws Throwable {

			Method method = invocation.getMethod();

			if (!implementations.contains(method)) {
				return invocation.proceed();
			}

			MethodInvocation oldInvocation = currentInvocation.get();
			currentInvocation.set(invocation);

			try {

				CrudMethodMetadata metadata = (CrudMethodMetadata) TransactionSynchronizationManager.getResource(method);

				if (metadata != null) {
					return invocation.proceed();
				}

				CrudMethodMetadata methodMetadata = metadataCache.get(method);

				if (methodMetadata == null) {

					methodMetadata = new DefaultCrudMethodMetadata(repositoryInformation.getRepositoryInterface(), method);
					CrudMethodMetadata tmp = metadataCache.putIfAbsent(method, methodMetadata);

					if (tmp != null) {
						methodMetadata = tmp;
					}
				}

				TransactionSynchronizationManager.bindResource(method, methodMetadata);

				try {
					return invocation.proceed();
				} finally {
					TransactionSynchronizationManager.unbindResource(method);
				}
			} finally {
				currentInvocation.set(oldInvocation);
			}
		}
	}

When adding @Meta annotation, my query is executed through invocation.proceed() within if (!implementations.contains(method)) block. If @Meta is not present, the execution is done by the rest of the code.

Repository example:

interface MyRepository : ReactiveMongoRepository<MyDocument, MyKey> {

	@Meta(maxExecutionTimeMs = 30000)
	override fun findAllById(ids: Iterable<MyKey>): Flux<MyDocument>

	@Meta(maxExecutionTimeMs = 30000)
	override fun findById(id: MyKey): Mono<MyDocument>
}

I'll update the issue if I have more information.

By the way, all I want in this case is to define maxExecutionTimeMs for my queries and I believe there should be a better way to customise it (and specially set a default) without having to define it for each method in the repository. Like having an interceptor for easily extending the Query options, for example.

Thank you for looking into it.

malaquf avatar Dec 12 '24 10:12 malaquf

@malaquf a complete minimal sample (something that we can unzip or git clone, build, and deploy) that reproduces the problem would help a lot. Thank you!

christophstrobl avatar Dec 12 '24 10:12 christophstrobl

@christophstrobl thank you for the very fast answer. Sorry for that, here it goes: https://github.com/malaquf/spring-data-mongodb-issue-4852 Thanks!

malaquf avatar Dec 12 '24 12:12 malaquf

There are a couple of aspects and the fix isn't trivial. @Meta wasn't intended for implementation method usage and it isn't applicable for each operation (e.g. our save method doesn't even consider meta settings). A key problem is that @Meta isn't documented properly and therefore, there's no documentation about the usage scope.

When using @Meta, your method is considered a query method using query derivation. When using a single id and when your identifier is annotated with @MongoId instead of @Id, then the resulting query is valid and will return a result.

Going forward, there are a few things we need to consider:

  1. Document the annotation properly (that's what we're going to do with this ticket)
  2. Consider where @Meta could be used. Likely, we can only leverage a few find methods. Query by Example or Querydsl methods won't work really because of different infrastructure.

mp911de avatar Dec 13 '24 09:12 mp911de

Interesting! Thank you for the info. I did some quick research and it seems I could simply replace @Id by @MongoId without any breaking changes. Is it really the case or there are some caveats?

Do you see a better/simpler approach to intercept the queries and append a default maxTimeMS?

malaquf avatar Dec 13 '24 09:12 malaquf

Ok, I think this should be good for my use cases:

@Configuration
class MongoConfig {
	@Value("\${spring.data.mongodb.maxTimeMs}")
	private var maxTimeMs: Long = 7000

	@Bean
	fun mongoDBDefaultSettings(credential: MongoCredential): MongoClientSettingsBuilderCustomizer {
		return MongoClientSettingsBuilderCustomizer { builder: MongoClientSettings.Builder ->
			builder.addCommandListener(MaxTimeMsCommandListener(maxTimeMs))
		}
	}
}

class MaxTimeMsCommandListener(private val maxTimeMs: Long) : CommandListener {
	private val commandsWithMaxTimeMs = setOf(
		"find",
		"aggregate",
		"count",
		"distinct",
		"mapReduce",
		"group"
	)

	override fun commandStarted(event: CommandStartedEvent) {
		if (event.commandName in commandsWithMaxTimeMs) {
			event.command["maxTimeMS"] = BsonInt64(maxTimeMs)
		}
	}
}

malaquf avatar Dec 13 '24 13:12 malaquf

Unfortunately, it is not that easy, as it turned out, command is immutable. I see no way to extend spring data for that, unless I use Mongo Templates instead :/

malaquf avatar Dec 13 '24 14:12 malaquf

It looks like org.springframework.data.mongodb.core.FindPublisherPreparer#prepare would be an ideal place to extend, but unfortunately, it's an internal class. I'd like to ask you if you could provide us a way to set maxTimeMS, as this is a very important configuration.

I'm also aware of this new mongodb driver feature, and would also like to ask you to consider supporting it soon: https://jira.mongodb.org/browse/JAVA-3828

Thank you.

malaquf avatar Dec 13 '24 15:12 malaquf

Hello @mp911de, I've seen the changes in this PR but I honestly, with all respect, don't know how this solves any problem. I use @Query without any problems on methods like the one I sent in my example, also with no need for a @MongoId annotated document. The documentation added is still not clear regarding the limitations mentioned in your comment in my opinion, and it is still not possible to set the maxTimeMS property in repository interfaces.

malaquf avatar Dec 17 '24 16:12 malaquf