Upgrade to Hibernate Search 6.2.0.CR1

[yrodiere]

Creating as draft, because we will need to release Hibernate Search 6.2.0.CR1 (soon) first. Currently, to test this you'd need to build Hibernate Search locally and change the version of the Hibernate Search dependency in this PR to 6.2.0-SNAPSHOT. => Hibernate Search 6.2.0.CR1 is out, we can upgrade.

I checked locally that we could run integration-tests/hibernate-search-orm-elasticsearch, including in native mode, but I didn't run many more tests.

@marko-bekhta would you have a look, please?

[quarkus-bot]

Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

• title should preferably start with an uppercase character (if it makes sense!)

_{This message is automatically generated by a bot.}

[quarkus-bot]

/cc @gsmet (hibernate-search)

[marko-bekhta]

looks good 🎉 I left a couple questions below 😃

[marko-bekhta]

🎉

[yrodiere]

Hibernate Search 6.2.0.CR1 is out, we can upgrade. Marking as ready for review (and ready to merge as far as I'm concerned :) )

[github-actions]

🙈 The PR is closed and the preview is expired.

[yrodiere]

I tried to investigate the failure, and I believe it's a bug in the config retrieval...

To reproduce, run this on this branch:

mvn clean install -f extensions/hibernate-search-orm-elasticsearch/deployment -Dstart-containers -Dtest-containers -Dtest=MultiplePersistenceUnitsCdiTest

You'll get a failure, but it won't tell much. The cause can be witnessed in debug mode:

mvn clean install -f extensions/hibernate-search-orm-elasticsearch/deployment -Dstart-containers -Dtest-containers -Dtest=MultiplePersistenceUnitsCdiTest -Dmaven.surefire.debug

Put a breakpoint in io.quarkus.hibernate.search.orm.elasticsearch.runtime.HibernateSearchElasticsearchRecorder#createRuntimeInitListener. You'll see that the config object (HibernateSearchElasticsearchRuntimeConfigPersistenceUnit) for the persistence units pu1/pu2 completely ignores these lines in the configuration file:

quarkus.hibernate-search-orm."pu2".elasticsearch.hosts=${elasticsearch.hosts:localhost:9200}
quarkus.hibernate-search-orm."pu2".elasticsearch.protocol=${elasticsearch.protocol:http}
quarkus.hibernate-search-orm."pu2".schema-management.strategy=drop-and-create-and-drop
quarkus.hibernate-search-orm."pu2".indexing.plan.synchronization.strategy=sync

Remove the double quotes around pu1/pu2, and the config will no longer ignore them...

Hey @radcortez , is this a know bug? Maybe quotes around map keys are no longer supported for some reason?

[radcortez]

Hey @radcortez , is this a know bug? Maybe quotes around map keys are no longer supported for some reason?

I'm not aware of anything. Is this already using @ConfigMapping?

[yrodiere]

Hey @radcortez , is this a know bug? Maybe quotes around map keys are no longer supported for some reason?

I'm not aware of anything. Is this already using @ConfigMapping?

Yes it is.

[radcortez]

Ok, I'll have a look.

[yrodiere]

FWIW I just tried on the main branch and I didn't reproduce the problem. So there's something in this PR that causes the problem.

[radcortez]

This seems to be an issue between the combination of quoted keys and @WithName containing multiple dotted segments. Dropping @WithName from planSynchronizationStrategy and using quarkus.hibernate-search-orm.indexing.plan-synchronization-strategy instead works. This will require a fix from SmallRye Config side.

In the meanwhile, to move forward, does it make sense for planSynchronizationStrategy to be split by dots? Usually, we use dots to separate groups, but there isn't a group here or sub groups on each segment.

[yrodiere]

Thanks Roberto.

In the meanwhile, to move forward, does it make sense for planSynchronizationStrategy to be split by dots? Usually, we use dots to separate groups, but there isn't a group here or sub groups on each segment.

It makes sense, but modeling groups is more verbose and not strictly necessary, which is why I avoided that. But since it's becoming strictly necessary after all... I'll change it :)

[radcortez]

I should be able to get a fix for this today, but it may take some time until it reaches Quarkus.

[yrodiere]

@radcortez Defining one config group class per segment instead of using @WithName seems to work around the issue, so I did that.

[radcortez]

Yes, that also works. Anyway, I'm pushing a fix to support your previous use case.

[quarkus-bot]

Failing Jobs - Building be15a3a

Status	Name	Step	Failures	Logs	Raw logs
✔️	Maven Tests - JDK 11
⌛	Maven Tests - JDK 11 Windows	Build	⚠️ Check →	Logs	Raw logs

[YassinHajaj]

Good job @yrodiere !
Do you have an idea on when to expect this in a released version of Quarkus ? Is it within the two coming weeks or later ?

[yrodiere]

@YassinHajaj Thanks. Probably the end of July, I'm not sure. IIRC @gsmet still has to draft a release schedule.

[YassinHajaj]

Super, thanks for the quick reply @yrodiere