From 601332cb6c4483038fda785706a998d13964e014 Mon Sep 17 00:00:00 2001 From: Adrian Nistor Date: Wed, 4 May 2016 14:06:49 +0300 Subject: [PATCH] ISPN-6589 ProtobufIndexedFieldProvider may mistakenly report nested fields as unindexed --- .../impl/ProtobufIndexedFieldProvider.java | 43 ++++------- .../ProtobufIndexedFieldProviderTest.java | 73 +++++++++++++++++++ 2 files changed, 89 insertions(+), 27 deletions(-) create mode 100644 remote-query/remote-query-server/src/test/java/org/infinispan/query/remote/impl/ProtobufIndexedFieldProviderTest.java diff --git a/remote-query/remote-query-server/src/main/java/org/infinispan/query/remote/impl/ProtobufIndexedFieldProvider.java b/remote-query/remote-query-server/src/main/java/org/infinispan/query/remote/impl/ProtobufIndexedFieldProvider.java index 101e4fd74f5f..6f8d0504d386 100644 --- a/remote-query/remote-query-server/src/main/java/org/infinispan/query/remote/impl/ProtobufIndexedFieldProvider.java +++ b/remote-query/remote-query-server/src/main/java/org/infinispan/query/remote/impl/ProtobufIndexedFieldProvider.java @@ -7,6 +7,7 @@ import org.infinispan.query.remote.impl.indexing.IndexingMetadata; import java.util.List; +import java.util.function.BiFunction; /** * Tests if a field is indexed by examining the Protobuf metadata. @@ -18,36 +19,24 @@ final class ProtobufIndexedFieldProvider implements BooleShannonExpansion.Indexe private final Descriptor messageDescriptor; - public ProtobufIndexedFieldProvider(Descriptor messageDescriptor) { + ProtobufIndexedFieldProvider(Descriptor messageDescriptor) { + if (messageDescriptor == null) { + throw new IllegalArgumentException("argument cannot be null"); + } this.messageDescriptor = messageDescriptor; } @Override public boolean isIndexed(List propertyPath) { - Descriptor md = messageDescriptor; - int i = 0; - for (String p : propertyPath) { - i++; - FieldDescriptor field = md.findFieldByName(p); - if (field == null) { - break; - } - if (field.getJavaType() == JavaType.MESSAGE) { - md = field.getMessageType(); - } else { - if (i == propertyPath.size()) { - IndexingMetadata indexingMetadata = messageDescriptor.getProcessedAnnotation(IndexingMetadata.INDEXED_ANNOTATION); - return indexingMetadata == null || indexingMetadata.isFieldIndexed(field.getNumber()); - } else { - break; - } - } - } - return false; + return getMetadata(propertyPath, IndexingMetadata::isFieldIndexed); } @Override public boolean isStored(List propertyPath) { + return getMetadata(propertyPath, IndexingMetadata::isFieldStored); + } + + private boolean getMetadata(List propertyPath, BiFunction metadataFun) { Descriptor md = messageDescriptor; int i = 0; for (String p : propertyPath) { @@ -56,15 +45,15 @@ public boolean isStored(List propertyPath) { if (field == null) { break; } + IndexingMetadata indexingMetadata = md.getProcessedAnnotation(IndexingMetadata.INDEXED_ANNOTATION); + boolean res = indexingMetadata == null || metadataFun.apply(indexingMetadata, field.getNumber()); + if (!res) { + break; + } if (field.getJavaType() == JavaType.MESSAGE) { md = field.getMessageType(); } else { - if (i == propertyPath.size()) { - IndexingMetadata indexingMetadata = messageDescriptor.getProcessedAnnotation(IndexingMetadata.INDEXED_ANNOTATION); - return indexingMetadata == null || indexingMetadata.isFieldStored(field.getNumber()); - } else { - break; - } + return i == propertyPath.size(); } } return false; diff --git a/remote-query/remote-query-server/src/test/java/org/infinispan/query/remote/impl/ProtobufIndexedFieldProviderTest.java b/remote-query/remote-query-server/src/test/java/org/infinispan/query/remote/impl/ProtobufIndexedFieldProviderTest.java new file mode 100644 index 000000000000..37bb2d6bd506 --- /dev/null +++ b/remote-query/remote-query-server/src/test/java/org/infinispan/query/remote/impl/ProtobufIndexedFieldProviderTest.java @@ -0,0 +1,73 @@ +package org.infinispan.query.remote.impl; + +import org.infinispan.configuration.cache.ConfigurationBuilder; +import org.infinispan.configuration.cache.Index; +import org.infinispan.manager.EmbeddedCacheManager; +import org.infinispan.protostream.FileDescriptorSource; +import org.infinispan.protostream.SerializationContext; +import org.infinispan.test.SingleCacheManagerTest; +import org.infinispan.test.fwk.TestCacheManagerFactory; +import org.infinispan.transaction.TransactionMode; +import org.testng.annotations.Test; + +import java.util.Arrays; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * Test that ProtobufIndexedFieldProvider correctly identifies indexed fields based on the protostream annotations + * applied in the test model (bank.proto). + * + * @author anistor@redhat.com + * @since 9.0 + */ +@Test(groups = "functional", testName = "query.remote.impl.ProtobufIndexedFieldProviderTest") +public class ProtobufIndexedFieldProviderTest extends SingleCacheManagerTest { + + private static final String PROTO_DEFINITIONS = + "/** @Indexed */ message User {\n" + + "\n" + + " /** @IndexedField */ required string name = 1;\n" + + "\n" + + " required string surname = 2;\n" + + "\n" + + " /** @Indexed(true) */" + + " message Address {\n" + + " /** @IndexedField */ required string street = 10;\n" + + " required string postCode = 20;\n" + + " }\n" + + "\n" + + " /** @IndexedField */ repeated Address indexedAddresses = 3; \n" + + "\n" + + " /** @IndexedField(index=false, store=false) */ repeated Address unindexedAddresses = 4;\n" + + "}\n"; + + protected EmbeddedCacheManager createCacheManager() throws Exception { + ConfigurationBuilder cfg = getDefaultStandaloneCacheConfig(true); + cfg.transaction().transactionMode(TransactionMode.TRANSACTIONAL) + .indexing().index(Index.ALL) + .addProperty("default.directory_provider", "ram") + .addProperty("lucene_version", "LUCENE_CURRENT"); + return TestCacheManagerFactory.createCacheManager(cfg); + } + + public void testProvider() throws Exception { + SerializationContext serCtx = ProtobufMetadataManagerImpl.getSerializationContext(cacheManager); + serCtx.registerProtoFiles(FileDescriptorSource.fromString("user_definition.proto", PROTO_DEFINITIONS)); + ProtobufIndexedFieldProvider userIndexedFieldProvider = new ProtobufIndexedFieldProvider(serCtx.getMessageDescriptor("User")); + ProtobufIndexedFieldProvider addressIndexedFieldProvider = new ProtobufIndexedFieldProvider(serCtx.getMessageDescriptor("User.Address")); + + // try top level attributes + assertTrue(userIndexedFieldProvider.isIndexed(Arrays.asList("name"))); + assertFalse(userIndexedFieldProvider.isIndexed(Arrays.asList("surname"))); + assertTrue(addressIndexedFieldProvider.isIndexed(Arrays.asList("street"))); + assertFalse(addressIndexedFieldProvider.isIndexed(Arrays.asList("postCode"))); + + // try nested attributes + assertTrue(userIndexedFieldProvider.isIndexed(Arrays.asList("indexedAddresses", "street"))); + assertFalse(userIndexedFieldProvider.isIndexed(Arrays.asList("indexedAddresses", "postCode"))); + assertFalse(userIndexedFieldProvider.isIndexed(Arrays.asList("unindexedAddresses", "street"))); + assertFalse(userIndexedFieldProvider.isIndexed(Arrays.asList("unindexedAddresses", "postCode"))); + } +}