Skip to content

Commit

Permalink
HSEARCH-4980 Restore support for embedded IDs in Jakarta Batch job
Browse files Browse the repository at this point in the history 
These tests shouldn't have been disabled in the first place.
  • Loading branch information
@yrodiere
yrodiere committed Sep 29, 2023
1 parent 5add68e commit fe81398
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 99 deletions.
28 changes: 13 additions & 15 deletions ...te/search/integrationtest/batch/jsr352/massindexing/MassIndexingJobWithCompositeIdIT.java
View file
Expand Up @@ -93,7 +93,7 @@ public void initData() throws Exception {

@Test
@Ignore("HSEARCH-4033") // TODO HSEARCH-4033 Support mass-indexing of composite id entities
public void canHandleIdClass_strategyFull() throws Exception {
public void canHandleIdClass() throws Exception {
Properties props = MassIndexingJob.parameters()
.forEntities( EntityWithIdClass.class )
.rowsPerPartition( 13 ) // Ensure there're more than 1 partition, so that a WHERE clause is applied.
Expand All @@ -107,10 +107,10 @@ public void canHandleIdClass_strategyFull() throws Exception {

@Test
@Ignore("HSEARCH-4033") // TODO HSEARCH-4033 Support mass-indexing of composite id entities
public void canHandleIdClass_strategyHql() throws Exception {
public void canHandleIdClass_hql() throws Exception {
Properties props = MassIndexingJob.parameters()
.forEntities( EntityWithIdClass.class )
.restrictedBy( "select e from EntityWithIdClass e where e.month = 6" )
.restrictedBy( "select e from EntityWithIdClass e where e.month >= 7" )
.rowsPerPartition( 13 ) // Ensure there're more than 1 partition, so that a WHERE clause is applied.
.checkpointInterval( 4 )
.build();
Expand All @@ -122,8 +122,7 @@ public void canHandleIdClass_strategyHql() throws Exception {
}

@Test
@Ignore("HSEARCH-4033") // TODO HSEARCH-4033 Support mass-indexing of composite id entities
public void canHandleEmbeddedId_strategyFull() throws Exception {
public void canHandleEmbeddedId() throws Exception {
Properties props = MassIndexingJob.parameters()
.forEntities( EntityWithEmbeddedId.class )
.rowsPerPartition( 13 ) // Ensure there're more than 1 partition, so that a WHERE clause is applied.
Expand All @@ -138,11 +137,10 @@ public void canHandleEmbeddedId_strategyFull() throws Exception {
}

@Test
@Ignore("HSEARCH-4033") // TODO HSEARCH-4033 Support mass-indexing of composite id entities
public void canHandleEmbeddedId_strategyHql() throws Exception {
public void canHandleEmbeddedId_hql() throws Exception {
Properties props = MassIndexingJob.parameters()
.forEntities( EntityWithEmbeddedId.class )
.restrictedBy( "select e from EntityWithIdClass e where e.embeddableDateId.month = 6" )
.restrictedBy( "select e from EntityWithEmbeddedId e where e.embeddableDateId.month >= 7" )
.rowsPerPartition( 13 ) // Ensure there're more than 1 partition, so that a WHERE clause is applied.
.checkpointInterval( 4 )
.build();
Expand All @@ -165,14 +163,14 @@ public static class EntityWithEmbeddedId {
private EmbeddableDateId embeddableDateId;

@FullTextField
private String value;
private String text;

public EntityWithEmbeddedId() {
}

public EntityWithEmbeddedId(LocalDate d) {
this.embeddableDateId = new EmbeddableDateId( d );
this.value = DateTimeFormatter.ofPattern( "yyyyMMdd", Locale.ROOT ).format( d );
this.text = DateTimeFormatter.ofPattern( "yyyyMMdd", Locale.ROOT ).format( d );
}

public EmbeddableDateId getEmbeddableDateId() {
Expand All @@ -183,17 +181,17 @@ public void setEmbeddableDateId(EmbeddableDateId embeddableDateId) {
this.embeddableDateId = embeddableDateId;
}

public String getValue() {
return value;
public String getText() {
return text;
}

public void setValue(String value) {
this.value = value;
public void setText(String text) {
this.text = text;
}

@Override
public String toString() {
return "MyDate [embeddableDateId=" + embeddableDateId + ", value=" + value + "]";
return "MyDate [embeddableDateId=" + embeddableDateId + ", text=" + text + "]";
}

/**
Expand Down
55 changes: 27 additions & 28 deletions .../java/org/hibernate/search/batch/jsr352/core/massindexing/util/impl/CompositeIdOrder.java
View file
Expand Up @@ -17,7 +17,9 @@
import javax.persistence.IdClass;
import javax.persistence.criteria.CriteriaBuilder;
import javax.persistence.criteria.CriteriaQuery;
import javax.persistence.criteria.Expression;
import javax.persistence.criteria.Order;
import javax.persistence.criteria.Path;
import javax.persistence.criteria.Predicate;
import javax.persistence.criteria.Root;

Expand Down Expand Up @@ -78,43 +80,34 @@ public CompositeIdOrder(String componentPath, ComponentType componentType) {
}

@Override
@SuppressWarnings("unchecked")
public Predicate idGreater(CriteriaBuilder builder, Root<?> root, Object idObj) {
BiFunction<String, Object, Predicate> strictOperator =
(String path, Object obj) -> builder.greaterThan( root.get( path ), (Comparable<? super Object>) idObj );
return restrictLexicographically( strictOperator, builder, root, idObj, false );
return restrictLexicographically( builder::greaterThan, builder, root, idObj, false );
}

@Override
@SuppressWarnings("unchecked")
public Predicate idGreaterOrEqual(CriteriaBuilder builder, Root<?> root, Object idObj) {
/*
* Caution, using Restrictions::ge here won't cut it, we really need
* to separate the strict operator from the equals.
*/
BiFunction<String, Object, Predicate> strictOperator =
(String path, Object obj) -> builder.greaterThan( root.get( path ), (Comparable<? super Object>) idObj );
return restrictLexicographically( strictOperator, builder, root, idObj, true );
// Caution, using 'builder::greaterThanOrEqualTo' here won't cut it, we really need
// to separate the strict operator from the equals.
return restrictLexicographically( builder::greaterThan, builder, root, idObj, true );
}

@Override
@SuppressWarnings("unchecked")
public Predicate idLesser(CriteriaBuilder builder, Root<?> root, Object idObj) {
BiFunction<String, Object, Predicate> strictOperator =
(String path, Object obj) -> builder.lessThan( root.get( path ), (Comparable<? super Object>) idObj );
return restrictLexicographically( strictOperator, builder, root, idObj, false );
return restrictLexicographically( builder::lessThan, builder, root, idObj, false );
}

@Override
public void addAscOrder(CriteriaBuilder builder, CriteriaQuery<?> criteria, Root<?> root) {
ArrayList<Order> orders = new ArrayList<>( propertyPaths.size() );
for ( String path : propertyPaths ) {
orders.add( builder.asc( root.get( path ) ) );
for ( String pathString : propertyPaths ) {
orders.add( builder.asc( toPath( root, pathString ) ) );
}
criteria.orderBy( orders );
}

private Predicate restrictLexicographically(BiFunction<String, Object, Predicate> strictOperator,
@SuppressWarnings("unchecked")
private Predicate restrictLexicographically(
BiFunction<Expression<Comparable<? super Object>>, Comparable<? super Object>, Predicate> strictOperator,
CriteriaBuilder builder, Root<?> root, Object idObj, boolean orEquals) {
int propertyPathsSize = propertyPaths.size();
int expressionsInOr = propertyPathsSize + ( orEquals ? 1 : 0 );
Expand All @@ -127,24 +120,21 @@ private Predicate restrictLexicographically(BiFunction<String, Object, Predicate
int j = 0;
for ( ; j < and.length - 1; j++ ) {
// The first N-1 expressions have symbol `=`
String path = propertyPaths.get( j );
Object val = getPropertyValue( idObj, j );
and[j] = builder.equal( root.get( path ), val );
and[j] = builder.equal( toPath( root, propertyPaths.get( j ) ),
getPropertyValue( idObj, j ) );
}
// The last expression has whatever symbol is defined by "strictOperator"
String path = propertyPaths.get( j );
Object val = getPropertyValue( idObj, j );
and[j] = strictOperator.apply( path, val );
and[j] = strictOperator.apply( toPath( root, propertyPaths.get( j ) ),
(Comparable<? super Object>) getPropertyValue( idObj, j ) );

or[i] = builder.and( and );
}

if ( orEquals ) {
Predicate[] and = new Predicate[propertyPathsSize];
for ( int i = 0; i < propertyPathsSize; i++ ) {
String path = propertyPaths.get( i );
Object val = getPropertyValue( idObj, i );
and[i] = builder.equal( root.get( path ), val );
and[i] = builder.equal( toPath( root, propertyPaths.get( i ) ),
getPropertyValue( idObj, i ) );
}
or[or.length - 1] = builder.and( and );
}
Expand All @@ -153,6 +143,15 @@ private Predicate restrictLexicographically(BiFunction<String, Object, Predicate
return builder.or( or );
}

@SuppressWarnings("unchecked")
private <T> Path<T> toPath(Path<?> parent, String pathString) {
Path<?> result = parent;
for ( String pathElement : pathString.split( "\\." ) ) {
result = result.get( pathElement );
}
return (Path<T>) result;
}

private Object getPropertyValue(Object obj, int ourIndex) {
int theirIndex = propertyIndices.get( ourIndex );
return componentType.getPropertyValue( obj, theirIndex );
Expand Down
95 changes: 55 additions & 40 deletions orm6/mapper/orm-batch-jsr352/core/ant-src-changes.patch
View file
Expand Up @@ -96,7 +96,7 @@ index 9d1db50953..237341e6a5 100644
}

diff --git a/main/java/org/hibernate/search/batch/jsr352/core/massindexing/util/impl/CompositeIdOrder.java b/main/java/org/hibernate/search/batch/jsr352/core/massindexing/util/impl/CompositeIdOrder.java
index 34524445c8..7a1b35c4b0 100644
index cd105c6dbe..2aab3bce78 100644
--- a/main/java/org/hibernate/search/batch/jsr352/core/massindexing/util/impl/CompositeIdOrder.java
+++ b/main/java/org/hibernate/search/batch/jsr352/core/massindexing/util/impl/CompositeIdOrder.java
@@ -7,10 +7,7 @@
Expand All @@ -110,23 +110,25 @@ index 34524445c8..7a1b35c4b0 100644
import java.util.function.BiFunction;

import jakarta.persistence.EmbeddedId;
@@ -21,7 +18,8 @@
@@ -23,7 +20,10 @@
import jakarta.persistence.criteria.Predicate;
import jakarta.persistence.criteria.Root;

-import org.hibernate.type.ComponentType;
+import org.hibernate.metamodel.mapping.EmbeddableMappingType;
+import org.hibernate.metamodel.mapping.EntityIdentifierMapping;
+import org.hibernate.metamodel.mapping.ModelPart;
+import org.hibernate.metamodel.model.domain.NavigableRole;

/**
* Order over multiple ID attributes.
@@ -49,32 +47,12 @@
@@ -51,32 +51,12 @@
*/
public class CompositeIdOrder implements IdOrder {

- private final ComponentType componentType;
+ private final EntityIdentifierMapping mapping;
+ private final EmbeddableMappingType mappingType;
+ private final EntityIdentifierMapping idMapping;
+ private final EmbeddableMappingType idMappingType;

- private final List<String> propertyPaths;
-
Expand All @@ -152,82 +154,76 @@ index 34524445c8..7a1b35c4b0 100644
- // Prepend the path prefix to each property; we will only use absolute path from now on
- iterator.set( pathPrefix + propertyName );
- }
+ public CompositeIdOrder(EntityIdentifierMapping mapping, EmbeddableMappingType mappingType) {
+ this.mapping = mapping;
+ this.mappingType = mappingType;
+ public CompositeIdOrder(EntityIdentifierMapping idMapping, EmbeddableMappingType idMappingType) {
+ this.idMapping = idMapping;
+ this.idMappingType = idMappingType;
}

@Override
@@ -107,54 +85,51 @@ public Predicate idLesser(CriteriaBuilder builder, Root<?> root, Object idObj) {
@@ -98,10 +78,8 @@ public Predicate idLesser(CriteriaBuilder builder, Root<?> root, Object idObj) {

@Override
public void addAscOrder(CriteriaBuilder builder, CriteriaQuery<?> criteria, Root<?> root) {
- ArrayList<Order> orders = new ArrayList<>( propertyPaths.size() );
- for ( String path : propertyPaths ) {
- orders.add( builder.asc( root.get( path ) ) );
- for ( String pathString : propertyPaths ) {
- orders.add( builder.asc( toPath( root, pathString ) ) );
- }
+ ArrayList<Order> orders = new ArrayList<>();
+ mapping.forEachSelectable( (i, selectable) ->
+ orders.add( builder.asc( root.get( selectable.getSelectablePath().getFullPath() ) ) ) );
+ idMappingType.forEachSubPart( (i, subPart) -> orders.add( builder.asc( toPath( root, subPart ) ) ) );
criteria.orderBy( orders );
}

private Predicate restrictLexicographically(BiFunction<String, Object, Predicate> strictOperator,
@@ -109,51 +87,49 @@ public void addAscOrder(CriteriaBuilder builder, CriteriaQuery<?> criteria, Root
private Predicate restrictLexicographically(
BiFunction<Expression<Comparable<? super Object>>, Comparable<? super Object>, Predicate> strictOperator,
CriteriaBuilder builder, Root<?> root, Object idObj, boolean orEquals) {
- int propertyPathsSize = propertyPaths.size();
- int expressionsInOr = propertyPathsSize + ( orEquals ? 1 : 0 );
+ Object[] selectableValues = mappingType.getValues( idObj );
+ int selectablesSize = selectableValues.length;

- Predicate[] or = new Predicate[expressionsInOr];
+ Object[] selectableValues = idMappingType.getValues( idObj );
+ List<Predicate> or = new ArrayList<>();

- Predicate[] or = new Predicate[expressionsInOr];
-
- for ( int i = 0; i < propertyPathsSize; i++ ) {
+ mapping.forEachSelectable( (i, selectable) -> {
+ idMappingType.forEachSubPart( (i, subPart) -> {
// Group expressions together in a single conjunction (A and B and C...).
Predicate[] and = new Predicate[i + 1];
- int j = 0;
- for ( ; j < and.length - 1; j++ ) {
- // The first N-1 expressions have symbol `=`
- String path = propertyPaths.get( j );
- Object val = getPropertyValue( idObj, j );
- and[j] = builder.equal( root.get( path ), val );
- and[j] = builder.equal( toPath( root, propertyPaths.get( j ) ),
- getPropertyValue( idObj, j ) );
- }
- // The last expression has whatever symbol is defined by "strictOperator"
- String path = propertyPaths.get( j );
- Object val = getPropertyValue( idObj, j );
- and[j] = strictOperator.apply( path, val );
- and[j] = strictOperator.apply( toPath( root, propertyPaths.get( j ) ),
- (Comparable<? super Object>) getPropertyValue( idObj, j ) );

- or[i] = builder.and( and );
- }
+ mapping.forEachSelectable( (j, previousSelectable) -> {
+ idMappingType.forEachSubPart( (j, previousSubPart) -> {
+ if ( j < i ) {
+ // The first N-1 expressions have symbol `=`
+ String path = previousSelectable.getSelectablePath().getFullPath();
+ Object val = selectableValues[j];
+ and[j] = builder.equal( root.get( path ), val );
+ and[j] = builder.equal( toPath( root, previousSubPart ),
+ selectableValues[j] );
+ }
+ } );
+ // The last expression has whatever symbol is defined by "strictOperator"
+ String path = selectable.getSelectablePath().getFullPath();
+ Object val = selectableValues[i];
+ and[i] = strictOperator.apply( path, val );
+ and[i] = strictOperator.apply( toPath( root, subPart ),
+ (Comparable<? super Object>) selectableValues[i] );
+
+ or.add( builder.and( and ) );
+ } );

if ( orEquals ) {
- Predicate[] and = new Predicate[propertyPathsSize];
- for ( int i = 0; i < propertyPathsSize; i++ ) {
- String path = propertyPaths.get( i );
- Object val = getPropertyValue( idObj, i );
+ Predicate[] and = new Predicate[selectablesSize];
+ mapping.forEachSelectable( (i, previousSelectable) -> {
+ String path = previousSelectable.getSelectablePath().getFullPath();
+ Object val = selectableValues[i];
and[i] = builder.equal( root.get( path ), val );
- and[i] = builder.equal( toPath( root, propertyPaths.get( i ) ),
- getPropertyValue( idObj, i ) );
- }
- or[or.length - 1] = builder.and( and );
+ Predicate[] and = new Predicate[selectableValues.length];
+ idMappingType.forEachSubPart( (i, subPart) -> {
+ and[i] = builder.equal( toPath( root, subPart ), selectableValues[i] );
+ } );
+ or.add( builder.and( and ) );
}
Expand All @@ -237,10 +233,29 @@ index 34524445c8..7a1b35c4b0 100644
+ return builder.or( or.toArray( new Predicate[0] ) );
}

- @SuppressWarnings("unchecked")
- private <T> Path<T> toPath(Path<?> parent, String pathString) {
- Path<?> result = parent;
- for ( String pathElement : pathString.split( "\\." ) ) {
- result = result.get( pathElement );
+ private <T> Path<T> toPath(Root<?> root, ModelPart subPart) {
+ return toPath( root, subPart.getNavigableRole() );
+ }
+
+ private <T> Path<T> toPath(Path<?> parent, NavigableRole role) {
+ if ( role == idMappingType.getNavigableRole() ) {
+ return parent.get( idMapping.getAttributeName() );
+ }
+ else {
+ return toPath( parent, role.getParent() ).get( role.getLocalName() );
}
- return (Path<T>) result;
- }
-
- private Object getPropertyValue(Object obj, int ourIndex) {
- int theirIndex = propertyIndices.get( ourIndex );
- return componentType.getPropertyValue( obj, theirIndex );
- }
}
}
diff --git a/main/java/org/hibernate/search/batch/jsr352/core/massindexing/util/impl/PersistenceUtil.java b/main/java/org/hibernate/search/batch/jsr352/core/massindexing/util/impl/PersistenceUtil.java
index 2faf9f6a2e..51639b2963 100644
Expand Down

0 comments on commit fe81398

Please sign in to comment.