Merge pull request #1026 from Alfresco/fix/SEARCH-2548_cherrypick_2.0.X

Fix/search 2538 (#987)
2026-09-16 18:12:56 +00:00 · 2020-11-26 17:38:09 +00:00
parent 72218571be c95bfcf8df
commit 7b1508e4c9
7 changed files with 387 additions and 4 deletions
@@ -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<String, Object> 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<String, String> foundProperties = (Map<String, String>) 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<Object> aspects = doc.getProperty(PropertyIds.SECONDARY_OBJECT_TYPE_IDS).getValues();
+        aspects.remove("P:finance:ParkEx");
+        Map<String, Object> 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<String, String>) 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");
+
+    }
 }
@@ -284,9 +284,30 @@ public class SolrInformationServer implements InformationServer
     *         unless you call it with a list, any subsequent call will replace the existing value.
     *     </li>
     * </ul>
+     *
+     * 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<String, String> 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<String, List<Object>> fieldModifier =
                    (Map<String, List<Object>>)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<SolrInputDocument> initialEmptyDocumentSupplier)
+    private <T extends SolrInputDocument> T basicDocument(NodeMetaData metadata, String docType, Supplier<T> initialEmptyDocumentSupplier)
    {
-        SolrInputDocument doc = initialEmptyDocumentSupplier.get();
+        T doc = initialEmptyDocumentSupplier.get();
        doc.setField(FIELD_SOLR4_ID,
                AlfrescoSolrDataModel.getNodeDocumentId(
                        metadata.getTenantDomain(),
@@ -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 <http://www.gnu.org/licenses/>.
+ * #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);
+    }
+}
@@ -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 <http://www.gnu.org/licenses/>.
+ * #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<String, Object>)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);
+    }
+}
@@ -1635,6 +1635,7 @@

    <updateRequestProcessorChain name="uniq-fields" default="true" post-processor="field-values-deduplication">
        <processor class="solr.LogUpdateProcessorFactory" />
+        <processor class="org.alfresco.solr.update.processor.AlfrescoDistributedUpdateProcessorFactory" />
        <processor class="solr.RunUpdateProcessorFactory" />
    </updateRequestProcessorChain>

@@ -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 <http://www.gnu.org/licenses/>.
+ * #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);
+    }
+}
@@ -484,6 +484,7 @@

    <updateRequestProcessorChain name="uniq-fields" default="true" post-processor="field-values-deduplication">
        <processor class="solr.LogUpdateProcessorFactory" />
+        <processor class="org.alfresco.solr.update.processor.AlfrescoDistributedUpdateProcessorFactory" />
        <processor class="solr.RunUpdateProcessorFactory" />
    </updateRequestProcessorChain>