Skip to content

Commit

Permalink
fix(sites): Unable to Delete Site with Content Types and Contentlet #…
Browse files Browse the repository at this point in the history 
…27862  (#28044)

* fix(sites) #27862 : Unable to Delete Site with Content Types and Contentlet

* Testing code change in Integration Test.

* Adding Code Review feedback form Daniel Silva. Updating Integration Test.
  • Loading branch information
@jcastro-dotcms
jcastro-dotcms committed Mar 22, 2024
1 parent 1e1f69f commit c7e4c5c
Show file tree
Hide file tree
Showing 8 changed files with 352 additions and 152 deletions.
68 changes: 42 additions & 26 deletions dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java
View file
Expand Up @@ -31,6 +31,7 @@
import com.dotcms.contenttype.transform.contenttype.ContentTypeTransformer;
import com.dotcms.contenttype.transform.contenttype.StructureTransformer;
import com.dotcms.contenttype.transform.field.LegacyFieldTransformer;
import com.dotcms.exception.ExceptionUtil;
import com.dotcms.notifications.bean.NotificationLevel;
import com.dotcms.publisher.business.DotPublisherException;
import com.dotcms.publisher.business.PublisherAPI;
Expand Down Expand Up @@ -2416,25 +2417,24 @@ public boolean delete(final Contentlet contentlet, final User user,
final Optional<Boolean> deleteOpt = this.checkAndRunDeleteAsWorkflow(contentlet, user,
respectFrontendRoles);
if (deleteOpt.isPresent()) {

Logger.info(this, "A Workflow has been ran instead of delete the contentlet: " +
contentlet.getIdentifier());

Logger.info(this, String.format("A delete workflow has been used to delete Contentlet ID " +
"'%s' instead of calling the APIs", contentlet.getIdentifier()));
return deleteOpt.get();
}

boolean deleted = false;
boolean deleted;
final List<Contentlet> contentlets = new ArrayList<>();
contentlets.add(contentlet);

try {
deleted = delete(contentlets, user, respectFrontendRoles);
boolean isSite = contentlet.isHost();
deleted = this.deleteContentlets(contentlets, user, respectFrontendRoles, isSite);
HibernateUtil.addCommitListener
(() -> this.localSystemEventsAPI.notify(
new ContentletDeletedEvent(contentlet, user)));
} catch (DotDataException | DotSecurityException e) {

logContentletActivity(contentlets, "Error Deleting Content", user);
new ContentletDeletedEvent<>(contentlet, user)));
} catch (final DotDataException | DotSecurityException e) {
this.logContentletActivity(contentlets, String.format("Error deleting content with ID " +
"'%s': %s", contentlet.getIdentifier(), ExceptionUtil.getErrorMessage(e)), user);
throw e;
}

