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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

HSEARCH-3319 WIP: DRAFT: IDEA: TEST: Type-safe field references #4113

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

HSEARCH-3319 WIP: DRAFT: IDEA: TEST: Type-safe field references #4113

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d7de38d
HSEARCH-3319 WIP: DRAFT: IDEA: TEST: Type-safe field references
marko-bekhta Apr 9, 2024
33d7e5b
HSEARCH-3319 WIP: a few changes
marko-bekhta May 7, 2024
bdedc9b
HSEARCH-3319 WIP: add a test
marko-bekhta May 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
254 changes: 254 additions & 0 deletions ...n/src/test/java/org/hibernate/search/documentation/search/predicate/FieldReferenceIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,254 @@
/*
* Hibernate Search, full-text search for your domain model
*
* License: GNU Lesser General Public License (LGPL), version 2.1 or later
* See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
*/
package org.hibernate.search.documentation.search.predicate;

import static org.assertj.core.api.Assertions.assertThat;
import static org.hibernate.search.util.impl.integrationtest.mapper.orm.OrmUtils.with;

import java.util.List;
import java.util.function.Consumer;

import jakarta.persistence.Entity;
import jakarta.persistence.EntityManagerFactory;
import jakarta.persistence.Id;

import org.hibernate.search.documentation.testsupport.BackendConfigurations;
import org.hibernate.search.documentation.testsupport.DocumentationSetupHelper;
import org.hibernate.search.engine.backend.types.Projectable;
import org.hibernate.search.engine.search.common.ValueConvert;
import org.hibernate.search.engine.search.predicate.SearchPredicate;
import org.hibernate.search.engine.search.reference.traits.predicate.MatchPredicateFieldReference;
import org.hibernate.search.engine.search.reference.traits.projection.FieldProjectionFieldReference;
import org.hibernate.search.mapper.orm.Search;
import org.hibernate.search.mapper.orm.scope.SearchScope;
import org.hibernate.search.mapper.orm.session.SearchSession;
import org.hibernate.search.mapper.pojo.mapping.definition.annotation.FullTextField;
import org.hibernate.search.mapper.pojo.mapping.definition.annotation.Indexed;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

