Spring Nullpointer Exception on Generating Key with SimpleKeyGenerator

The issue https://github.com/spring-projects/spring-framework/issues/31412 introduced an regression for us. The check KotlinDetector.isSuspendingFunction(method) results in a Nullpointer-Exception since we are calling the generate Method in the following way

                    cacheKeyGenerator.generate(
                        null,
                        null,
                        new CodelistKey(entry.getUri(), entry.getVersion(), entry.getCode())
                    )

Comment From: sdeleuze

KeyGenerator#generate method parameter is not annotated with @Nullable and all invocations done in Spring Framework are done with a non-null parameter. Before the change you mentioned, the implementation was leniently accept null value, with this change it does not, but still respect the contract.

Would you consider updating your usage to follow the contract?

Comment From: snicoll

You should be calling SimpleKeyGenerator.generateKey(Object...arguments) rather than the current method if you can't honor its contract.

Comment From: fabian-froehlich

Our usecase is an afterStartup script that populates the cache with information that is retrieved from a database. At the time I implemented it in our boot application I found no clean solution that would work with @Cacheable annotated methods to achieve the same.

I think it would be no problem to change our code in that way, that I directly call SimpleKeyGenerator#generateKey.

Today there is an automatic injected KeyGenerator that auto configured SimpleKeyGenerator.

I can't really tell if I found that solution online or made it up by myself. But if it was the former, then others might experience similar issues which could result in a slower adaption rate of the newer spring framework version.

Comment From: sdeleuze

Thanks for your quick feedback.

I am going to decline this change request because the team is not in favor of making the Method parameter @Nullable and I would like to avoid introducing artificial null-checks that may be reported as unneeded based on our null-safety annotations analysis. So please adapt your code accordingly.

Comment From: snicoll

If you are populating the cache, expecting that further calls on the actual annotated method(s) will match, it's even more important to take control over key generation. Relying on what's auto-configured and calling it will null while the actual production code will be invoked with a method and a target class is an important risk that the generated keys would not match.

To be clear, calling this method with null values does not respect the contract of the API.

Comment From: fabian-froehlich

Thanks for the constructive feedback and conversation. I will review our caching approach. @snicoll I tried to ensure the correct key mapping with tests but that already felt fuzzy at that time.