Expand Down Expand Up @@ -2953,10 +2953,12 @@ private void backupDestroyedContentlets(final List<Contentlet> contentlets, fina
* any of the specified contentlets is not archived, an exception will be thrown. If there's
* only one language for a given contentlet, the object will be destroyed.
*
* @param contentlets - The list of contentlets that will be deleted.
* @param user - The {@link User} performing this action.
* @param respectFrontendRoles -
* @param isDeletingAHost - If the code calling this method is trying to delete a given
* @param contentlets The list of contentlets that will be deleted.
* @param user The {@link User} performing this action.
* @param respectFrontendRoles If the User executing this action has the front-end role, or if
* front-end roles must be validated against this user, set to
* {@code true}.
* @param isDeletingAHost If the code calling this method is trying to delete a given
* Site (host), set to {@code true}. Otherwise, set to
* {@code false}.
* @return If the contentlets were successfully deleted, returns {@code true}. Otherwise,
Expand All @@ -2974,21 +2976,20 @@ private boolean deleteContentlets(final List<Contentlet> contentlets, final User

boolean noErrors = true;

if (contentlets == null || contentlets.size() == 0) {

Logger.info(this, "No contents passed to delete so returning");
if (UtilMethods.isNotSet(contentlets)) {
Logger.info(this, "No contents passed to delete");
noErrors = false;
return noErrors;
}

this.logContentletActivity(contentlets, "Deleting Content", user);
this.logContentletActivity(contentlets, "Deleting Contents", user);

final List<Contentlet> filteredContentlets = this.validateAndFilterContentletsToDelete(
contentlets, user, respectFrontendRoles);

if (filteredContentlets.size() != contentlets.size()) {

this.logContentletActivity(contentlets, "Error Deleting Content", user);
this.logContentletActivity(contentlets, "Error Deleting Contents.", user);
throw new DotSecurityException("User: " + (user != null ? user.getUserId() : "Unknown")
+ " does not have permission to delete some or all of the contentlets");
}
Expand All @@ -3000,21 +3001,36 @@ private boolean deleteContentlets(final List<Contentlet> contentlets, final User
contentletIdentifiers.add(contentlet.getIdentifier());
}

AdminLogger.log(this.getClass(), "delete",
"User trying to delete the following contents: " +
contentletIdentifiers.toString(), user);
AdminLogger.log(this.getClass(), "delete", "User trying to delete the following contents: "
+ contentletIdentifiers, user);

final HashSet<String> deletedIdentifiers = new HashSet();
final Iterator<Contentlet> contentletIterator = filteredContentlets.iterator();
while (contentletIterator.hasNext()) {
final HashSet<String> deletedIdentifiers = new HashSet<>();
for (final Contentlet filteredContentlet : filteredContentlets) {

this.deleteContentlet(contentlets, user, isDeletingAHost,
deletedIdentifiers, contentletIterator.next());
deletedIdentifiers, filteredContentlet);
}

return noErrors;
}

/**
* Deletes the specified list of Contentlets.
*
* @param contentlets The complete list of {@link Contentlet} objects to delete.
* @param user The {@link User} that is trying to delete such Contentlets.
* @param isDeletingAHost If the Contentlet being deleted is a Site, set this to {@code true}
* so that the "destroy" method is called directly, with no
* validations.
* @param deletedIdentifiers A list that keeps track of the Identifiers that have already been
* deleted.
* @param contentletToDelete The actual {@link Contentlet} that is being deleted.
*
* @throws DotDataException The specified {@link User} does not have the required
* permissions to perform this action.
* @throws DotSecurityException An error occurred when deleting the information from the
* database.
*/
private void deleteContentlet(final List<Contentlet> contentlets, final User user,
final boolean isDeletingAHost,
final HashSet<String> deletedIdentifiers, final Contentlet contentletToDelete)
Expand Down
51 changes: 38 additions & 13 deletions dotCMS/src/main/java/com/dotcms/contenttype/business/ContentTypeAPI.java
View file
@@ -1,5 +1,6 @@
package com.dotcms.contenttype.business;

import com.dotcms.contenttype.exception.NotFoundInDbException;
import com.dotcms.contenttype.model.field.Field;
import com.dotcms.contenttype.model.field.FieldVariable;
import com.dotcms.contenttype.model.type.BaseContentType;
Expand Down Expand Up @@ -53,23 +54,47 @@ public interface ContentTypeAPI {
"container", "template", "user", "calendarEvent");

/**
* Deletes the given Content Type
*
* @param st Content Type that will be deleted
* @throws DotSecurityException The user does not have permissions to perform this action.
* @throws DotDataException Error occurred when performing the action.
* Deletes the specified Content Type. By default, the process that actually deletes it is
* asynchronous, meaning that a separate thread/transaction takes care of it. You can change this
* behavior by updating the value of the {@link ContentTypeAPIImpl#DELETE_CONTENT_TYPE_ASYNC}
* property to {@code true}.
*
* @param contentType The {@link ContentType} being deleted.
*
* @throws DotSecurityException The User accessing this API does not have the required
* permissions to perform this action.
* @throws DotDataException An error occurred when interacting with the database.
*/
void delete(ContentType st) throws DotSecurityException, DotDataException;
void delete(final ContentType contentType) throws DotSecurityException, DotDataException;

/**
* Find a Content Type given the inode
*
* @param inodeOrVar Either the Inode or the Velocity var name representing the Structure to find
* @return Content Type Object
* @throws DotSecurityException The user does not have permissions to perform this action.
* @throws DotDataException Error occurred when performing the action.
* Deletes the specified Content Type. By default, the process that actually deletes it is
* asynchronous, meaning that a separate thread/transaction takes care of it. With this
* implementation, you can force dotCMS to <b>wait for the deletion process to be over before
* moving on</b>. Therefore, given the implications related to performance, this method must be
* used carefully
*
* @param contentType The {@link ContentType} being deleted.
*
* @throws DotSecurityException The User accessing this API does not have the required
* permissions to perform this action.
* @throws DotDataException An error occurred when interacting with the database.
*/
void deleteSync(final ContentType contentType) throws DotSecurityException, DotDataException;

/**
* Returns the Content Type that matches the specified Inode or Velocity Variable Name.
*
* @param inodeOrVar Either the Inode or the Velocity var name representing the Content Type to
* find.
*
* @return The {@link ContentType} that was requested.
*
* @throws DotSecurityException The user does not have permissions to perform this action.
* @throws DotDataException Error occurred when performing the action.
* @throws NotFoundInDbException The Content Type was not found in the database.
*/
ContentType find(String inodeOrVar) throws DotSecurityException, DotDataException;
ContentType find(final String inodeOrVar) throws DotSecurityException, DotDataException;

/**
* Returns a list of Content Types based on the specified list of Velocity Variable Names. If one or more Velocity
Expand Down
62 changes: 45 additions & 17 deletions dotCMS/src/main/java/com/dotcms/contenttype/business/ContentTypeAPIImpl.java
View file
Expand Up @@ -118,29 +118,45 @@ public ContentTypeAPIImpl(User user, boolean respectFrontendRoles) {
APILocator.getPermissionAPI(), APILocator.getContentTypeFieldAPI(), APILocator.getLocalSystemEventsAPI());
}

@Override
public void delete(final ContentType contentType) throws DotSecurityException, DotDataException {
this.delete(contentType, true);
}

@Override
public void deleteSync(final ContentType contentType) throws DotSecurityException, DotDataException {
this.delete(contentType, false);
}

/**
* Content-Type Delete Entry point
* @param type Content Type that will be deleted
* @throws DotSecurityException
* @throws DotDataException
* Deletes the specified Content Type, either in the same database transaction or as a separate
* process.
*
* @param contentType The {@link ContentType} being deleted.
* @param async If the deletion process should be executed asynchronously -- i.e.; in a
* separate process, set this to {@code true}.
*
* @throws DotSecurityException The specified User does not have edition permissions on this
* Content Type.
* @throws DotDataException An error occurred when interacting with the database.
*/
@Override
public void delete(ContentType type) throws DotSecurityException, DotDataException {
if (!contentTypeCanBeDeleted(type)) {
Logger.warn(this, "Content Type " + type.name()
+ " cannot be deleted because it is referenced by other content types");
private void delete(final ContentType contentType, final boolean async) throws DotSecurityException, DotDataException {
if (!contentTypeCanBeDeleted(contentType)) {
Logger.warn(this, String.format("Content Type '%s' does not exist", contentType.name()));
return;
}

final boolean asyncDelete = Config.getBooleanProperty(DELETE_CONTENT_TYPE_ASYNC, true);;
final boolean asyncDeleteWithJob = Config.getBooleanProperty(DELETE_CONTENT_TYPE_ASYNC_WITH_JOB, true);
boolean asyncDelete = async;
boolean asyncDeleteWithJob = Config.getBooleanProperty(DELETE_CONTENT_TYPE_ASYNC_WITH_JOB, true);
if (async) {
asyncDelete = Config.getBooleanProperty(DELETE_CONTENT_TYPE_ASYNC, true);
}

if (!asyncDelete) {
Logger.debug(this, String.format(" Content type (%s) will be deleted sequentially.", type.name()));
transactionalDelete(type);
Logger.debug(this, () -> String.format("Content Type '%s' will be deleted synchronously", contentType.name()));
this.transactionalDelete(contentType);
} else {
//We make a copy to hold all the contentlets that will be deleted asynchronously and then dispose the original one
triggerAsyncDelete(type, asyncDeleteWithJob);
this.triggerAsyncDelete(contentType, asyncDeleteWithJob);
}
}

Expand Down Expand Up @@ -209,8 +225,20 @@ private void transactionalDelete(ContentType type) throws DotDataException {
HibernateUtil.addCommitListener(() -> localSystemEventsAPI.notify(new ContentTypeDeletedEvent(type.variable())));
}

boolean contentTypeCanBeDeleted(ContentType type) throws DotDataException, DotSecurityException {

/**
* Verifies whether the {@link User} calling this method has the required {@code EDIT}
* permissions to delete the specified Content Type or not.
*
* @param type The {@link ContentType} to be deleted.
*
* @return If the {@link User} has the required permissions to delete the specified Content
* Type, returns {@code true}.
*
* @throws DotDataException An error occurred when accessing the database.
* @throws DotSecurityException The specified User does not have the necessary permissions to
* perform this action.
*/
protected boolean contentTypeCanBeDeleted(ContentType type) throws DotDataException, DotSecurityException {
if (null == type.id()) {
throw new DotDataException("ContentType must have an id set");
}
Expand Down

0 comments on commit c7e4c5c

Please sign in to comment.