class FieldReferenceIT {

@RegisterExtension
public DocumentationSetupHelper setupHelper = DocumentationSetupHelper.withSingleBackend( BackendConfigurations.simple() );

private EntityManagerFactory entityManagerFactory;

@BeforeEach
void setup() {
entityManagerFactory = setupHelper.start().setup( EntityA.class, EntityB.class, EntityC.class );
initData();
}

@Test
void smoke() {
withinSearchSession( searchSession -> {
assertThat(
searchSession.search( EntityA.class )
.select( f -> f.field( EntityA_.stringA ) )
.where( f -> f.match().field( EntityA_.stringA ).matching( "a" ) )
.fetchHits( 20 )
).containsOnly( "a" );

assertThat(
searchSession.search( EntityA.class )
.select( f -> f.field( EntityC_.stringA ) )
.where( f -> f.match().field( EntityC_.stringC ).matching( "c" ) )
.fetchHits( 20 )
).containsOnly( "c" );

SearchScope<EntityB> scope = searchSession.scope( List.of( EntityB.class, EntityC.class ) );
SearchPredicate searchPredicate = scope.predicate().match().field( EntityB_.stringA ).matching( "b" ).toPredicate();
SearchPredicate searchPredicate2 =
scope.predicate().match().field( EntityC_.stringC ).matching( "c" ).toPredicate();

assertThat(
searchSession.search( scope )
.select( f -> f.field( EntityC_.stringA ) )
.where( searchPredicate )
.fetchHits( 20 )
).containsOnly( "b" );

assertThat(
searchSession.search( scope )
.select( f -> f.field( EntityC_.stringA ) )
.where( searchPredicate2 )
.fetchHits( 20 )
).containsOnly( "c" );
Comment on lines +66 to +83
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This particular example worked nice, but ....

if we'd have a

@Entity
@Indexed
public static class EntityA {
    @Id
    Long id;

    @FullTextField(projectable = Projectable.YES)
    String stringA;
}

@Entity
@Indexed
public static class EntityB {
    @Id
    Long id;

    @FullTextField(projectable = Projectable.YES)
    String stringA;

    @FullTextField(projectable = Projectable.YES)
    String stringB;
}

@Entity
@Indexed
public static class EntityC {
    @Id
    Long id;

    @FullTextField(projectable = Projectable.YES)
    String stringA;

    @FullTextField(projectable = Projectable.YES)
    String stringC;
}

then the best scope limit we'd have would be an Object:

SearchScope<Object> scope = searchSession.scope( List.of( Entity2B.class, Entity2C.class ) );

and with that, anything can be applied:

// while the path is there, the EntityB isn't in the scope
SearchPredicate searchPredicate = scope.predicate().match().field( EntityB_.stringA ).matching( "b" ).toPredicate();

// while the path is there, what is EntityC ? 
.select( f -> f.field( EntityC_.stringA ) )

I was thinking about using something else instead of entity type, maybe the generated root type EntityA_ but for that I haven't found a nice entry point to pass the type. Unless we also introduce:

searchSession.search( EntityA_.self() )

// where self returns something like
interface ScopeReference<T, E> {
    Class<T> rootReferenceType(); // EntityA_
    Set<Class<? extends E> scopeClasses(); // EntityA.class ... 
}

and similar things for the scope methods receiving this generated "root" (which should also work with those unions we've been talking about on the call yesterday)...

WDYT @yrodiere ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd have expected field(...) to accept a *Reference<? super E>, see my other comment; but maybe I forgot what our conclusions were yesterday.
In any case, <? super E> instead of <? extends E> would solve your specific problem here.

For targeting multiple types in the same query, we would address that as a follow-up with the "generated union types".

For targeting fields that are only present in some subtypes of the target type, we would address that as part of https://hibernate.atlassian.net/browse/HSEARCH-3434 (though it would be about much more than predicates, obviously).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking a look at this馃槂 !
+1 on ? super E

For targeting multiple types in the same query, we would address that as a follow-up with the "generated union types".

Yeah, that's what I've started to think about, and how we could do it, so we wouldn't need to go through all these interfaces one more time 馃檲 馃槇
That's why I was thinking if we do this:

searchSession.search( EntityA_.scope() )

// where scope returns something like
interface ScopeReference<R, T> {
    Class<R> rootReferenceType(); // EntityA_
    Set<Class<T> scopeClasses(); // EntityA.class ... 
}

And then the "generated union type" is e.g.:

public static class Entity2A_union_Entity2B_ {
	public static ValueFieldReference1<Entity2A_union_Entity2B_, String, String, String> stringA;

	ReferenceScope<Entity2A_union_Entity2B_, Object> scope() {
		return new ReferenceScope<>() {
			@Override
			public Class<Entity2A_union_Entity2B_> rootReferenceType() {
				return Entity2A_union_Entity2B_.class;
			}

			@Override
			public Set<Class<?>> scopeClasses() {
				return Set.of( Entity2A.class, Entity2B.class );
			}
		};
	}
	
	static {
		stringA = ValueFieldReference1.of( "stringA", Entity2A_union_Entity2B_.class, String.class, String.class,
				String.class );
	}
}

and for non-union:

public static class Entity2A_ {
	public static ValueFieldReference1<Entity2A_, String, String, String> stringA;

	ReferenceScope<Entity2A_, Entity2A> scope() {
		return new ReferenceScope<>() {
			@Override
			public Class<Entity2A_> rootReferenceType() {
				return Entity2A_.class;
			}

			@Override
			public Set<Class<Entity2A>> scopeClasses() {
				return Set.of( Entity2A.class );
			}
		};
	}

	static {
		stringA = ValueFieldReference1.of( "stringA", Entity2A_.class, String.class, String.class, String.class );
	}
}

with that, we won't even need ? super R it'll all just be strictly limited to R itself.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's why I was thinking if we do this:

Makes sense, but using the word scope is likely to cause confusion... We'll need to think about it.

And then the "generated union type" is e.g.:

LGTM.

with that, we won't even need ? super R it'll all just be strictly limited to R itself.

What about when an entity extends another, though? We may want to use fields from the supertype's static metamodel.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#4156

I've tried out this idea in this other PR ^.

searchSession.search( EntityC_.scope )
	.select( f -> f.field( EntityC_.stringA ) )
	.where( f -> f.match().field( EntityC_.stringC ).matching( "c" ) )
	.fetchHits( 20 )
SearchScope<EntityB_union_EntityC_, EntityB> scope = EntityB_union_EntityC_.scope.scope( searchSession );
searchSession.search( scope )
	.select( f -> f.field( EntityB_union_EntityC_.stringA ) )
	.where( searchPredicate )
	.fetchHits( 20 )

Makes sense, but using the word scope is likely to cause confusion... We'll need to think about it.

yeah ... that's true 馃槂
so maybe instead of EntityB_union_EntityC_.scope.scope( searchSession ); do EntityB_union_EntityC_.scope.create( searchSession );

What about when an entity extends another, though? We may want to use fields from the supertype's static metamodel.

In that other PR everything is tied to the generated class, e.g. EntityC_, and I was thinking that if we start with the EntityC_ there's no reason to switch to a different type as this EntityC_ should contain all that is accessible within the scope created by that same generated class EntityC_

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that other PR everything is tied to the generated class, e.g. EntityC_, and I was thinking that if we start with the EntityC_ there's no reason to switch to a different type as this EntityC_ should contain all that is accessible within the scope created by that same generated class EntityC_

There definitely is a reason: reuse. Think of people having entities that extend e.g. some AuditableEntity with a creationDate and lastUpdateDate. I can definitely see them define a util to add a predicate on any subtype to restrict by last update date, and that util will not be able to take advantage of the metamodel of the specific subtype, it'll have to rely on AuditableEntity_.

... Which I guess raises the question of @MappedSuperclass xD Not today, though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, ok 馃槂 we probably can make it work for the "root" types, the inheritance part, I mean.. but not for any embeddable. (I'll give it a try in that PR)


} );
}

private void initData() {
with( entityManagerFactory ).runInTransaction( entityManager -> {
EntityA a = new EntityA();
a.id = 1L;
a.stringA = "a";

EntityB b = new EntityB();
b.id = 10L;
b.stringA = "b";
b.stringB = "b";

EntityC c = new EntityC();
c.id = 100L;
c.stringA = "c";
c.stringB = "c";
c.stringC = "c";

entityManager.persist( a );
entityManager.persist( b );
entityManager.persist( c );
} );
}

@Indexed
@Entity
public static class EntityA {
@Id
Long id;

@FullTextField(projectable = Projectable.YES)
String stringA;

}

@Entity
public static class EntityB extends EntityA {

@FullTextField(projectable = Projectable.YES)
String stringB;

}

@Entity
public static class EntityC extends EntityB {

@FullTextField(projectable = Projectable.YES)
String stringC;

}


public static class EntityA_ {
public static ValueFieldReference1<EntityA, String, String, String> stringA;

static {
stringA = ValueFieldReference1.of( "stringA", EntityA.class, String.class, String.class, String.class );
}
}

public static class EntityB_ {
public static ValueFieldReference1<EntityB, String, String, String> stringA;
public static ValueFieldReference1<EntityB, String, String, String> stringB;

static {
stringA = ValueFieldReference1.of( "stringA", EntityB.class, String.class, String.class, String.class );
stringB = ValueFieldReference1.of( "stringB", EntityB.class, String.class, String.class, String.class );
}
}

public static class EntityC_ {
public static ValueFieldReference1<EntityC, String, String, String> stringA;
public static ValueFieldReference1<EntityC, String, String, String> stringB;
public static ValueFieldReference1<EntityC, String, String, String> stringC;

static {
stringA = ValueFieldReference1.of( "stringA", EntityC.class, String.class, String.class, String.class );
stringB = ValueFieldReference1.of( "stringB", EntityC.class, String.class, String.class, String.class );
stringC = ValueFieldReference1.of( "stringC", EntityC.class, String.class, String.class, String.class );
}
}


public static class ValueFieldReference1<E, T, V, P> extends TypedFieldReference1<E, T, P> {

public static <E, T, V, P> ValueFieldReference1<E, T, V, P> of(
String path,
Class<E> documentReferenceClass,
Class<T> t,
Class<V> v,
Class<P> p) {
return new ValueFieldReference1<>( path, documentReferenceClass, t, v, p );
}

private final TypedFieldReference1<E, V, V> noConverter;
private final TypedFieldReference1<E, String, String> string;

public ValueFieldReference1(String absolutePath, Class<E> containing, Class<T> inputType, Class<V> indexType,
Class<P> projectionType) {
super( absolutePath, ValueConvert.YES, containing, inputType, projectionType );
this.noConverter = new TypedFieldReference1<>( absolutePath, ValueConvert.NO, containing, indexType, indexType );
this.string =
new TypedFieldReference1<>( absolutePath, ValueConvert.PARSE, containing, String.class, String.class );
}

public TypedFieldReference1<E, V, V> noConverter() {
return noConverter;
}


public TypedFieldReference1<E, String, String> asString() {
return string;
}

}

public static class TypedFieldReference1<E, T, P>
implements FieldProjectionFieldReference<E, P>,
MatchPredicateFieldReference<E, T> {

private final String absolutePath;
private final ValueConvert valueConvert;
private final Class<E> containing;
private final Class<T> input;
private final Class<P> projection;

public TypedFieldReference1(String absolutePath, ValueConvert valueConvert, Class<E> containing, Class<T> input,
Class<P> projection) {
this.absolutePath = absolutePath;
this.valueConvert = valueConvert;
this.containing = containing;
this.input = input;
this.projection = projection;
}

@Override
public String absolutePath() {
return absolutePath;
}

@Override
public Class<T> predicateType() {
return input;
}

@Override
public ValueConvert valueConvert() {
return valueConvert;
}

@Override
public Class<P> projectionType() {
return projection;
}

@Override
public Class<E> containing() {
return containing;
}
}

private void withinSearchSession(Consumer<SearchSession> action) {
with( entityManagerFactory ).runInTransaction( entityManager -> {
SearchSession searchSession = Search.session( entityManager );
action.accept( searchSession );
} );
}
}