HSEARCH-4533 Introduce customizing properties for outbox polling

[marko-bekhta]

https://hibernate.atlassian.net/browse/HSEARCH-4533

• define a set of properties for catalog/schema/tables names for
  agent and outbox events
• make mapping producers to consider introduced properties
• make sure that statement inspector doesn't pick up keys on partial matches

I didn't find any "programmatic configuration" capability, only through the properties, or have I missed something ?
Also, I wasn't sure how to test with the custom schema/catalog. Simply specifying something doesn't work out of the box as it throws an exception that the underlying H2 doesn't have such schema. And I haven't yet dug deep to figure out how the backendMock and ormSetupHelper internals are working. Could you maybe point me to a sample where tests are using various schemas?

[yrodiere]

Thanks! Here are a few comments; there's a lot but don't worry: they are mostly about documentation, and

Apart from that, could you please also add a section in the reference documentation about the properties you just added? Maybe under the existing section about the Impact on the database schema? Just add a Basics subsection to shove existing text in, and add a new section named Custom schema/table name/etc. to document your properties. You can take inspiration from the structure of this (unrelated) section : quick intro, followed by an example of properties being set, followed by a list explaining the purpose of each property.

I didn't find any "programmatic configuration" capability, only through the properties, or have I missed something ?

You didn't miss it, there is no such thing in Hibernate Search. We only have programmatic mapping, but that's the index mapping, so it's unrelated to what you did.
People who want to configure programmatically just have to build their properties map manually, which is something most frameworks allow nowadays.

Also, I wasn't sure how to test with the custom schema/catalog. Simply specifying something doesn't work out of the box as it throws an exception that the underlying H2 doesn't have such schema. And I haven't yet dug deep to figure out how the backendMock and ormSetupHelper internals are working. Could you maybe point me to a sample where tests are using various schemas?

I don't think we have any at the moment.

I believe the easiest option would be to allow Hibernate ORM to create schemas, but I'm not sure if it's supported for all databases. Try setting javax.persistence.create-database-schemas to true? Worst case, you can disable some tests depending on whether a dialect supports catalogs/schemas or not: see NameQualifierSupport, which you can get by calling sessionFactory.getJdbcServices().getJdbcEnvironment().getNameQualifierSupport().

Failing that, you could try adding your own SQL to create the schema; see org.hibernate.boot.MetadataBuilder#applyAuxiliaryDatabaseObject. You would have to add a way to pass options to the MetadataBuilder in SimpleSessionFactoryBuilder first, though: around this line you'd need to call getMetadataBuilder(), then apply a bunch of contributors, then call metadataBuilder.build(). Then you'd do something like this in your test:

			.withConfiguration( b -> b.onMetadataBuilder( metadataBuilder -> {
				metadataBuilder.applyAuxiliaryDatabaseObject( <your object that creates the schema >);

If that still doesn't cut it, I suppose you'll have to try another approach: start Hibernate ORM/Search with database schema generation disabled, trigger DDL scripts generation explicitly, and check the content of DDL scripts. That will be painstaking, though. You can find an example of how to do this in the Hibernate ORM codebase, see org.hibernate.orm.test.boot.database.qualfiedTableNaming.DefaultCatalogAndSchemaTest#createSchema_fromSessionFactory.

[marko-bekhta]

🙈
I think this is now ready for another pass. In addition to what you've suggested I have changed the property "structure" a bit. The idea was to have the difference on the far right side of the property as possible:

hibernate.search.coordination.outbox_polling.entity.mapping.*.catalog

This way properties look more grouped.

The schema property that you've suggested for tests did work, thanks!

Then there are a few questions ...

• You've mentioned it's not ideal to leak some internal classes into public API. Hence I was wondering if we should be adding {@link Agent} and {@link OutboxEvent} in the JavaDocs or if instead use the @value?
• I noticed that there are no @since sections in JavaDocs, do we need to add them to these new properties or it's not used in HSEARCH ?
• I was wondering if we should change to @value instead of @link in the doc section where we are referencing the coordination strategy:

do this:
Only available when "hibernate.search.coordination.strategy" is "outbox-polling". 
instead of:
Only available when HibernateOrmMapperSettings.COORDINATION_STRATEGY is "outbox-polling".

[yrodiere]

Thanks! A few comments below.

Regarding your questions:

I have changed the property "structure" a bit. The idea was to have the difference on the far right side of the property as possible

Makes sense, thanks.

You've mentioned it's not ideal to leak some internal classes into public API. Hence I was wondering if we should be adding {@link Agent} and {@link OutboxEvent} in the JavaDocs or if instead use the @value?

Good catch.

I'd avoid any javadoc tag in this case; I'm not even sure you can use @value on a class, I think it's limited to primitive-type constants.

I suppose we could rephrase the documentation (javadoc and reference documentation) that way:

• "the outbox event table" instead of "the table mapped to the {@link OutboxEvent} entity"
• "the identifier generator used for the outbox event table" instead of "the identifier generator mapped to the {@link OutboxEvent} entity"
• etc.

What do you think?

I noticed that there are no @SInCE sections in JavaDocs, do we need to add them to these new properties or it's not used in HSEARCH ?

I've never seen that used in HSEARCH. Don't bother: if we ever start using them, we'll have to go back and add them back everywhere anyway.

I was wondering if we should change to @value instead of @link in the doc section where we are referencing the coordination strategy:

do this:
Only available when "hibernate.search.coordination.strategy" is "outbox-polling". 
instead of:
Only available when HibernateOrmMapperSettings.COORDINATION_STRATEGY is "outbox-polling".

I suppose it would make sense in this specific case, yes. But if you apply this change, please apply it consistently everywhere this sentence appears ;)

[yrodiere]

Thanks, LGTM. I added a few follow-up commits, in particular one to update the patches related to ORM 6 and make the build pass. Let's see how it goes.

[yrodiere]

Tests are still failing, and I suspect that's related to the stricter check in org.hibernate.search.integrationtest.mapper.orm.coordination.outboxpolling.automaticindexing.OutboxPollingCustomEntityMappingIT.KeysStatementInspector#inspect: sequence names don't appear in SQL statements for PostgreSQL, for example.

I need to investigate further, and also to run tests on other databases.

[yrodiere]

Ok, that should do it. The problem was the pattern you used in the tokenizer, which didn't account for quotes (e.g. in postgresql's nextval('my_sequence')); see the last commit.

[marko-bekhta]

thanks! I was about to ping you and see if I could help with the tests, but you've figured it out already 😃
sorry that it generated so much additional work for you 😞

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[yrodiere]

No problem, I would have asked you to handle this, but initially I thought this was some obscure database-specific limitation requiring to dig into the internals of ORM, and since I had some experience with this part of ORM, I figured I would have a look.

Turns out I was wrong, the problem was much simpler :)

Thanks to you for your work!