Skip to content

Commit

Permalink
HHH-14334 Make dom4j jaxb-api optional as possible
Browse files Browse the repository at this point in the history
dependency dom4j and jaxb-api is optional if xml mapping disabled
continuation of HHH-13204
  • Loading branch information
quaff authored and Sanne committed Nov 20, 2020
1 parent de0f141 commit 34151a9
Showing 1 changed file with 11 additions and 2 deletions.
Expand Up @@ -32,8 +32,11 @@
import org.hibernate.boot.spi.MetadataBuildingOptions;
import org.hibernate.boot.spi.MetadataContributor;
import org.hibernate.boot.spi.MetadataImplementor;
import org.hibernate.cfg.AvailableSettings;
import org.hibernate.cfg.MetadataSourceType;
import org.hibernate.dialect.Dialect;
import org.hibernate.engine.config.spi.ConfigurationService;
import org.hibernate.engine.config.spi.StandardConverters;
import org.hibernate.engine.jdbc.spi.JdbcServices;
import org.hibernate.type.BasicType;
import org.hibernate.type.BasicTypeRegistry;
Expand Down Expand Up @@ -95,10 +98,16 @@ public static ManagedResources prepare(
final MetadataSources sources,
final BootstrapContext bootstrapContext) {
final ManagedResourcesImpl managedResources = ManagedResourcesImpl.baseline( sources, bootstrapContext );
final ConfigurationService configService = bootstrapContext.getServiceRegistry().getService( ConfigurationService.class );
final boolean xmlMappingEnabled = configService.getSetting(
AvailableSettings.XML_MAPPING_ENABLED,
StandardConverters.BOOLEAN,
true
);
ScanningCoordinator.INSTANCE.coordinateScan(
managedResources,
bootstrapContext,
sources.getXmlMappingBinderAccess()
xmlMappingEnabled ? sources.getXmlMappingBinderAccess() : null
);
return managedResources;
}
Expand Down Expand Up @@ -290,7 +299,7 @@ public void finishUp() {
final EntityHierarchyBuilder hierarchyBuilder = new EntityHierarchyBuilder();
// final MappingBinder mappingBinder = new MappingBinder( true );
// We need to disable validation here. It seems Envers is not producing valid (according to schema) XML
final MappingBinder mappingBinder = new MappingBinder( classLoaderService, false );
final MappingBinder mappingBinder = options.isXmlMappingEnabled() ? new MappingBinder( classLoaderService, false ) : null;

This comment has been minimized.

Copy link
@Roman-Skripka

Roman-Skripka Dec 2, 2020

fyi @quaff this change produces:

java.lang.NullPointerException: null
at org.hibernate.envers.boot.internal.AdditionalJaxbMappingProducerImpl$1.addDocument(AdditionalJaxbMappingProducerImpl.java:94)
at org.hibernate.envers.configuration.internal.EntitiesConfigurator.configure(EntitiesConfigurator.java:111)
at org.hibernate.envers.boot.internal.EnversServiceImpl.doInitialize(EnversServiceImpl.java:148)
at org.hibernate.envers.boot.internal.EnversServiceImpl.initialize(EnversServiceImpl.java:112)
at org.hibernate.envers.boot.internal.AdditionalJaxbMappingProducerImpl.produceAdditionalMappings(AdditionalJaxbMappingProducerImpl.java:101)
at org.hibernate.boot.model.process.spi.MetadataBuildingProcess.complete(MetadataBuildingProcess.java:305)

This comment has been minimized.

Copy link
@Sanne

Sanne Dec 2, 2020

Member

right, I noticed the same and worked around it by enabling XML mapping when using Envers.

Too bad I only noticed after the release, filing as new issue:

Thanks!

This comment has been minimized.

Copy link
@Sanne

Sanne Dec 2, 2020

Member

BTW, how did you notice? If you're using Envers, you're probably not disabling XML mapping? Or maybe you're not using Envers, in that case you could remove it from your application.

This comment has been minimized.

Copy link
@Roman-Skripka

Roman-Skripka Dec 2, 2020

We use hibernate and envers perfectly fine without xml mappings. Since this flag was initially switched off it failed right after the version upgrade.

This comment has been minimized.

Copy link
@quaff

quaff Dec 3, 2020

Author Contributor

@Sanne I'm not sure those codes should be executed if xml disabled.

ModelBinder binder = ModelBinder.prepare( rootMetadataBuildingContext );
for ( EntityHierarchySourceImpl entityHierarchySource : hierarchyBuilder.buildHierarchies() ) {
binder.bindEntityHierarchy( entityHierarchySource );
}

This comment has been minimized.

Copy link
@quaff

This comment has been minimized.

Copy link
@atzoum

atzoum Feb 10, 2021

We use hibernate and envers perfectly fine without xml mappings. Since this flag was initially switched off it failed right after the version upgrade.

@Sanne We are also using hibernate and envers perfectly fine without xml mappings. However, this change along with https://hibernate.atlassian.net/browse/HHH-14356 makes it impossible for us to upgrade. Could you please revise? What are your plans for supporting envers without xml mappings in the near future?

This comment has been minimized.

Copy link
@Sanne

Sanne Feb 10, 2021

Member

@atzoum could you elaborate on the problem? Ideally open a new JIRA.

We have many integration tests working just fine with Envers. Are you setting AvailableSettings.XML_MAPPING_ENABLED? Don't do that when using Envers.

This comment has been minimized.

Copy link
@Sanne

Sanne Feb 10, 2021

Member

See also commit e21f4d3 - you should get a warning, let me know if that's not the case, I might need to improve this?

This comment has been minimized.

Copy link
@atzoum

atzoum Feb 10, 2021

@Sanne thanks. Enabling xml mapping (which is on by default), although we are not using any, seems to be fixing the issue.

for ( AdditionalJaxbMappingProducer producer : producers ) {
log.tracef( "Calling AdditionalJaxbMappingProducer : %s", producer );
Collection<MappingDocument> additionalMappings = producer.produceAdditionalMappings(
Expand Down

0 comments on commit 34151a9

Please sign in to comment.