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

Hibernate Validator integration with Hibernate Reactive is untested, possibly dysfunctional #46327

Open
yrodiere opened this issue Feb 18, 2025 · 1 comment · May be fixed by #46398
Open

Hibernate Validator integration with Hibernate Reactive is untested, possibly dysfunctional #46327

yrodiere opened this issue Feb 18, 2025 · 1 comment · May be fixed by #46398
Assignees
Labels
area/hibernate-reactive Hibernate Reactive area/hibernate-validator Hibernate Validator kind/bug Something isn't working

Comments

@yrodiere
Copy link
Member

Describe the bug

It's unclear whether the Hibernate Validator integration in Hibernate Reactive, be it validation callbacks or DDL inference, works correctly at the moment.
In particular:

  1. Does it work correctly by default? E.g. does a @NotNull annotation on an attribute translate into a DDL constraint and/or a null validation on persist()?
  2. Does it comply with the configuration properties quarkus.hibernate-orm.validation.enabled and quarkus.hibernate-orm.validation.mode?

See #46264 (which fixes #46034):

.... we do not set the property in the reactive processor so it is defaulted to null which in turn behaves as AUTO so both CALLBACK and DDL work:

private static QuarkusPersistenceUnitDescriptor generateReactivePersistenceUnit(
HibernateOrmConfig hibernateOrmConfig, CombinedIndexBuildItem index,
HibernateOrmConfigPersistenceUnit persistenceUnitConfig,
JpaModelBuildItem jpaModel,
Optional<String> dbKindOptional,
Optional<String> explicitDialect,
Optional<String> explicitDbMinVersion,
ApplicationArchivesBuildItem applicationArchivesBuildItem,
LaunchMode launchMode,
BuildProducer<SystemPropertyBuildItem> systemProperties,
BuildProducer<NativeImageResourceBuildItem> nativeImageResources,
BuildProducer<HotDeploymentWatchedFileBuildItem> hotDeploymentWatchedFiles,
List<DatabaseKindDialectBuildItem> dbKindDialectBuildItems) {
//we have no persistence.xml so we will create a default one
String persistenceUnitConfigName = DEFAULT_PERSISTENCE_UNIT_NAME;
Map<String, Set<String>> modelClassesAndPackagesPerPersistencesUnits = HibernateOrmProcessor
.getModelClassesAndPackagesPerPersistenceUnits(hibernateOrmConfig, jpaModel, index.getIndex(), true);
Set<String> nonDefaultPUWithModelClassesOrPackages = modelClassesAndPackagesPerPersistencesUnits.entrySet().stream()
.filter(e -> !DEFAULT_PERSISTENCE_UNIT_NAME.equals(e.getKey()) && !e.getValue().isEmpty())
.map(Map.Entry::getKey)
.collect(Collectors.toSet());
if (!nonDefaultPUWithModelClassesOrPackages.isEmpty()) {
// Not supported yet; see https://github.com/quarkusio/quarkus/issues/21110
LOG.warnf("Entities are affected to non-default Hibernate Reactive persistence units %s."
+ " Since Hibernate Reactive only works with the default persistence unit, those entities will be ignored.",
nonDefaultPUWithModelClassesOrPackages);
}
Set<String> modelClassesAndPackages = modelClassesAndPackagesPerPersistencesUnits
.getOrDefault(DEFAULT_PERSISTENCE_UNIT_NAME, Collections.emptySet());
if (modelClassesAndPackages.isEmpty()) {
LOG.warnf("Could not find any entities affected to the Hibernate Reactive persistence unit.");
}
QuarkusPersistenceUnitDescriptor desc = new QuarkusPersistenceUnitDescriptor(
HibernateReactive.DEFAULT_REACTIVE_PERSISTENCE_UNIT_NAME, persistenceUnitConfigName,
PersistenceUnitTransactionType.RESOURCE_LOCAL,
new ArrayList<>(modelClassesAndPackages),
new Properties());
setDialectAndStorageEngine(dbKindOptional, explicitDialect, explicitDbMinVersion, dbKindDialectBuildItems,
persistenceUnitConfig.dialect().storageEngine(), systemProperties, desc);
// Physical Naming Strategy
persistenceUnitConfig.physicalNamingStrategy().ifPresent(
namingStrategy -> desc.getProperties()
.setProperty(AvailableSettings.PHYSICAL_NAMING_STRATEGY, namingStrategy));
// Implicit Naming Strategy
persistenceUnitConfig.implicitNamingStrategy().ifPresent(
namingStrategy -> desc.getProperties()
.setProperty(AvailableSettings.IMPLICIT_NAMING_STRATEGY, namingStrategy));
// Mapping
if (persistenceUnitConfig.mapping().timezone().timeZoneDefaultStorage().isPresent()) {
desc.getProperties().setProperty(AvailableSettings.TIMEZONE_DEFAULT_STORAGE,
persistenceUnitConfig.mapping().timezone().timeZoneDefaultStorage().get().name());
}
desc.getProperties().setProperty(AvailableSettings.PREFERRED_POOLED_OPTIMIZER,
persistenceUnitConfig.mapping().id().optimizer().idOptimizerDefault()
.orElse(HibernateOrmConfigPersistenceUnit.IdOptimizerType.POOLED_LO).configName);
//charset
desc.getProperties().setProperty(AvailableSettings.HBM2DDL_CHARSET_NAME,
persistenceUnitConfig.database().charset().name());
// Quoting strategy
if (persistenceUnitConfig.quoteIdentifiers().strategy() == IdentifierQuotingStrategy.ALL
|| persistenceUnitConfig.quoteIdentifiers()
.strategy() == IdentifierQuotingStrategy.ALL_EXCEPT_COLUMN_DEFINITIONS
|| persistenceUnitConfig.database().globallyQuotedIdentifiers()) {
desc.getProperties().setProperty(AvailableSettings.GLOBALLY_QUOTED_IDENTIFIERS, "true");
}
if (persistenceUnitConfig.quoteIdentifiers().strategy() == IdentifierQuotingStrategy.ALL_EXCEPT_COLUMN_DEFINITIONS) {
desc.getProperties().setProperty(AvailableSettings.GLOBALLY_QUOTED_IDENTIFIERS_SKIP_COLUMN_DEFINITIONS, "true");
} else if (persistenceUnitConfig.quoteIdentifiers().strategy() == IdentifierQuotingStrategy.ONLY_KEYWORDS) {
desc.getProperties().setProperty(AvailableSettings.KEYWORD_AUTO_QUOTING_ENABLED, "true");
}
// Query
// TODO ideally we should align on ORM and use 16 as a default, but that would break applications
// because of https://github.com/hibernate/hibernate-reactive/issues/742
int batchSize = firstPresent(persistenceUnitConfig.fetch().batchSize(), persistenceUnitConfig.batchFetchSize())
.orElse(-1);
if (batchSize > 0) {
desc.getProperties().setProperty(AvailableSettings.DEFAULT_BATCH_FETCH_SIZE,
Integer.toString(batchSize));
desc.getProperties().setProperty(AvailableSettings.BATCH_FETCH_STYLE, BatchFetchStyle.PADDED.toString());
}
if (persistenceUnitConfig.fetch().maxDepth().isPresent()) {
setMaxFetchDepth(desc, persistenceUnitConfig.fetch().maxDepth());
} else if (persistenceUnitConfig.maxFetchDepth().isPresent()) {
setMaxFetchDepth(desc, persistenceUnitConfig.maxFetchDepth());
}
desc.getProperties().setProperty(AvailableSettings.QUERY_PLAN_CACHE_MAX_SIZE, Integer.toString(
persistenceUnitConfig.query().queryPlanCacheMaxSize()));
desc.getProperties().setProperty(AvailableSettings.DEFAULT_NULL_ORDERING,
persistenceUnitConfig.query().defaultNullOrdering().name().toLowerCase());
desc.getProperties().setProperty(AvailableSettings.IN_CLAUSE_PARAMETER_PADDING,
String.valueOf(persistenceUnitConfig.query().inClauseParameterPadding()));
// JDBC
persistenceUnitConfig.jdbc().timezone().ifPresent(
timezone -> desc.getProperties().setProperty(AvailableSettings.JDBC_TIME_ZONE, timezone));
persistenceUnitConfig.jdbc().statementFetchSize().ifPresent(
fetchSize -> desc.getProperties().setProperty(AvailableSettings.STATEMENT_FETCH_SIZE,
String.valueOf(fetchSize)));
persistenceUnitConfig.jdbc().statementBatchSize().ifPresent(
statementBatchSize -> desc.getProperties().setProperty(AvailableSettings.STATEMENT_BATCH_SIZE,
String.valueOf(statementBatchSize)));
// Statistics
if (hibernateOrmConfig.metrics().enabled()
|| (hibernateOrmConfig.statistics().isPresent() && hibernateOrmConfig.statistics().get())) {
desc.getProperties().setProperty(AvailableSettings.GENERATE_STATISTICS, "true");
}
// sql-load-script
List<String> importFiles = getSqlLoadScript(persistenceUnitConfig.sqlLoadScript(), launchMode);
if (!importFiles.isEmpty()) {
for (String importFile : importFiles) {
Path loadScriptPath = applicationArchivesBuildItem.getRootArchive().getChildPath(importFile);
if (loadScriptPath != null && !Files.isDirectory(loadScriptPath)) {
// enlist resource if present
nativeImageResources.produce(new NativeImageResourceBuildItem(importFile));
hotDeploymentWatchedFiles.produce(new HotDeploymentWatchedFileBuildItem(importFile));
} else if (persistenceUnitConfig.sqlLoadScript().isPresent()) {
//raise exception if explicit file is not present (i.e. not the default)
String propertyName = HibernateOrmRuntimeConfig.puPropertyKey(persistenceUnitConfigName, "sql-load-script");
throw new ConfigurationException(
"Unable to find file referenced in '"
+ propertyName + "="
+ String.join(",", persistenceUnitConfig.sqlLoadScript().get())
+ "'. Remove property or add file to your path.",
Collections.singleton(propertyName));
}
}
if (persistenceUnitConfig.sqlLoadScript().isPresent()) {
desc.getProperties().setProperty(AvailableSettings.HBM2DDL_IMPORT_FILES, String.join(",", importFiles));
}
} else {
//Disable implicit loading of the default import script (import.sql)
desc.getProperties().setProperty(AvailableSettings.HBM2DDL_IMPORT_FILES, "");
}
// Caching
if (persistenceUnitConfig.secondLevelCachingEnabled()) {
Properties p = desc.getProperties();
//Only set these if the user isn't making an explicit choice:
p.putIfAbsent(USE_DIRECT_REFERENCE_CACHE_ENTRIES, Boolean.TRUE);
p.putIfAbsent(USE_SECOND_LEVEL_CACHE, Boolean.TRUE);
p.putIfAbsent(USE_QUERY_CACHE, Boolean.TRUE);
p.putIfAbsent(JAKARTA_SHARED_CACHE_MODE, SharedCacheMode.ENABLE_SELECTIVE);
Map<String, String> cacheConfigEntries = HibernateConfigUtil.getCacheConfigEntries(persistenceUnitConfig);
for (Entry<String, String> entry : cacheConfigEntries.entrySet()) {
desc.getProperties().setProperty(entry.getKey(), entry.getValue());
}
} else {
//Unless the global switch is explicitly set to off, in which case we disable all caching:
Properties p = desc.getProperties();
p.put(USE_DIRECT_REFERENCE_CACHE_ENTRIES, Boolean.FALSE);
p.put(USE_SECOND_LEVEL_CACHE, Boolean.FALSE);
p.put(USE_QUERY_CACHE, Boolean.FALSE);
p.put(JAKARTA_SHARED_CACHE_MODE, SharedCacheMode.NONE);
}
return desc;
}

Expected behavior

Tests and documentation.

Actual behavior

No tests and sparse, possibly incorrect documentation.

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

3.18

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

I think we should:

  1. Port to the Hibernate Reactive extension the tests introduced in Introduce validation mode instead of a boolean flag for BV integration in ORM #46264 for the Hibernate ORM extension.
  2. Fix/report whatever doesn't work correctly.
  3. Possibly document the Hibernate Validator integration in the Hibernate ORM/Reactive extensions. Currently the only documentation seems to be that of a configuration property.
@yrodiere yrodiere added the kind/bug Something isn't working label Feb 18, 2025
@quarkus-bot quarkus-bot bot added area/hibernate-validator Hibernate Validator area/hibernate-reactive Hibernate Reactive labels Feb 18, 2025
Copy link

quarkus-bot bot commented Feb 18, 2025

/cc @DavideD (hibernate-reactive), @gavinking (hibernate-reactive), @gsmet (hibernate-validator), @marko-bekhta (hibernate-validator)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/hibernate-reactive Hibernate Reactive area/hibernate-validator Hibernate Validator kind/bug Something isn't working
Projects
None yet
2 participants