From 79ae9d2e22fe6b73694e3f69b79434277a50523b Mon Sep 17 00:00:00 2001 From: Davide D'Alto Date: Fri, 15 Apr 2016 16:33:43 +0100 Subject: [PATCH] HSEARCH-2071 Use correct defaults for fields provided by field bridges For example, if the field bridge is called for an id, the custom fields won't be analyzed; when used as a normal field they are going to be analyzed. Elasticsearch should stick to the same behaviour. --- .../impl/ElasticsearchIndexManager.java | 41 ++++++++++++--- .../elasticsearch/impl/FieldHelper.java | 25 ++++++++++ .../impl/AnnotationMetadataProvider.java | 50 +++++++++---------- .../metadata/impl/BridgeDefinedField.java | 14 ++++-- .../impl/FieldMetadataBuilderImpl.java | 7 ++- 5 files changed, 100 insertions(+), 37 deletions(-) diff --git a/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/impl/ElasticsearchIndexManager.java b/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/impl/ElasticsearchIndexManager.java index 4cffceb6f2f..d85758ee954 100644 --- a/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/impl/ElasticsearchIndexManager.java +++ b/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/impl/ElasticsearchIndexManager.java @@ -12,6 +12,7 @@ import java.util.Set; import org.apache.lucene.analysis.Analyzer; +import org.apache.lucene.document.Field; import org.apache.lucene.search.similarities.Similarity; import org.hibernate.search.analyzer.impl.AnalyzerReference; import org.hibernate.search.analyzer.impl.RemoteAnalyzer; @@ -345,11 +346,9 @@ private void addFieldMapping(JsonObject payload, EntityIndexBinding descriptor, String index = getIndex( descriptor, fieldMetadata ); field.addProperty( "index", index ); - if ( isAnalyzed( index ) && fieldMetadata.getAnalyzerReference() != null ) { - String analyzerName = analyzerName( descriptor.getDocumentBuilder().getBeanClass(), fieldMetadata ); - if ( analyzerName != null ) { - field.addProperty( "analyzer", analyzerName ); - } + String analyzerName = getAnalyzerName( descriptor, fieldMetadata, index ); + if ( analyzerName != null ) { + field.addProperty( "analyzer", analyzerName ); } if ( fieldMetadata.getBoost() != null ) { @@ -372,6 +371,13 @@ private void addFieldMapping(JsonObject payload, EntityIndexBinding descriptor, } } + private String getAnalyzerName(EntityIndexBinding descriptor, DocumentFieldMetadata fieldMetadata, String index) { + if ( isAnalyzed( index ) && fieldMetadata.getAnalyzerReference() != null ) { + return analyzerName( descriptor.getDocumentBuilder().getBeanClass(), fieldMetadata ); + } + return null; + } + private boolean isAnalyzed(String index) { return ANALYZED.equals( index ); } @@ -386,7 +392,9 @@ private void addFieldMapping(JsonObject payload, BridgeDefinedField bridgeDefine JsonObject field = new JsonObject(); field.addProperty( "type", getFieldType( bridgeDefinedField ) ); - field.addProperty( "index", ANALYZED ); + + String index = getIndex( bridgeDefinedField ); + field.addProperty( "index", index ); // we don't overwrite already defined fields. Typically, in the case of spatial, the geo_point field // is defined before the double field and we want to keep the geo_point one @@ -443,7 +451,24 @@ private String getIndex(EntityIndexBinding binding, DocumentFieldMetadata fieldM return "not_analyzed"; } - switch ( fieldMetadata.getIndex() ) { + return getIndexValue( fieldMetadata.getIndex() ); + } + + @SuppressWarnings("deprecation") + private String getIndex(BridgeDefinedField bridgeDefinedField) { + // Never analyze boolean + if ( FieldHelper.isBoolean( bridgeDefinedField ) + || FieldHelper.isNumeric( bridgeDefinedField ) + || FieldHelper.isDate( bridgeDefinedField ) ) { + return "not_analyzed"; + } + + Field.Index index = bridgeDefinedField.getIndex(); + return getIndexValue( index ); + } + + private String getIndexValue(Field.Index index) { + switch ( index ) { case ANALYZED: case ANALYZED_NO_NORMS: return ANALYZED; @@ -453,7 +478,7 @@ private String getIndex(EntityIndexBinding binding, DocumentFieldMetadata fieldM case NO: return "no"; default: - throw new IllegalArgumentException( "Unexpected index type: " + fieldMetadata.getIndex() ); + throw new IllegalArgumentException( "Unexpected index type: " + index ); } } diff --git a/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/impl/FieldHelper.java b/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/impl/FieldHelper.java index d0719b9a4ee..ff58d1eb687 100644 --- a/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/impl/FieldHelper.java +++ b/elasticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/impl/FieldHelper.java @@ -21,6 +21,7 @@ import org.hibernate.search.engine.metadata.impl.PropertyMetadata; import org.hibernate.search.engine.metadata.impl.TypeMetadata; import org.hibernate.search.engine.spi.EntityIndexBinding; +import org.hibernate.search.exception.AssertionFailure; import org.hibernate.search.metadata.NumericFieldSettingsDescriptor.NumericEncodingType; /** @@ -79,6 +80,10 @@ static boolean isBoolean(EntityIndexBinding indexBinding, String fieldName) { return boolean.class.equals( propertyClass ) || Boolean.class.isAssignableFrom( propertyClass ); } + static boolean isBoolean(BridgeDefinedField field) { + return FieldType.BOOLEAN == field.getType(); + } + static boolean isDate(EntityIndexBinding indexBinding, String fieldName) { Class propertyClass = getPropertyClass( indexBinding, fieldName ); if ( propertyClass == null ) { @@ -95,6 +100,10 @@ static boolean isCalendar(EntityIndexBinding indexBinding, String fieldName) { return Calendar.class.isAssignableFrom( propertyClass ); } + static boolean isDate(BridgeDefinedField field) { + return FieldType.DATE == field.getType(); + } + static boolean isNumeric(DocumentFieldMetadata field) { if ( field.isNumeric() ) { return true; @@ -109,6 +118,22 @@ static boolean isNumeric(DocumentFieldMetadata field) { return false; } + static boolean isNumeric(BridgeDefinedField field) { + switch ( field.getType() ) { + case LONG: + case INTEGER: + case FLOAT: + case DOUBLE: + return true; + case STRING: + case BOOLEAN: + case DATE: + return false; + default: + throw new AssertionFailure( "Type not recognized: " + field.getType() ); + } + } + static String[] getFieldNameParts(String fieldName) { boolean isEmbeddedField = isEmbeddedField( fieldName ); return isEmbeddedField ? DOT.split( fieldName ) : new String[]{ fieldName }; diff --git a/engine/src/main/java/org/hibernate/search/engine/metadata/impl/AnnotationMetadataProvider.java b/engine/src/main/java/org/hibernate/search/engine/metadata/impl/AnnotationMetadataProvider.java index 817276c1bdf..c5dad4d2aac 100644 --- a/engine/src/main/java/org/hibernate/search/engine/metadata/impl/AnnotationMetadataProvider.java +++ b/engine/src/main/java/org/hibernate/search/engine/metadata/impl/AnnotationMetadataProvider.java @@ -368,7 +368,7 @@ private void createIdPropertyMetadata(XProperty member, checkForSortableFields( member, typeMetadataBuilder, propertyMetadataBuilder, "", true, null, parseContext ); if ( idBridge instanceof MetadataProvidingFieldBridge ) { - FieldMetadataBuilderImpl bridgeDefinedMetadata = getBridgeContributedFieldMetadata( path, (MetadataProvidingFieldBridge) idBridge ); + FieldMetadataBuilderImpl bridgeDefinedMetadata = getBridgeContributedFieldMetadata( fieldMetadata, (MetadataProvidingFieldBridge) idBridge ); for ( BridgeDefinedField bridgeDefinedField : bridgeDefinedMetadata.getFields() ) { propertyMetadataBuilder.addBridgeDefinedField( bridgeDefinedField ); @@ -699,7 +699,7 @@ private void bindClassBridgeAnnotation(String prefix, typeMetadataBuilder.addClassBridgeField( fieldMetadata ); - contributeClassBridgeDefinedFields( typeMetadataBuilder, fieldName, fieldBridge ); + contributeClassBridgeDefinedFields( typeMetadataBuilder, fieldMetadata, fieldBridge ); if ( !parseContext.skipAnalyzers() ) { AnalyzerReference analyzer = AnnotationProcessingHelper.getAnalyzerReference( @@ -745,7 +745,7 @@ private void bindSpatialAnnotation(Spatial spatialAnnotation, if ( fieldBridge instanceof MetadataProvidingFieldBridge ) { MetadataProvidingFieldBridge metadataProvidingFieldBridge = (MetadataProvidingFieldBridge) fieldBridge; - FieldMetadataBuilderImpl bridgeContributedMetadata = getBridgeContributedFieldMetadata( fieldName, metadataProvidingFieldBridge ); + FieldMetadataBuilderImpl bridgeContributedMetadata = getBridgeContributedFieldMetadata( fieldMetadata, metadataProvidingFieldBridge ); for ( BridgeDefinedField field : bridgeContributedMetadata.getFields() ) { propertyMetadataBuilder.addBridgeDefinedField( field ); } @@ -786,7 +786,7 @@ private void bindSpatialAnnotation(Spatial spatialAnnotation, typeMetadataBuilder.addClassBridgeField( fieldMetadata ); - contributeClassBridgeDefinedFields( typeMetadataBuilder, fieldName, spatialBridge ); + contributeClassBridgeDefinedFields( typeMetadataBuilder, fieldMetadata, spatialBridge ); AnalyzerReference analyzerReference = typeMetadataBuilder.getAnalyzerReference(); if ( analyzerReference == null ) { @@ -794,11 +794,11 @@ private void bindSpatialAnnotation(Spatial spatialAnnotation, } } - private void contributeClassBridgeDefinedFields(TypeMetadata.Builder typeMetadataBuilder, String fieldName, FieldBridge fieldBridge) { + private void contributeClassBridgeDefinedFields(TypeMetadata.Builder typeMetadataBuilder, DocumentFieldMetadata fieldMetadata, FieldBridge fieldBridge) { if ( fieldBridge instanceof MetadataProvidingFieldBridge ) { MetadataProvidingFieldBridge metadataProvidingFieldBridge = (MetadataProvidingFieldBridge) fieldBridge; - FieldMetadataBuilderImpl classBridgeContributedFieldMetadata = getBridgeContributedFieldMetadata( fieldName, metadataProvidingFieldBridge ); + FieldMetadataBuilderImpl classBridgeContributedFieldMetadata = getBridgeContributedFieldMetadata( fieldMetadata, metadataProvidingFieldBridge ); typeMetadataBuilder.addClassBridgeSortableFields( classBridgeContributedFieldMetadata.getSortableFields() ); typeMetadataBuilder.addClassBridgeDefinedFields( classBridgeContributedFieldMetadata.getFields() ); @@ -1207,22 +1207,6 @@ private void bindFieldAnnotation( ); } - if ( fieldBridge instanceof MetadataProvidingFieldBridge ) { - MetadataProvidingFieldBridge metadataProvidingFieldBridge = (MetadataProvidingFieldBridge) fieldBridge; - FieldMetadataBuilderImpl bridgeContributedMetadata = getBridgeContributedFieldMetadata( fieldName, metadataProvidingFieldBridge ); - - for ( String sortableField : bridgeContributedMetadata.getSortableFields() ) { - SortableFieldMetadata sortableFieldMetadata = new SortableFieldMetadata.Builder() - .fieldName( sortableField ) - .build(); - propertyMetadataBuilder.addSortableField( sortableFieldMetadata ); - } - - for ( BridgeDefinedField field : bridgeContributedMetadata.getFields() ) { - propertyMetadataBuilder.addBridgeDefinedField( field ); - } - } - final NumericEncodingType numericEncodingType = determineNumericFieldEncoding( fieldBridge ); final NullMarkerCodec nullTokenCodec = determineNullMarkerCodec( fieldAnnotation, configContext, numericEncodingType, fieldName ); if ( nullTokenCodec != NotEncodingCodec.SINGLETON && fieldBridge instanceof TwoWayFieldBridge ) { @@ -1290,6 +1274,22 @@ private void bindFieldAnnotation( DocumentFieldMetadata fieldMetadata = fieldMetadataBuilder.build(); propertyMetadataBuilder.addDocumentField( fieldMetadata ); + if ( fieldBridge instanceof MetadataProvidingFieldBridge ) { + MetadataProvidingFieldBridge metadataProvidingFieldBridge = (MetadataProvidingFieldBridge) fieldBridge; + FieldMetadataBuilderImpl bridgeContributedMetadata = getBridgeContributedFieldMetadata( fieldMetadata, metadataProvidingFieldBridge ); + + for ( String sortableField : bridgeContributedMetadata.getSortableFields() ) { + SortableFieldMetadata sortableFieldMetadata = new SortableFieldMetadata.Builder() + .fieldName( sortableField ) + .build(); + propertyMetadataBuilder.addSortableField( sortableFieldMetadata ); + } + + for ( BridgeDefinedField field : bridgeContributedMetadata.getFields() ) { + propertyMetadataBuilder.addBridgeDefinedField( field ); + } + } + // keep track of collection role names for ORM integration optimization based on collection update events parseContext.collectUnqualifiedCollectionRole( member.getName() ); } @@ -1969,9 +1969,9 @@ boolean stateInspectionOptimizationsEnabled(TypeMetadata.Builder typeMetadataBui return true; } - private FieldMetadataBuilderImpl getBridgeContributedFieldMetadata(String fieldName, MetadataProvidingFieldBridge metadataProvidingFieldBridge) { - FieldMetadataBuilderImpl builder = new FieldMetadataBuilderImpl(); - metadataProvidingFieldBridge.configureFieldMetadata( fieldName, builder ); + private FieldMetadataBuilderImpl getBridgeContributedFieldMetadata(DocumentFieldMetadata fieldMetadata, MetadataProvidingFieldBridge metadataProvidingFieldBridge) { + FieldMetadataBuilderImpl builder = new FieldMetadataBuilderImpl( fieldMetadata ); + metadataProvidingFieldBridge.configureFieldMetadata( fieldMetadata.getFieldName(), builder ); return builder; } diff --git a/engine/src/main/java/org/hibernate/search/engine/metadata/impl/BridgeDefinedField.java b/engine/src/main/java/org/hibernate/search/engine/metadata/impl/BridgeDefinedField.java index 78c1266b87b..05c61a643f1 100644 --- a/engine/src/main/java/org/hibernate/search/engine/metadata/impl/BridgeDefinedField.java +++ b/engine/src/main/java/org/hibernate/search/engine/metadata/impl/BridgeDefinedField.java @@ -6,6 +6,8 @@ */ package org.hibernate.search.engine.metadata.impl; +import org.apache.lucene.document.Field; +import org.apache.lucene.document.Field.Index; import org.hibernate.search.bridge.spi.FieldType; /** @@ -15,12 +17,18 @@ */ public class BridgeDefinedField { - private String name; - private FieldType type; + private final String name; + private final FieldType type; + private final Index index; - public BridgeDefinedField(String name, FieldType type) { + public BridgeDefinedField(String name, FieldType type, Index index) { this.name = name; this.type = type; + this.index = index; + } + + public Field.Index getIndex() { + return index; } public String getName() { diff --git a/engine/src/main/java/org/hibernate/search/engine/metadata/impl/FieldMetadataBuilderImpl.java b/engine/src/main/java/org/hibernate/search/engine/metadata/impl/FieldMetadataBuilderImpl.java index cd184413f79..e9883c62846 100644 --- a/engine/src/main/java/org/hibernate/search/engine/metadata/impl/FieldMetadataBuilderImpl.java +++ b/engine/src/main/java/org/hibernate/search/engine/metadata/impl/FieldMetadataBuilderImpl.java @@ -22,6 +22,11 @@ class FieldMetadataBuilderImpl implements FieldMetadataBuilder { private final Set sortableFields = new HashSet<>(); private final Set fields = new HashSet<>(); + private final DocumentFieldMetadata fieldMetadata; + + public FieldMetadataBuilderImpl(DocumentFieldMetadata fieldMetadata) { + this.fieldMetadata = fieldMetadata; + } @Override public FieldMetadataCreationContext field(String name, FieldType type) { @@ -43,7 +48,7 @@ private class FieldMetadataCreationContextImpl implements FieldMetadataCreationC public FieldMetadataCreationContextImpl(String name, FieldType type) { this.fieldName = name; - fields.add( new BridgeDefinedField( name, type ) ); + fields.add( new BridgeDefinedField( name, type, fieldMetadata.getIndex() ) ); } @Override