#30285 Fixing several unique fields database validation bugs (#30872)

I am fixing a couple of bugs here: - The unique_fields table is not getting clean up after deleting a ContentType - When try to update a Publish Contentlet with a unique fields but the unique fields is not changed ### Proposed Changes * Create a method to listen when a ContenType is deleted to clean up the unique_fields table https://github.com/dotCMS/core/pull/30872/files#diff-217758731836e15813d5ad2e85a398e4d52317cbbb9334d5961895f9d7883851R113 * Used the ContentTypeDeletedEvent to include the whole ContentType and not just the variable name, we are going to need the field list to know if this ContentType has unique fields later https://github.com/dotCMS/core/pull/30872/files#diff-b3de838f3681e084b3121056ed3a3002a19761e286f17a26ba8f466842b345a9R7 * The Unique value can be used by a LIVE or WORKING version, If the value for the LIVE and WORKING version are different then you are going to have a register for each but if the value is the same then you are going to have just 1 register for both with the LIVE equals true, so we when we clean uo the table after a update we need to check if the register that already exists are LIVE or WORKING https://github.com/dotCMS/core/pull/30872/files#diff-217758731836e15813d5ad2e85a398e4d52317cbbb9334d5961895f9d7883851R113-R137 --------- Co-authored-by: fabrizzio-dotCMS <[email protected]>
dotCMS · Dec 7, 2024 · 6181061 · 6181061
1 parent 4aaeb73
commit 6181061
Show file tree

Hide file tree

Showing 9 changed files with 506 additions and 35 deletions.
diff --git a/dotCMS/src/main/java/com/dotcms/contenttype/business/ContentTypeAPIImpl.java b/dotCMS/src/main/java/com/dotcms/contenttype/business/ContentTypeAPIImpl.java
@@ -191,7 +191,7 @@ private void transactionalDelete(ContentType type) throws DotDataException {
         Logger.error(ContentType.class, e.getMessage(), e);
         throw new BaseRuntimeInternationalizationException(e);
       }
-      HibernateUtil.addCommitListener(() -> localSystemEventsAPI.notify(new ContentTypeDeletedEvent(type.variable())));
+      HibernateUtil.addCommitListener(() -> localSystemEventsAPI.notify(new ContentTypeDeletedEvent(type)));
   }
 
   /**
@@ -341,7 +341,7 @@ private void disposeSourceThenFireContentDelete( final ContentType source, final
 
       HibernateUtil.addCommitListener(() -> {
         //Notify the system events API that the content type has been deleted, so it can take care of the WF clean up
-        localSystemEventsAPI.notify(new ContentTypeDeletedEvent(source.variable()));
+        localSystemEventsAPI.notify(new ContentTypeDeletedEvent(source));
           //By default, the deletion process takes placed within job
           Logger.info(this, String.format(" Content type (%s) will be deleted asynchronously using Quartz Job.", source.name()));
           if(asyncDeleteWithJob) {

diff --git a/dotCMS/src/main/java/com/dotcms/contenttype/business/ContentTypeDestroyAPIImpl.java b/dotCMS/src/main/java/com/dotcms/contenttype/business/ContentTypeDestroyAPIImpl.java
@@ -328,7 +328,7 @@ void broadcastEvents(final ContentType type, final User user)  {
             throw new BaseRuntimeInternationalizationException(e);
         }
         final LocalSystemEventsAPI localSystemEventsAPI = APILocator.getLocalSystemEventsAPI();
-        localSystemEventsAPI.notify(new ContentTypeDeletedEvent(type.variable()));
+        localSystemEventsAPI.notify(new ContentTypeDeletedEvent(type));
         notifyContentTypeDestroyed(type);
     }
 

diff --git a/...main/java/com/dotcms/contenttype/business/uniquefields/UniqueFieldValidationStrategy.java b/...main/java/com/dotcms/contenttype/business/uniquefields/UniqueFieldValidationStrategy.java
@@ -162,4 +162,15 @@ default void cleanUp(final Field field) throws DotDataException {
         //Default implementation do nothing
     }
 
+    /**
+     * Method called after delete a {@link ContentType}, to allow the {@link UniqueFieldValidationStrategy} do any extra
+     * work that it need it.
+     *
+     * @param contentType deleted ContentType
+     * @throws DotDataException
+     */
+    default void cleanUp(final ContentType contentType) throws DotDataException {
+        //Default implementation do nothing
+    }
+
 }
