From c95bfcf8df5db5b2dcb99f783dc9d2bdd425c5da Mon Sep 17 00:00:00 2001 From: Elia Porciani Date: Fri, 30 Oct 2020 09:22:42 +0100 Subject: [PATCH] Fix/search 2538 (#987) * [SEARCH-2538] Implementation of explicit atomic updates for fixing aspect removal issue (cherry picked from commit e13bc4c2f7b05febabb27372e9bb96dc28e99b60) --- .../search/SearchAspectTest.java | 85 +++++++++++++ .../alfresco/solr/SolrInformationServer.java | 57 ++++++++- ...escoDistributedUpdateProcessorFactory.java | 47 ++++++++ .../AlfrescoExplicitDocumentMerger.java | 86 +++++++++++++ .../templates/rerank/conf/solrconfig.xml | 1 + ...RemovePropertiesAlfrescoSolrTrackerIT.java | 114 ++++++++++++++++++ .../collection1/conf/solrconfig.xml | 1 + 7 files changed, 387 insertions(+), 4 deletions(-) create mode 100644 search-services/alfresco-search/src/main/java/org/alfresco/solr/update/processor/AlfrescoDistributedUpdateProcessorFactory.java create mode 100644 search-services/alfresco-search/src/main/java/org/alfresco/solr/update/processor/AlfrescoExplicitDocumentMerger.java create mode 100644 search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedRemovePropertiesAlfrescoSolrTrackerIT.java diff --git a/e2e-test/src/test/java/org/alfresco/test/search/functional/searchServices/search/SearchAspectTest.java b/e2e-test/src/test/java/org/alfresco/test/search/functional/searchServices/search/SearchAspectTest.java index 13b0d5548..d1cb39268 100644 --- a/e2e-test/src/test/java/org/alfresco/test/search/functional/searchServices/search/SearchAspectTest.java +++ b/e2e-test/src/test/java/org/alfresco/test/search/functional/searchServices/search/SearchAspectTest.java @@ -23,9 +23,13 @@ package org.alfresco.test.search.functional.searchServices.search; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNull; import static org.testng.Assert.assertTrue; +import java.util.HashMap; import java.util.List; +import java.util.Map; import org.alfresco.rest.search.RestRequestQueryModel; import org.alfresco.rest.search.SearchRequest; @@ -33,11 +37,17 @@ import org.alfresco.rest.search.SearchResponse; import org.alfresco.test.search.functional.AbstractE2EFunctionalTest; import org.alfresco.utility.data.CustomObjectTypeProperties; import org.alfresco.utility.model.FileModel; +import org.alfresco.utility.model.FileType; import org.alfresco.utility.model.FolderModel; +import org.apache.chemistry.opencmis.client.api.Document; import org.apache.chemistry.opencmis.commons.PropertyIds; +import org.apache.chemistry.opencmis.commons.enums.VersioningState; +import org.testng.Assert; import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; +import javax.validation.constraints.AssertTrue; + /** * Test class tests aspects are added and removed from Solr Documents * Created for Search-2379 @@ -62,6 +72,9 @@ public class SearchAspectTest extends AbstractE2EFunctionalTest new CustomObjectTypeProperties()); waitForMetadataIndexing(file.getName(), true); + + assertTrue(deployCustomModel("model/finance-model.xml"), + "failing while deploying model"); } @Test(priority = 1) @@ -99,4 +112,76 @@ public class SearchAspectTest extends AbstractE2EFunctionalTest "checkedOut aspect was NOT expected"); } + + /** + * Check that when an aspect is removed, all the properties defined in the aspect are removed as well. + * Created for Search-2538 + */ + @Test(priority = 2) + public void testAspectIsRemovedWithItsProperties() + { + String parkingLocationFieldName = "finance:ParkingLocation"; + String financeLocationFieldName = "finance:Location"; + + FileModel expenseLondon = FileModel.getRandomFileModel(FileType.TEXT_PLAIN, "Expense"); + + Map properties = new HashMap<>(); + properties.put(PropertyIds.OBJECT_TYPE_ID, "D:finance:Expense"); + properties.put(PropertyIds.NAME, expenseLondon.getName()); + properties.put(PropertyIds.SECONDARY_OBJECT_TYPE_IDS, List.of("P:finance:ParkEx")); + properties.put(financeLocationFieldName, "LondonBridge"); + properties.put(parkingLocationFieldName, "LiverpoolStreet"); + + // Compose query + String queryFile = "cm:name:'" + expenseLondon.getName() + "'"; + RestRequestQueryModel queryModel = new RestRequestQueryModel(); + queryModel.setQuery(queryFile); + queryModel.setLanguage(SearchLanguage.AFTS.toString()); + SearchRequest searchRequest = new SearchRequest(); + searchRequest.setQuery(queryModel); + searchRequest.setInclude(List.of("aspectNames","properties")); + + cmisApi.authenticateUser(testUser) + .usingSite(testSite) + .usingResource(folder) + .createFile(expenseLondon, properties, VersioningState.MAJOR) + .assertThat().existsInRepo(); + + String parkingLocationQuery = parkingLocationFieldName + ":LiverpoolStreet"; + Assert.assertTrue(waitForIndexing(parkingLocationQuery, true)); + + // check that the document found has the expected properties and aspects defined. + SearchResponse response = restClient.authenticateUser(testUser).withSearchAPI().search(searchRequest); + assertTrue(response.getEntries().get(0).getModel().getAspectNames().contains("finance:ParkEx"), + "parkEx aspect was expected"); + Map foundProperties = (Map) response.getEntries().get(0).getModel().getProperties(); + assertEquals("LondonBridge", foundProperties.get(financeLocationFieldName), + "finance:Location property is expected to be defined with 'LondonBridge' as value"); + assertEquals("LiverpoolStreet", foundProperties.get(parkingLocationFieldName), + "finance:ParkingLocation property is expected to be defined with 'LiverpoolStreet' as value"); + + // remove aspect + Document doc = cmisApi.withCMISUtil().getCmisDocument(cmisApi.getLastResource()); + List aspects = doc.getProperty(PropertyIds.SECONDARY_OBJECT_TYPE_IDS).getValues(); + aspects.remove("P:finance:ParkEx"); + Map updateProperties = new HashMap<>(); + updateProperties.put(PropertyIds.SECONDARY_OBJECT_TYPE_IDS, aspects); + + doc.updateProperties(updateProperties); + + // Check that the field related to the removed aspect is not longer indexed for tested file + Assert.assertTrue(waitForIndexing(parkingLocationQuery, false)); + + response = restClient.authenticateUser(testUser).withSearchAPI().search(searchRequest); + assertTrue(!response.getEntries().get(0).getModel().getAspectNames().contains("finance:ParkEx"), + "parkEx aspect was NOT expected"); + + // check that the document found has finance:Location property defined and finance:ParkingLocation has been removed + foundProperties = (Map) response.getEntries().get(0).getModel().getProperties(); + assertEquals("LondonBridge", foundProperties.get(financeLocationFieldName), + "finance:Location property is expected to be defined with 'LondonBridge' as value"); + assertNull(foundProperties.get(parkingLocationFieldName), "finance:ParkingLocation should not be included " + + "into the document anymore"); + + } } diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/SolrInformationServer.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/SolrInformationServer.java index 997acb053..a75e6da86 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/SolrInformationServer.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/SolrInformationServer.java @@ -284,9 +284,30 @@ public class SolrInformationServer implements InformationServer * unless you call it with a list, any subsequent call will replace the existing value. * * + * + * It has been introduced a new method "keepField". If this method is called, an explicit atomic update is executed + * instead of a standard atomic updated. + * + * An explicit atomic update is an atomic update with the difference that we must specify all the fields that we want + * to keep from the document already indexed in solr. + * */ static class PartialSolrInputDocument extends SolrInputDocument { + private static final Map KEEP_MAP = Map.of("keep", ""); + + /** + * Keep the field from the indexed solr document. + * + * Calling this method at least once provokes the execution of an explicit atomic update. + * With explicit atomic update, all the fields defined in the indexed solr document + * that are not explicitly inserted into the inputDocument will be discarded. + */ + public void keepField(String name) + { + setField(name, KEEP_MAP); + } + @Override @SuppressWarnings("unchecked") public void addField(String name, Object value) @@ -294,7 +315,7 @@ public class SolrInformationServer implements InformationServer Map> fieldModifier = (Map>)computeIfAbsent(name, k -> { remove(name); - setField(name, newFieldModifier("add")); + setField(name, newFieldModifier("set")); return getField(name); }).getValue(); @@ -2118,7 +2139,7 @@ public class SolrInformationServer implements InformationServer } } - private SolrInputDocument populateWithMetadata(SolrInputDocument document, NodeMetaData metadata, NodeMetaDataParameters nmdp) + private PartialSolrInputDocument populateWithMetadata(PartialSolrInputDocument document, NodeMetaData metadata, NodeMetaDataParameters nmdp) { populateFields(metadata, document, nmdp); @@ -2130,6 +2151,8 @@ public class SolrInformationServer implements InformationServer document, contentIndexingHasBeenEnabledOnThisInstance); + keepContentFields(document); + LOGGER.debug("Document size (fields) after getting properties from node {} metadata: {}", metadata.getId(), document.size()); return document; @@ -2677,6 +2700,8 @@ public class SolrInformationServer implements InformationServer addContentPropertyToDocUsingAlfrescoRepository(doc, propertyQName, dbId, locale); } + + /** * Extracts the text content from the given API response. * @@ -2749,6 +2774,30 @@ public class SolrInformationServer implements InformationServer } } + private void keepContentFields(PartialSolrInputDocument doc) + { + String qNamePart = CONTENT_LOCALE_FIELD.substring(AlfrescoSolrDataModel.CONTENT_S_LOCALE_PREFIX.length()); + QName propertyQName = QName.createQName(qNamePart); + + dataModel.getIndexedFieldForSpecializedPropertyMetadata(propertyQName, AlfrescoSolrDataModel.SpecializedFieldType.TRANSFORMATION_STATUS) + .getFields() + .stream() + .forEach(field -> doc.keepField(field.getField())); + + dataModel.getIndexedFieldForSpecializedPropertyMetadata(propertyQName, AlfrescoSolrDataModel.SpecializedFieldType.TRANSFORMATION_EXCEPTION) + .getFields() + .stream() + .forEach(field -> doc.keepField(field.getField())); + + dataModel.getIndexedFieldForSpecializedPropertyMetadata(propertyQName, AlfrescoSolrDataModel.SpecializedFieldType.TRANSFORMATION_TIME) + .getFields() + .stream() + .forEach(field -> doc.keepField(field.getField())); + + doc.keepField(FINGERPRINT_FIELD); + doc.keepField(dataModel.getStoredContentField(propertyQName)); + } + private String languageFrom(String locale) { int indexOfSeparator = locale.indexOf("_"); @@ -3365,9 +3414,9 @@ public class SolrInformationServer implements InformationServer * @param initialEmptyDocumentSupplier a factory for creating the initial {@link SolrInputDocument} instance. * @return a basic {@link SolrInputDocument} instance populated with the minimal set of information. */ - private SolrInputDocument basicDocument(NodeMetaData metadata, String docType, Supplier initialEmptyDocumentSupplier) + private T basicDocument(NodeMetaData metadata, String docType, Supplier initialEmptyDocumentSupplier) { - SolrInputDocument doc = initialEmptyDocumentSupplier.get(); + T doc = initialEmptyDocumentSupplier.get(); doc.setField(FIELD_SOLR4_ID, AlfrescoSolrDataModel.getNodeDocumentId( metadata.getTenantDomain(), diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/update/processor/AlfrescoDistributedUpdateProcessorFactory.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/update/processor/AlfrescoDistributedUpdateProcessorFactory.java new file mode 100644 index 000000000..7767df6f3 --- /dev/null +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/update/processor/AlfrescoDistributedUpdateProcessorFactory.java @@ -0,0 +1,47 @@ +/* + * #%L + * Alfresco Search Services + * %% + * Copyright (C) 2005 - 2020 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + * #L% + */ +package org.alfresco.solr.update.processor; + +import org.apache.solr.request.SolrQueryRequest; +import org.apache.solr.response.SolrQueryResponse; +import org.apache.solr.update.processor.DistributedUpdateProcessor; +import org.apache.solr.update.processor.DistributedUpdateProcessorFactory; +import org.apache.solr.update.processor.UpdateRequestProcessor; + +/** + * @Author elia + * + * Produce a DistributedUpdateProcessor using the AlfrescoExplicitDocumentMerger + */ +public class AlfrescoDistributedUpdateProcessorFactory extends DistributedUpdateProcessorFactory +{ + @Override + public DistributedUpdateProcessor getInstance(SolrQueryRequest req, + SolrQueryResponse rsp, UpdateRequestProcessor next) + { + return new DistributedUpdateProcessor(req, rsp, new AlfrescoExplicitDocumentMerger(req), next); + } +} diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/update/processor/AlfrescoExplicitDocumentMerger.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/update/processor/AlfrescoExplicitDocumentMerger.java new file mode 100644 index 000000000..bffb57bf6 --- /dev/null +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/update/processor/AlfrescoExplicitDocumentMerger.java @@ -0,0 +1,86 @@ +/* + * #%L + * Alfresco Search Services + * %% + * Copyright (C) 2005 - 2020 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + * #L% + */ +package org.alfresco.solr.update.processor; + +import org.apache.solr.common.SolrInputDocument; +import org.apache.solr.common.SolrInputField; +import org.apache.solr.request.SolrQueryRequest; +import org.apache.solr.update.processor.AtomicUpdateDocumentMerger; + +import java.util.Map; + +/** + * @Author elia + * + * Provide the possibility to perform a an explicit atomic update: + * We need to specify all the fields that we want to use from the old documents. The other fields will be discarded. + * + * In order to distinguish between standard atomic updates and explicit atomic updates we search for a keep entry in + * the input document. If the input document contains an entry map with "keep" as a key, then the update is managed + * as an explicit atomic update. + */ +public class AlfrescoExplicitDocumentMerger extends AtomicUpdateDocumentMerger { + public AlfrescoExplicitDocumentMerger(SolrQueryRequest queryReq) + { + super(queryReq); + } + + @Override + public SolrInputDocument merge(final SolrInputDocument fromDoc, SolrInputDocument toDoc) + { + if (isExplicitAtomicUpdate(fromDoc)) + { + processInputDocuments(fromDoc, toDoc); + } + return super.merge(fromDoc, toDoc); + } + + /** + * Process the input documents: + * all fields that are not contained in fromDocu are removed from toDoc. + * all fields with keep key are removed from fromDoc in order to avoid warnings. + */ + private void processInputDocuments(final SolrInputDocument fromDoc, SolrInputDocument toDoc) + { + toDoc.entrySet().removeIf(entry -> fromDoc.getField(entry.getKey()) == null); + fromDoc.values().removeIf(sif -> + sif.getValue() instanceof Map && + ((Map)sif.getValue()).get("keep") != null + ); + } + + /** + * Any value including a value with Map having "keep" key is marking an explicitAtomicUpdate + */ + private boolean isExplicitAtomicUpdate(final SolrInputDocument fromDoc) + { + return fromDoc.values() + .stream() + .map(SolrInputField::getValue) + .filter(value -> value instanceof Map) + .anyMatch(value -> ((Map) value).get("keep") != null); + } +} diff --git a/search-services/alfresco-search/src/main/resources/solr/instance/templates/rerank/conf/solrconfig.xml b/search-services/alfresco-search/src/main/resources/solr/instance/templates/rerank/conf/solrconfig.xml index c001c6f77..004f4840d 100644 --- a/search-services/alfresco-search/src/main/resources/solr/instance/templates/rerank/conf/solrconfig.xml +++ b/search-services/alfresco-search/src/main/resources/solr/instance/templates/rerank/conf/solrconfig.xml @@ -1635,6 +1635,7 @@ + diff --git a/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedRemovePropertiesAlfrescoSolrTrackerIT.java b/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedRemovePropertiesAlfrescoSolrTrackerIT.java new file mode 100644 index 000000000..2aebe0b26 --- /dev/null +++ b/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedRemovePropertiesAlfrescoSolrTrackerIT.java @@ -0,0 +1,114 @@ +/* + * #%L + * Alfresco Search Services + * %% + * Copyright (C) 2005 - 2020 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + * #L% + */ +package org.alfresco.solr.tracker; + +import org.alfresco.model.ContentModel; +import org.alfresco.solr.AbstractAlfrescoDistributedIT; +import org.alfresco.solr.client.Acl; +import org.alfresco.solr.client.AclChangeSet; +import org.alfresco.solr.client.AclReaders; +import org.alfresco.solr.client.ContentPropertyValue; +import org.alfresco.solr.client.Node; +import org.alfresco.solr.client.NodeMetaData; +import org.alfresco.solr.client.StringPropertyValue; +import org.alfresco.solr.client.Transaction; +import org.apache.lucene.index.Term; +import org.apache.lucene.search.TermQuery; +import org.apache.solr.SolrTestCaseJ4; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +import java.util.Locale; + +import static org.alfresco.solr.AlfrescoSolrUtils.MAX_WAIT_TIME; +import static org.alfresco.solr.AlfrescoSolrUtils.getAcl; +import static org.alfresco.solr.AlfrescoSolrUtils.getAclChangeSet; +import static org.alfresco.solr.AlfrescoSolrUtils.getAclReaders; +import static org.alfresco.solr.AlfrescoSolrUtils.getNode; +import static org.alfresco.solr.AlfrescoSolrUtils.getNodeMetaData; +import static org.alfresco.solr.AlfrescoSolrUtils.getTransaction; +import static org.alfresco.solr.AlfrescoSolrUtils.indexAclChangeSet; +import static org.alfresco.solr.AlfrescoSolrUtils.list; + +/** + * @author Elia + * + * Test added for SEARCH-2538 + * This test checks that when a property is removed from a document it is also removed from index. + */ +@SolrTestCaseJ4.SuppressSSL +public class DistributedRemovePropertiesAlfrescoSolrTrackerIT extends AbstractAlfrescoDistributedIT { + final private String authorField = "text@s__lt@{http://www.alfresco.org/model/content/1.0}author"; + + @BeforeClass + public static void initData() throws Throwable { + initSolrServers(1, ContentPropertyValueTrackerIT.class.getSimpleName(), null); + } + + @AfterClass + public static void destroyData() { + dismissSolrServers(); + } + + @Test + public void propertyRemovedIsNoLongerInIndexTest() throws Exception { + putHandleDefaults(); + AclChangeSet aclChangeSet = getAclChangeSet(1, 1); + Acl acl = getAcl(aclChangeSet); + + // Arbitrary acl data. + AclReaders aclReaders = getAclReaders(aclChangeSet, acl, list("joel"), list("phil"), null); + indexAclChangeSet(aclChangeSet, + list(acl), + list(aclReaders)); + + Transaction txn = getTransaction(0, 1); + Node fileNode = getNode(txn, acl, Node.SolrApiNodeStatus.UPDATED); + NodeMetaData fileMetaData = getNodeMetaData(fileNode, txn, acl, "mike", null, false); + String author = "Mario"; + + fileMetaData.getProperties() + .put(ContentModel.PROP_TITLE, new ContentPropertyValue(Locale.CANADA, 100, "UTF8", "txt", 10l)); + fileMetaData.getProperties().put(ContentModel.PROP_AUTHOR, new StringPropertyValue(author)); + indexTransaction(txn, + list(fileNode), + list(fileMetaData)); + + // Check the document is correctly indexed + waitForDocCount(new TermQuery(new Term(authorField, author)), 1, MAX_WAIT_TIME); + + // remove the author + Transaction txn1 = getTransaction(0, 1); + fileMetaData.getProperties().remove(ContentModel.PROP_AUTHOR); + indexTransaction(txn1, + list(fileNode), + list(fileMetaData)); + + // Check that author is removed from index. + waitForDocCount(new TermQuery(new Term(authorField, author)), 0, MAX_WAIT_TIME); + } +} \ No newline at end of file diff --git a/search-services/alfresco-search/src/test/resources/test-files/collection1/conf/solrconfig.xml b/search-services/alfresco-search/src/test/resources/test-files/collection1/conf/solrconfig.xml index 1a935612f..745fb2610 100644 --- a/search-services/alfresco-search/src/test/resources/test-files/collection1/conf/solrconfig.xml +++ b/search-services/alfresco-search/src/test/resources/test-files/collection1/conf/solrconfig.xml @@ -484,6 +484,7 @@ +