Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HHH-19103 add SchemaManager.populate() #9893

Merged
merged 3 commits into from
Mar 21, 2025
Merged

Conversation

gavinking
Copy link
Member

@gavinking gavinking commented Mar 21, 2025

[Please describe here what your change is about]


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.


https://hibernate.atlassian.net/browse/HHH-19103

* lists of many {@code insert} statements on the same table, which is what
* we typically expect, it's probably better to not format.
*/
private static Formatter getImportScriptFormatter(boolean format) {

Check notice

Code scanning / CodeQL

Useless parameter Note

The parameter 'format' is never used.
@@ -21,6 +21,9 @@
SchemaDropper getSchemaDropper(Map<String,Object> options);
SchemaMigrator getSchemaMigrator(Map<String,Object> options);
SchemaValidator getSchemaValidator(Map<String,Object> options);
default SchemaPopulator getSchemaPopulator(Map<String,Object> options) {

Check notice

Code scanning / CodeQL

Useless parameter Note

The parameter 'options' is never used.
@hibernate-github-bot
Copy link

hibernate-github-bot bot commented Mar 21, 2025

Thanks for your pull request!

This pull request does not follow the contribution rules. Could you have a look?

❌ All commit messages should start with a JIRA issue key matching pattern HHH-\d+
    ↳ Offending commits: [629d41d, f030015]

› This message was automatically generated.

@sebersole sebersole merged commit 982ab4e into hibernate:main Mar 21, 2025
19 of 23 checks passed
Metadata metadata,
final boolean manageNamespaces,
GenerationTarget... targets) {
final ServiceRegistry serviceRegistry =
Copy link
Member

Choose a reason for hiding this comment

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

sooo... these were used in Search tests 🥲, not anymore, I guess ...

Copy link
Member Author

Choose a reason for hiding this comment

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

Whhaaa? You were testing the SchemaTruncator from Hibernate Search??? What for?

Copy link
Member

Choose a reason for hiding this comment

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

ohh, no... not testing this method or truncator in general, but using it to clean the test data 🙈

Copy link
Member Author

Choose a reason for hiding this comment

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

I mean we're talking about an unused method annotated @Internal of an Impl class in an internal package.

We really need to avoid cross-project dependencies going through methods like that. That's why we define SPIs.

Copy link
Member Author

Choose a reason for hiding this comment

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

using it to clean the test data

The you should be using the API for that, not depending on internal implementation details.

If that API doesn't work, then you should propose introduction of an SPI.

Otherwise, when we're working on ORM, we have absolutely no warning that we've done something that might break Search, and indeed, Search deserves to break.

Copy link
Member

@sebersole sebersole Mar 24, 2025

Choose a reason for hiding this comment

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

Do you happen to use the @SessionFactory stuff for testing @marko-bekhta ? If so I added SessionFactoryScope#dropData. Otherwise you could just use sessionFactory.getSchemaManager().truncateMappedObjects()

Copy link
Member

Choose a reason for hiding this comment

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

ohh Search has a set of its own extensions to deal with tests 🙈 so we don't leverage the @SessionFactory (at least not yet 😉).

Otherwise you could just use sessionFactory.getSchemaManager().truncateMappedObjects()

right ... that's what we do, it's just that we have this extension to deal with a multitenancy, and it has a custom schema manager with overrides, that's where that method was used from 😃.

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.

3 participants