diff --git a/.../java/com/dotcms/contenttype/business/uniquefields/UniqueFieldsValidationInitializer.java b/.../java/com/dotcms/contenttype/business/uniquefields/UniqueFieldsValidationInitializer.java
@@ -26,8 +26,8 @@
 @Dependent
 public class UniqueFieldsValidationInitializer  implements DotInitializer {
 
-    private UniqueFieldDataBaseUtil uniqueFieldDataBaseUtil;
-    private DotDatabaseMetaData dotDatabaseMetaData;
+    private final UniqueFieldDataBaseUtil uniqueFieldDataBaseUtil;
+    private final DotDatabaseMetaData dotDatabaseMetaData;
 
     @Inject
     public UniqueFieldsValidationInitializer(final UniqueFieldDataBaseUtil uniqueFieldDataBaseUtil){

diff --git a/.../dotcms/contenttype/business/uniquefields/extratable/DBUniqueFieldValidationStrategy.java b/.../dotcms/contenttype/business/uniquefields/extratable/DBUniqueFieldValidationStrategy.java
@@ -33,8 +33,7 @@
 import java.util.stream.Collectors;
 
 import static com.dotcms.content.elasticsearch.business.ESContentletAPIImpl.UNIQUE_PER_SITE_FIELD_VARIABLE_NAME;
-import static com.dotcms.contenttype.business.uniquefields.extratable.UniqueFieldCriteria.CONTENTLET_IDS_ATTR;
-import static com.dotcms.contenttype.business.uniquefields.extratable.UniqueFieldCriteria.UNIQUE_PER_SITE_ATTR;
+import static com.dotcms.contenttype.business.uniquefields.extratable.UniqueFieldCriteria.*;
 import static com.dotmarketing.util.Constants.DONT_RESPECT_FRONT_END_ROLES;
 
 /**
@@ -111,10 +110,40 @@ private static boolean isContentletBeingUpdated(final Contentlet contentlet) {
      */
     @SuppressWarnings("unchecked")
     private  void cleanUniqueFieldsUp(final Contentlet contentlet, final Field field) throws DotDataException {
-        Optional<Map<String, Object>> uniqueFieldOptional = uniqueFieldDataBaseUtil.get(contentlet, field);
+        List<Map<String, Object>> uniqueFields = uniqueFieldDataBaseUtil.get(contentlet, field);
+
+        if (UtilMethods.isSet(uniqueFields)) {
+            final List<Map<String, Object>> workingUniqueFields = uniqueFields.stream()
+                    .filter(uniqueValue -> Boolean.FALSE.equals(getSupportingValues(uniqueValue).get("live")))
+                    .collect(Collectors.toList());
+
+            if (!workingUniqueFields.isEmpty()) {
+                workingUniqueFields.forEach(uniqueField -> cleanUniqueFieldUp(contentlet.getIdentifier(), uniqueField));
+            } else  {
+                uniqueFields.stream()
+                        .filter(uniqueValue -> Boolean.TRUE.equals(getSupportingValues(uniqueValue).get("live")))
+                        .limit(1)
+                        .findFirst()
+                        .ifPresent(uniqueFieldValue -> {
+                            final Map<String, Object> supportingValues = getSupportingValues(uniqueFieldValue);
+                            final String oldUniqueValue = supportingValues.get(FIELD_VALUE_ATTR).toString();
+                            final String newUniqueValue = contentlet.getStringProperty(field.variable());
+
+                            if (oldUniqueValue.equals(newUniqueValue)) {
+                                cleanUniqueFieldUp(contentlet.getIdentifier(), uniqueFieldValue);
+                            }
+                        });
+            }
+
 
-        if (uniqueFieldOptional.isPresent()) {
-            cleanUniqueFieldUp(contentlet.getIdentifier(), uniqueFieldOptional.get());
+        }
+    }
+
+    private static Map<String, Object> getSupportingValues(Map<String, Object> uniqueField) {
+        try {
+            return JsonUtil.getJsonFromString(uniqueField.get("supporting_values").toString());
+        } catch (IOException e) {
+            throw new DotRuntimeException(e);
         }
     }
 
@@ -271,12 +300,18 @@ public void cleanUp(final Field field) throws DotDataException {
         uniqueFieldDataBaseUtil.delete(field);
     }
 
+    @Override
+    public void cleanUp(final ContentType contentType) throws DotDataException {
+        uniqueFieldDataBaseUtil.delete(contentType);
+    }
+
     @Override
     public void afterPublish(final String inode) {
         try {
             final Contentlet contentlet = APILocator.getContentletAPI().find(inode, APILocator.systemUser(), false);
 
             if (hasUniqueField(contentlet.getContentType())) {
+                uniqueFieldDataBaseUtil.removeLive(contentlet);
                 uniqueFieldDataBaseUtil.setLive(contentlet, true);
             }
         } catch (DotDataException | DotSecurityException e) {

diff --git a/...java/com/dotcms/contenttype/business/uniquefields/extratable/UniqueFieldDataBaseUtil.java b/...java/com/dotcms/contenttype/business/uniquefields/extratable/UniqueFieldDataBaseUtil.java
@@ -5,6 +5,7 @@
 import com.dotcms.business.WrapInTransaction;
 
 import com.dotcms.contenttype.model.field.Field;
+import com.dotcms.contenttype.model.type.ContentType;
 import com.dotmarketing.common.db.DotConnect;
 import com.dotmarketing.exception.DotDataException;
 import com.dotmarketing.exception.DotRuntimeException;
@@ -62,7 +63,6 @@ public class UniqueFieldDataBaseUtil {
             "WHERE supporting_values->'" + CONTENTLET_IDS_ATTR + "' @> ?::jsonb " +
             "AND supporting_values->>'" + VARIANT_ATTR + "' = ? " +
             "AND (supporting_values->>'"+ LANGUAGE_ID_ATTR + "')::INTEGER = ? " +
-            "AND (supporting_values->>'" + LIVE_ATTR + "')::BOOLEAN = ? " +
             "AND supporting_values->>'" + FIELD_VARIABLE_NAME_ATTR + "' = ?";
 
     private final static String DELETE_UNIQUE_FIELDS_BY_CONTENTLET = "DELETE FROM unique_fields " +
@@ -89,6 +89,10 @@ public class UniqueFieldDataBaseUtil {
     private final static String DELETE_UNIQUE_FIELDS_BY_FIELD = "DELETE FROM unique_fields " +
             "WHERE supporting_values->>'" + FIELD_VARIABLE_NAME_ATTR + "' = ?";
 
+    private final static String DELETE_UNIQUE_FIELDS_BY_CONTENT_TYPE = "DELETE FROM unique_fields " +
+            "WHERE supporting_values->>'" + CONTENT_TYPE_ID_ATTR + "' = ?";
+
+
     private final static String POPULATE_UNIQUE_FIELDS_VALUES_QUERY = "INSERT INTO unique_fields (unique_key_val, supporting_values) " +
             "SELECT encode(sha256(CONCAT(content_type_id, field_var_name, language_id, field_value, " +
             "                  CASE WHEN uniquePerSite = 'true' THEN host_id ELSE '' END)::bytea), 'hex') as unique_key_val, " +
@@ -208,20 +212,13 @@ public void updateContentListWithHash(final String hash, final List<String> cont
      * @throws DotDataException If an error occurs when interacting with the database.
      */
     @CloseDBIfOpened
-    public Optional<Map<String, Object>> get(final Contentlet contentlet, final Field field) throws DotDataException {
-        try {
-            final List<Map<String, Object>> results = new DotConnect().setSQL(GET_UNIQUE_FIELDS_BY_CONTENTLET)
-                    .addParam("\"" + contentlet.getIdentifier() + "\"")
-                    .addParam(contentlet.getVariantId())
-                    .addParam(contentlet.getLanguageId())
-                    .addParam(contentlet.isLive())
-                    .addParam(field.variable())
-                    .loadObjectResults();
-
-            return results.isEmpty() ? Optional.empty() : Optional.of(results.get(0));
-        } catch (DotSecurityException e) {
-            throw new DotRuntimeException(e);
-        }
+    public List<Map<String, Object>> get(final Contentlet contentlet, final Field field) throws DotDataException {
+        return new DotConnect().setSQL(GET_UNIQUE_FIELDS_BY_CONTENTLET)
+                .addParam("\"" + contentlet.getIdentifier() + "\"")
+                .addParam(contentlet.getVariantId())
+                .addParam(contentlet.getLanguageId())
+                .addParam(field.variable())
+                .loadObjectResults();
     }
 
     /**
@@ -275,10 +272,10 @@ private static String getUniqueRecalculationQuery(final boolean uniquePerSite) {
     }
 
     @CloseDBIfOpened
-    public List<Map<String, Object>> get(final String contentId, final long languegeId) throws DotDataException {
+    public List<Map<String, Object>> get(final String contentId, final long languageId) throws DotDataException {
         return new DotConnect().setSQL(GET_UNIQUE_FIELDS_BY_CONTENTLET_AND_LANGUAGE)
                 .addParam("\"" + contentId + "\"")
-                .addParam(languegeId)
+                .addParam(languageId)
                 .loadObjectResults();
     }
 
@@ -324,6 +321,19 @@ public void delete(final Field field) throws DotDataException {
                 .loadObjectResults();
     }
 
+    /**
+     * Delete all the unique values for a {@link ContentType}
+     *
+     * @param contentType
+     * @throws DotDataException
+     */
+    @WrapInTransaction
+    public void delete(final ContentType contentType) throws DotDataException {
+        new DotConnect().setSQL(DELETE_UNIQUE_FIELDS_BY_CONTENT_TYPE)
+                .addParam(contentType.id())
+                .loadObjectResults();
+    }
+
     /**
      * Set the supporting_value->live attribute to true to any register with the same Content's id, variant and language
      *

diff --git a/...ava/com/dotcms/contenttype/business/uniquefields/extratable/UniqueFieldsTableCleaner.java b/...ava/com/dotcms/contenttype/business/uniquefields/extratable/UniqueFieldsTableCleaner.java
@@ -1,6 +1,7 @@
 package com.dotcms.contenttype.business.uniquefields.extratable;
 
 import com.dotcms.contenttype.business.uniquefields.UniqueFieldValidationStrategyResolver;
+import com.dotcms.contenttype.model.event.ContentTypeDeletedEvent;
 import com.dotcms.contenttype.model.field.Field;
 import com.dotcms.contenttype.model.field.event.FieldDeletedEvent;
 import com.dotcms.contenttype.model.type.ContentType;
@@ -68,7 +69,7 @@ public void cleanUpAfterDeleteContentlet(final DeleteContentletVersionInfoEvent
             final ContentType contentType = APILocator.getContentTypeAPI(APILocator.systemUser())
                     .find(contentlet.getContentTypeId());
 
-            boolean hasUniqueField = contentType.fields().stream().anyMatch(Field::unique);
+            boolean hasUniqueField = hasUniqueField(contentType);
 
             if (hasUniqueField) {
                 uniqueFieldValidationStrategyResolver.get().cleanUp(contentlet, event.isDeleteAllVariant());
@@ -78,8 +79,12 @@ public void cleanUpAfterDeleteContentlet(final DeleteContentletVersionInfoEvent
         }
     }
 
+    private static boolean hasUniqueField(ContentType contentType) {
+        return contentType.fields().stream().anyMatch(Field::unique);
+    }
+
     /**
-     * Listen when a Field is deleted and if this ia a Unique Field then delete all the register in
+     * Listen when a Field is deleted and if this is a Unique Field then delete all the register in
      * unique_fields table for this Field
      *
      * @param event
@@ -94,4 +99,23 @@ public void cleanUpAfterDeleteUniqueField(final FieldDeletedEvent event) throws
             uniqueFieldValidationStrategyResolver.get().cleanUp(deletedField);
         }
     }
+
+    /**
+     * Listen when a {@link ContentType} is deleted and if this has at least one Unique Field then delete all the register in
+     * unique_fields table for this {@link ContentType}
+     *
+     * @param event
+     *
+     * @throws DotDataException
+     */
+    @Subscriber
+    public void cleanUpAfterDeleteContentType(final ContentTypeDeletedEvent contentTypeDeletedEvent) throws DotDataException {
+        final ContentType contentType = contentTypeDeletedEvent.getContentType();
+
+        boolean hasUniqueField = hasUniqueField(contentType);
+
+        if (hasUniqueField) {
+            uniqueFieldValidationStrategyResolver.get().cleanUp(contentType);
+        }
+    }
 }
diff --git a/dotCMS/src/main/java/com/dotcms/contenttype/model/event/ContentTypeDeletedEvent.java b/dotCMS/src/main/java/com/dotcms/contenttype/model/event/ContentTypeDeletedEvent.java
@@ -1,14 +1,20 @@
 package com.dotcms.contenttype.model.event;
 
+import com.dotcms.contenttype.model.type.ContentType;
+
 public class ContentTypeDeletedEvent {
 
-    private final String contentTypeVar;
+    private final ContentType contentType;
 
-    public ContentTypeDeletedEvent(final String contentTypeVar) {
-        this.contentTypeVar = contentTypeVar;
+    public ContentTypeDeletedEvent(final ContentType contentType) {
+        this.contentType = contentType;
     }
 
     public String getContentTypeVar() {
-        return contentTypeVar;
+        return contentType.variable();
+    }
+
+    public ContentType getContentType() {
+        return contentType;
     }
 }