Skip to content

Conversation

@jdaugherty
Copy link
Contributor

@jdaugherty jdaugherty commented Oct 30, 2024

I've been debugging the gorm-hibernate5 tests and discovered that any classpath based initialization of the HibernateDatastore was failing. @jamesfredley changed the AnnotationMetadataReader to use the helper AnnotationMetadata.from() from Spring. It was failing because this helper restricts to the spring & java packages.

For now, I've copied their StandardMetadata implementation and enhanced it to take an annotation filter, which I can then use to initialize AnnotationMetadataReaderFactory properly. I renamed AnnotationMetadataReaderFactory to reflect it's only for entities since I'm now passing that filter explicitly. From searching the grails repos, I don't see any other usage of it. I hope to open an upstream PR with spring to see if they'll accept an additional argument to specify custom annotation filters.

This PR also removes some left over imports from the inheritance fixes & it fixes an ignored test - the $target method added by the AST transformation is now found by the groovy getObjectProperties in DefaultJsonGenerator, which causes a stackoverflow since it's recursive. This may be a larger issue for json in groovy4, but for now I've explicitly ignored the found field to fix the test. I assume most people will be using Jackson and will just igore it as part of converting to Grails 7.

@jdaugherty
Copy link
Contributor Author

Once this is merged, I should be able to fix most of the gorm-hibernate5 tests.

Copy link
Contributor

@matrei matrei left a comment

Choose a reason for hiding this comment

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

Looks great!
Do you want to update the year range in the license header of ClasspathEntityScanner?

@jdaugherty jdaugherty merged commit 0cfee5f into apache:9.0.x Oct 30, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants