diff --git a/dotCMS/src/main/java/com/dotcms/contenttype/business/FieldAPIImpl.java b/dotCMS/src/main/java/com/dotcms/contenttype/business/FieldAPIImpl.java index 8825e97669e8..208e4e66a2d1 100644 --- a/dotCMS/src/main/java/com/dotcms/contenttype/business/FieldAPIImpl.java +++ b/dotCMS/src/main/java/com/dotcms/contenttype/business/FieldAPIImpl.java @@ -75,6 +75,7 @@ import com.dotmarketing.portlets.contentlet.business.ContentletAPI; import com.dotmarketing.portlets.structure.model.Relationship; import com.dotmarketing.portlets.structure.model.Structure; +import com.dotmarketing.quartz.QuartzUtils; import com.dotmarketing.quartz.job.CleanUpFieldReferencesJob; import com.dotmarketing.util.ActivityLogger; import com.dotmarketing.util.Config; @@ -92,12 +93,7 @@ import org.apache.commons.lang.StringUtils; import java.net.ConnectException; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.Optional; +import java.util.*; import static com.dotcms.content.elasticsearch.business.ESContentletAPIImpl.UNIQUE_PER_SITE_FIELD_VARIABLE_NAME; import static com.dotcms.util.CollectionsUtils.list; @@ -106,65 +102,69 @@ public class FieldAPIImpl implements FieldAPI { - private final List> baseFieldTypes = ImmutableList.of(BinaryField.class, StoryBlockField.class, - CategoryField.class, ConstantField.class, CheckboxField.class, CustomField.class, DateField.class, - DateTimeField.class, FileField.class, HiddenField.class, HostFolderField.class, - ImageField.class, KeyValueField.class, LineDividerField.class, MultiSelectField.class, - PermissionTabField.class, RadioField.class, RelationshipField.class, RelationshipsTabField.class, SelectField.class, - TabDividerField.class, TagField.class, TextAreaField.class, TimeField.class, - WysiwygField.class); - - private final PermissionAPI permissionAPI; - private final ContentletAPI contentletAPI; - private final UserAPI userAPI; - private final RelationshipAPI relationshipAPI; - private final LocalSystemEventsAPI localSystemEventsAPI; - private final LanguageVariableAPI languageVariableAPI; - private final FieldFactory fieldFactory; - - public FieldAPIImpl() { - this(APILocator.getPermissionAPI(), - APILocator.getContentletAPI(), - APILocator.getUserAPI(), - APILocator.getRelationshipAPI(), - APILocator.getLocalSystemEventsAPI(), - APILocator.getLanguageVariableAPI(), - FactoryLocator.getFieldFactory()); - } + private final List> baseFieldTypes = ImmutableList.of(BinaryField.class, StoryBlockField.class, + CategoryField.class, ConstantField.class, CheckboxField.class, CustomField.class, DateField.class, + DateTimeField.class, FileField.class, HiddenField.class, HostFolderField.class, + ImageField.class, KeyValueField.class, LineDividerField.class, MultiSelectField.class, + PermissionTabField.class, RadioField.class, RelationshipField.class, RelationshipsTabField.class, SelectField.class, + TabDividerField.class, TagField.class, TextAreaField.class, TimeField.class, + WysiwygField.class); + + private final PermissionAPI permissionAPI; + private final ContentletAPI contentletAPI; + private final UserAPI userAPI; + private final RelationshipAPI relationshipAPI; + private final LocalSystemEventsAPI localSystemEventsAPI; + private final LanguageVariableAPI languageVariableAPI; + private final FieldFactory fieldFactory; + + public FieldAPIImpl() { + this(APILocator.getPermissionAPI(), + APILocator.getContentletAPI(), + APILocator.getUserAPI(), + APILocator.getRelationshipAPI(), + APILocator.getLocalSystemEventsAPI(), + APILocator.getLanguageVariableAPI(), + FactoryLocator.getFieldFactory()); + } - @VisibleForTesting - public FieldAPIImpl(final PermissionAPI perAPI, - final ContentletAPI conAPI, - final UserAPI userAPI, - final RelationshipAPI relationshipAPI, - final LocalSystemEventsAPI localSystemEventsAPI, - final LanguageVariableAPI languageVariableAPI, - final FieldFactory fieldFactory) { - this.permissionAPI = perAPI; - this.contentletAPI = conAPI; - this.userAPI = userAPI; - this.relationshipAPI = relationshipAPI; - this.localSystemEventsAPI = localSystemEventsAPI; - this.languageVariableAPI = languageVariableAPI; - this.fieldFactory = fieldFactory; - } + @VisibleForTesting + public FieldAPIImpl(final PermissionAPI perAPI, + final ContentletAPI conAPI, + final UserAPI userAPI, + final RelationshipAPI relationshipAPI, + final LocalSystemEventsAPI localSystemEventsAPI, + final LanguageVariableAPI languageVariableAPI, + final FieldFactory fieldFactory) { + this.permissionAPI = perAPI; + this.contentletAPI = conAPI; + this.userAPI = userAPI; + this.relationshipAPI = relationshipAPI; + this.localSystemEventsAPI = localSystemEventsAPI; + this.languageVariableAPI = languageVariableAPI; + this.fieldFactory = fieldFactory; + } - @WrapInTransaction - @Override - public Field save(final Field field, final User user) throws DotDataException, DotSecurityException { + @WrapInTransaction + @Override + public Field save(final Field field, final User user) throws DotDataException, DotSecurityException { return save(field, user, true); - } + } @WrapInTransaction @Override public Field save(final Field field, final User user, final boolean reorder) throws DotDataException, DotSecurityException { + + if (!UtilMethods.isSet(field.contentTypeId())) { Logger.error(this, "ContentTypeId needs to be set to save the Field"); throw new DotDataValidationException("ContentTypeId needs to be set to save the Field"); } + QuartzUtils.validateFieldReferences(field); + ContentTypeAPI contentTypeAPI = APILocator.getContentTypeAPI(user); ContentType type = contentTypeAPI.find(field.contentTypeId()); permissionAPI.checkPermission(type, PermissionLevel.EDIT_PERMISSIONS, user); @@ -247,6 +247,7 @@ public Field save(final Field field, final User user, final boolean reorder) return result; } + /** * Processes relationships for the given field. If the field is a {@link RelationshipField}, it * ensures that the relationship record is added or updated. @@ -260,7 +261,7 @@ public Field save(final Field field, final User user, final boolean reorder) * @throws DotSecurityException if a security violation occurs */ private Field processRelationships(final Field field, final User user, - final ContentTypeAPI contentTypeAPI, final ContentType type) + final ContentTypeAPI contentTypeAPI, final ContentType type) throws DotDataException, DotSecurityException { //if RelationshipField, Relationship record must be added/updated @@ -298,7 +299,7 @@ private Field processRelationships(final Field field, final User user, * @throws DotDataException if a data access error occurs */ private void validateAndReorder(final String contentTypeId, final boolean reorder, - final Field field, final Field oldField) throws DotDataException { + final Field field, final Field oldField) throws DotDataException { if (null != oldField) { @@ -481,7 +482,7 @@ private void validateRelationshipField(Field field) throws DotDataValidationExce */ @VisibleForTesting Optional getRelationshipForField(final Field field, final ContentTypeAPI contentTypeAPI, - final ContentType type, final User user) throws DotDataException, DotSecurityException { + final ContentType type, final User user) throws DotDataException, DotSecurityException { Relationship relationship; ContentType relatedContentType; try { @@ -540,7 +541,7 @@ Optional getRelationshipForField(final Field field, final ContentT * @throws DotDataException */ private void updateRelationshipObject(final Field field, final ContentType type, final ContentType relatedContentType, - final Relationship relationship, final int cardinality, final User user) + final Relationship relationship, final int cardinality, final User user) throws DotDataException { final boolean isChildField; @@ -627,17 +628,17 @@ private void sendRelationshipErrorMessage( try { systemMessageEventUtil.pushMessage( - new SystemMessageBuilder() - .setMessage(LanguageUtil.format( - user.getLocale(), - "contenttypes.field.properties.relationship.required.error", - relationName) - ) - .setSeverity(MessageSeverity.INFO) - .setType(MessageType.SIMPLE_MESSAGE) - .setLife(6000) - .create(), - list(user.getUserId()) + new SystemMessageBuilder() + .setMessage(LanguageUtil.format( + user.getLocale(), + "contenttypes.field.properties.relationship.required.error", + relationName) + ) + .setSeverity(MessageSeverity.INFO) + .setType(MessageType.SIMPLE_MESSAGE) + .setLife(6000) + .create(), + list(user.getUserId()) ); } catch (LanguageException e) { throw new DotRuntimeException(e); @@ -645,54 +646,54 @@ private void sendRelationshipErrorMessage( } @WrapInTransaction - @Override - public FieldVariable save(final FieldVariable var, final User user) throws DotDataException, DotSecurityException { - ContentTypeAPI contentTypeAPI = APILocator.getContentTypeAPI(user); - Field field = fieldFactory.byId(var.fieldId()); - - ContentType type = contentTypeAPI.find(field.contentTypeId()) ; - APILocator.getPermissionAPI().checkPermission(type, PermissionLevel.EDIT_PERMISSIONS, user); - - FieldVariable newFieldVariable = fieldFactory.save(ImmutableFieldVariable.builder().from(var).userId(user.getUserId()).build()); - - //update Content Type mod_date to detect the changes done on the field variables - contentTypeAPI.updateModDate(type); - - //Validates custom mapping format - if (var.key().equals(FieldVariable.ES_CUSTOM_MAPPING_KEY)){ - try { - new JSONObject(var.value()); - } catch (JSONException e) { - handleInvalidCustomMappingError(var, user, field, type, e); - } - - //Verifies the field is marked as System Indexed. In case it isn't, the field will be updated with this flag on - if (!field.indexed()) { - save(FieldBuilder.builder(field).indexed(true).build(), user); - Logger.info(this, "Field " + type.variable() + "." + field.variable() - + " has been marked as System Indexed as it has defined a field variable with key " - + FieldVariable.ES_CUSTOM_MAPPING_KEY); - } - } else if (var.key().equals(UNIQUE_PER_SITE_FIELD_VARIABLE_NAME)) { - final Optional previousValueOpt = field.fieldVariableValue(UNIQUE_PER_SITE_FIELD_VARIABLE_NAME); - if (previousValueOpt.isPresent() && previousValueOpt.get().equalsIgnoreCase(newFieldVariable.value())) { - // 'uniquePerSite' value was not changed, do not recalculate - return newFieldVariable; - } - final UniqueFieldValidationStrategyResolver resolver = - CDIUtils.getBeanThrows(UniqueFieldValidationStrategyResolver.class); - try { - this.sendStartRecalculationNotification(user, field); - resolver.get().recalculate(field, Boolean.parseBoolean(newFieldVariable.value())); - this.sendEndRecalculationNotification(user, field); - } catch (final UniqueFieldValueDuplicatedException e) { - this.sendFailedRecalculationNotification(user, field); - Logger.error(this, ExceptionUtil.getErrorMessage(e), e); - throw new DotDataException(e); + @Override + public FieldVariable save(final FieldVariable var, final User user) throws DotDataException, DotSecurityException { + ContentTypeAPI contentTypeAPI = APILocator.getContentTypeAPI(user); + Field field = fieldFactory.byId(var.fieldId()); + + ContentType type = contentTypeAPI.find(field.contentTypeId()) ; + APILocator.getPermissionAPI().checkPermission(type, PermissionLevel.EDIT_PERMISSIONS, user); + + FieldVariable newFieldVariable = fieldFactory.save(ImmutableFieldVariable.builder().from(var).userId(user.getUserId()).build()); + + //update Content Type mod_date to detect the changes done on the field variables + contentTypeAPI.updateModDate(type); + + //Validates custom mapping format + if (var.key().equals(FieldVariable.ES_CUSTOM_MAPPING_KEY)){ + try { + new JSONObject(var.value()); + } catch (JSONException e) { + handleInvalidCustomMappingError(var, user, field, type, e); + } + + //Verifies the field is marked as System Indexed. In case it isn't, the field will be updated with this flag on + if (!field.indexed()) { + save(FieldBuilder.builder(field).indexed(true).build(), user); + Logger.info(this, "Field " + type.variable() + "." + field.variable() + + " has been marked as System Indexed as it has defined a field variable with key " + + FieldVariable.ES_CUSTOM_MAPPING_KEY); + } + } else if (var.key().equals(UNIQUE_PER_SITE_FIELD_VARIABLE_NAME)) { + final Optional previousValueOpt = field.fieldVariableValue(UNIQUE_PER_SITE_FIELD_VARIABLE_NAME); + if (previousValueOpt.isPresent() && previousValueOpt.get().equalsIgnoreCase(newFieldVariable.value())) { + // 'uniquePerSite' value was not changed, do not recalculate + return newFieldVariable; + } + final UniqueFieldValidationStrategyResolver resolver = + CDIUtils.getBeanThrows(UniqueFieldValidationStrategyResolver.class); + try { + this.sendStartRecalculationNotification(user, field); + resolver.get().recalculate(field, Boolean.parseBoolean(newFieldVariable.value())); + this.sendEndRecalculationNotification(user, field); + } catch (final UniqueFieldValueDuplicatedException e) { + this.sendFailedRecalculationNotification(user, field); + Logger.error(this, ExceptionUtil.getErrorMessage(e), e); + throw new DotDataException(e); + } } - } - return newFieldVariable; - } + return newFieldVariable; + } /** * Sends a system message (warning) when a custom mapping is invalid @@ -703,7 +704,7 @@ public FieldVariable save(final FieldVariable var, final User user) throws DotDa * @param exception */ private void handleInvalidCustomMappingError(final FieldVariable fieldVariable, final User user, - final Field field, final ContentType type, final JSONException exception) { + final Field field, final ContentType type, final JSONException exception) { final String message; try { message = "Invalid format on field variable value. Value should be a JSON object. Field variable: " @@ -730,57 +731,60 @@ private void handleInvalidCustomMappingError(final FieldVariable fieldVariable, } @Override - public void delete(final Field field) throws DotDataException { - try { - this.delete(field, this.userAPI.getSystemUser()); - } catch (DotSecurityException e){ - throw new DotDataException(e); - } - } + public void delete(final Field field) throws DotDataException { + try { + this.delete(field, this.userAPI.getSystemUser()); + } catch (DotSecurityException e){ + throw new DotDataException(e); + } + } - @WrapInTransaction - @Override - public void delete(final Field field, final User user) throws DotDataException, DotSecurityException { + @WrapInTransaction + @Override + public void delete(final Field field, final User user) throws DotDataException, DotSecurityException { - final ContentTypeAPI contentTypeAPI = APILocator.getContentTypeAPI(user); - final ContentType type = contentTypeAPI.find(field.contentTypeId()); + final ContentTypeAPI contentTypeAPI = APILocator.getContentTypeAPI(user); + final ContentType type = contentTypeAPI.find(field.contentTypeId()); - permissionAPI.checkPermission(type, PermissionLevel.EDIT_PERMISSIONS, user); + permissionAPI.checkPermission(type, PermissionLevel.EDIT_PERMISSIONS, user); - Field oldField = fieldFactory.byId(field.id()); - if(oldField.fixed() || oldField.readOnly()){ - throw new DotDataException("You cannot delete a fixed or read only field"); - } + Field oldField = fieldFactory.byId(field.id()); + if(oldField.fixed() || oldField.readOnly()){ + throw new DotDataException("You cannot delete a fixed or read only field"); + } - final Structure structure = new StructureTransformer(type).asStructure(); + final Structure structure = new StructureTransformer(type).asStructure(); - fieldFactory.moveSortOrderBackward(type.id(), oldField.sortOrder()); + fieldFactory.moveSortOrderBackward(type.id(), oldField.sortOrder()); - fieldFactory.delete(field); + fieldFactory.delete(field); - ActivityLogger.logInfo(ActivityLogger.class, "Delete Field Action", - String.format("User %s/%s deleted field %s from %s Content Type.", user.getUserId(), user.getFirstName(), - field.name(), structure.getName())); + ActivityLogger.logInfo(ActivityLogger.class, "Delete Field Action", + String.format("User %s/%s deleted field %s from %s Content Type.", user.getUserId(), user.getFirstName(), + field.name(), structure.getName())); - //update Content Type mod_date to detect the changes done on the field - contentTypeAPI.updateModDate(type); + //update Content Type mod_date to detect the changes done on the field + contentTypeAPI.updateModDate(type); - CacheLocator.getContentTypeCache().remove(structure); + CacheLocator.getContentTypeCache().remove(structure); - //if RelationshipField, Relationship record must be updated/deleted - if (field instanceof RelationshipField) { - removeRelationshipLink(field, type, contentTypeAPI); - } + //if RelationshipField, Relationship record must be updated/deleted + if (field instanceof RelationshipField) { + removeRelationshipLink(field, type, contentTypeAPI); + } - // rebuild contentlets indexes - if(field.indexed()){ - contentletAPI.reindex(structure); - } + // rebuild contentlets indexes + if(field.indexed()){ + contentletAPI.reindex(structure); + } - CleanUpFieldReferencesJob.triggerCleanUpJob(field, user); - localSystemEventsAPI.notify(new FieldDeletedEvent(field)); + CleanUpFieldReferencesJob.triggerCleanUpJob(field, user); + sendToast(user, "Starting job", "Starting cleanUpFieldReferencesJob for field '" + field.name() + "'"); + final SystemMessageEventUtil messageEventUtil = SystemMessageEventUtil.getInstance(); + sendLegacyNotification(user,messageEventUtil, "Starting cleanUpFieldReferencesJob for field '" + field.name() + "'"); + localSystemEventsAPI.notify(new FieldDeletedEvent(field)); - } + } /** * Given a field load and return its variables. @@ -803,7 +807,7 @@ public List loadVariables(final Field field) throws DotDataExcept * @throws DotDataException */ private void removeRelationshipLink(final Field field, final ContentType type, - final ContentTypeAPI contentTypeAPI) + final ContentTypeAPI contentTypeAPI) throws DotDataException, DotSecurityException { final Optional result = relationshipAPI @@ -865,55 +869,55 @@ private void unlinkChild(final Relationship relationship) { } - @CloseDBIfOpened - @Override - public List byContentTypeId(final String typeId) throws DotDataException { - return fieldFactory.byContentTypeId(typeId); - } + @CloseDBIfOpened + @Override + public List byContentTypeId(final String typeId) throws DotDataException { + return fieldFactory.byContentTypeId(typeId); + } - @CloseDBIfOpened - @Override - public String nextAvailableColumn(final Field field) throws DotDataException{ - return fieldFactory.nextAvailableColumn(field); - } + @CloseDBIfOpened + @Override + public String nextAvailableColumn(final Field field) throws DotDataException{ + return fieldFactory.nextAvailableColumn(field); + } - @CloseDBIfOpened - @Override - public Field find(final String id) throws DotDataException { - return fieldFactory.byId(id); - } + @CloseDBIfOpened + @Override + public Field find(final String id) throws DotDataException { + return fieldFactory.byId(id); + } - @CloseDBIfOpened - @Override - public Field byContentTypeAndVar(final ContentType type, final String fieldVar) throws DotDataException { - return fieldFactory.byContentTypeFieldVar(type, fieldVar); - } + @CloseDBIfOpened + @Override + public Field byContentTypeAndVar(final ContentType type, final String fieldVar) throws DotDataException { + return fieldFactory.byContentTypeFieldVar(type, fieldVar); + } @CloseDBIfOpened @Override public Optional byContentTypeAndFieldRelationType(final String id, - final String fieldRelationType) throws DotDataException { + final String fieldRelationType) throws DotDataException { return fieldFactory.byContentTypeIdFieldRelationTypeInDb(id, fieldRelationType); } - @CloseDBIfOpened - @Override - public Field byContentTypeIdAndVar(final String id, final String fieldVar) throws DotDataException { - try { - return byContentTypeAndVar(APILocator.getContentTypeAPI(APILocator.systemUser()).find(id), fieldVar); - } catch (DotSecurityException e) { - throw new DotDataException(e); + @CloseDBIfOpened + @Override + public Field byContentTypeIdAndVar(final String id, final String fieldVar) throws DotDataException { + try { + return byContentTypeAndVar(APILocator.getContentTypeAPI(APILocator.systemUser()).find(id), fieldVar); + } catch (DotSecurityException e) { + throw new DotDataException(e); + } } - } - @WrapInTransaction - @Override - public void deleteFieldsByContentType(final ContentType type) throws DotDataException { - final List fields = byContentTypeId(type.id()); - for (Field field : fields) { - delete(field); - } - } + @WrapInTransaction + @Override + public void deleteFieldsByContentType(final ContentType type) throws DotDataException { + final List fields = byContentTypeId(type.id()); + for (Field field : fields) { + delete(field); + } + } @Override public List> fieldTypes() { diff --git a/dotCMS/src/main/java/com/dotmarketing/quartz/QuartzUtils.java b/dotCMS/src/main/java/com/dotmarketing/quartz/QuartzUtils.java index 812a31586e7b..2509dffaa1a1 100644 --- a/dotCMS/src/main/java/com/dotmarketing/quartz/QuartzUtils.java +++ b/dotCMS/src/main/java/com/dotmarketing/quartz/QuartzUtils.java @@ -10,6 +10,7 @@ import java.util.Map; import com.dotcms.business.WrapInTransaction; +import com.dotcms.contenttype.model.field.Field; import com.dotmarketing.common.db.DotConnect; import com.dotmarketing.exception.DotDataException; import org.quartz.CronTrigger; @@ -25,26 +26,26 @@ import com.dotmarketing.util.WebKeys; /** - * + * * This utility class let you schedule and check scheduled task through the two configured dotCMS schedulers: * - The standard scheduler * That let you run multiple task in parallel * - The sequential scheduler * That let you run only one task at a time, the order of the execution of the tasks is managed by quartz priorities - * + * * @author David Torres * */ public class QuartzUtils { - - + + private static final Map runtimeTaskValues = new HashMap<>(); - + /** - * + * * Lists all jobs scheduled through the standard scheduler, the standard scheduler * let you run multiple jobs in parallel * @@ -70,13 +71,13 @@ public static Scheduler getScheduler() { } } - - - + + + /** - * + * * Lists all jobs scheduled through the standard scheduler that belong to the given group - * + * * @return */ public static List getScheduledTasks(final String group) throws SchedulerException { @@ -96,7 +97,7 @@ public static List getScheduledTasks(final String group) throws S */ @SuppressWarnings("unchecked") private static List getScheduledTasks(final Scheduler scheduler, final boolean sequential, final String group) throws SchedulerException { - + final List result = new ArrayList<>(100); final String[] groupNames = scheduler.getJobGroupNames(); @@ -104,10 +105,10 @@ private static List getScheduledTasks(final Scheduler scheduler, JobDetail jobDetail; for (final String groupName : groupNames) { - + if(group != null && !groupName.equals(group)) continue; - + jobNames = scheduler.getJobNames(groupName); for (final String jobName : jobNames) { @@ -144,14 +145,14 @@ private static List getScheduledTasks(final Scheduler scheduler, return result; } - + /** * This methods checks all jobs configured either in the standard or the sequential schedulers that match * the given jobName and jobGroup - * + * * Even if the job is only configured in a single scheduler this method could return a list with multiple instances * because the same job could be configured with multiple triggers. - * + * * @param jobName * @param jobGroup * @return @@ -161,14 +162,14 @@ public static List getScheduledTask(final String jobName, final S final Scheduler scheduler = getScheduler(); return getScheduledTask(jobName, jobGroup, scheduler, false); } - + /** * This methods checks all jobs configured either in the standard scheduler that match * the given jobName and jobGroup - * + * * This method could return a list with multiple instances * because the same job could be configured with multiple triggers. - * + * * @param jobName * @param jobGroup * @return @@ -225,11 +226,11 @@ private static List getScheduledTask(final String jobName, final return result; } - + /** * Tells you whether the given jobName and jobGroup is set in the jobs DB to be executed or was executed and set with durability true * so still exists in the DB - * + * * @param jobName * @param jobGroup * @return @@ -238,13 +239,13 @@ private static List getScheduledTask(final String jobName, final public static boolean isJobScheduled(final String jobName, final String jobGroup) throws SchedulerException { final Scheduler scheduler = getScheduler(); return isJobScheduled(jobName, jobGroup, scheduler); - + } /** * Tells you whether the given jobName and jobGroup is set in the jobs DB to be sequentially executed or was executed and set with durability true * so still exists in the DB - * + * * @param jobName * @param jobGroup * @return @@ -268,12 +269,12 @@ public static boolean isJobSequentiallyScheduled(final String jobName, final Str */ private static boolean isJobScheduled(final String jobName, final String jobGroup, final Scheduler scheduler) throws SchedulerException { return scheduler.getJobDetail(jobName, jobGroup) != null; - + } - + /** * Returns the current task progress, it returns -1 if no task progress is found - * + * * @param jobName * @param jobGroup * @return @@ -299,7 +300,7 @@ public static void updateTaskProgress(final String jobName, final String jobGrou /** * Returns a task starting point of progress, by default 0 unless the task set it to something different - * + * * @param jobName * @param jobGroup * @return @@ -322,11 +323,11 @@ public static void setTaskStartProgress(final String jobName, final String jobGr if(runtimeValues == null) return; runtimeValues.startProgress = startProgress; } - + /** * Returns a task ending point to track progress, by default 100 unless the task set it to something different - * + * * @param jobName * @param jobGroup * @return @@ -346,7 +347,7 @@ public static int getTaskEndProgress(final String jobName, final String jobGroup */ public static TaskRuntimeValues getTaskRuntimeValues(final String jobName, final String jobGroup) { - + return runtimeTaskValues.get(jobName + "-" + jobGroup); } @@ -371,26 +372,26 @@ public static void setTaskEndProgress(final String jobName, final String jobGrou TaskRuntimeValues runtimeValues = runtimeTaskValues.get(jobName + "-" + jobGroup); if(runtimeValues == null) return; runtimeValues.endProgress = endProgress; - + } /** * Shuts down all schedulers - * @throws SchedulerException + * @throws SchedulerException */ public static void stopSchedulers() throws SchedulerException { Collection list= DotSchedulerFactory.getInstance().getAllSchedulers(); for (Scheduler s:list) { s.shutdown(); } - - - + + + } - + /** * Returns the current task progress, it returns -1 if no task progress is found - * + * * @param jobName * @param jobGroup * @return @@ -421,7 +422,7 @@ public static void addTaskMessage(final String jobName, final String jobGroup,fi */ public static void initializeTaskRuntimeValues(final String jobName, final String jobGroup) { runtimeTaskValues.put(jobName + "-" + jobGroup, new TaskRuntimeValues()); - + } /** @@ -431,18 +432,18 @@ public static void initializeTaskRuntimeValues(final String jobName, final Strin */ public static void removeTaskRuntimeValues(final String jobName, final String jobGroup) { runtimeTaskValues.remove(jobName + "-" + jobGroup); - } - + } + /** - * + * * This methods schedules the given job in the quartz system, and depending on the sequentialScheduled property * it will use the sequential of the standard scheduler - * + * * @param job * @return - * @throws SchedulerException - * @throws ParseException - * @throws ClassNotFoundException + * @throws SchedulerException + * @throws ParseException + * @throws ClassNotFoundException */ public static void scheduleTask(final ScheduledTask job) throws SchedulerException, ParseException, ClassNotFoundException { @@ -469,13 +470,13 @@ public static void scheduleTask(final ScheduledTask job) throws SchedulerExcepti } final JobDataMap dataMap = new JobDataMap(job.getProperties()); - + jobDetail.setDescription(job.getJobDescription()); jobDetail.setJobDataMap(dataMap); jobDetail.setDurability(job.getDurability()); - + if (job instanceof CronScheduledTask) { - trigger = new CronTrigger(triggerName, triggerGroup, jobName, jobGroup, startDate, endDate, ((CronScheduledTask) job).getCronExpression()); + trigger = new CronTrigger(triggerName, triggerGroup, jobName, jobGroup, startDate, endDate, ((CronScheduledTask) job).getCronExpression()); } else { trigger = new SimpleTrigger(triggerName, triggerGroup, jobName, jobGroup, startDate, endDate, ((SimpleScheduledTask) job).getRepeatCount(), ((SimpleScheduledTask) job).getRepeatInterval()); @@ -495,21 +496,21 @@ public static void scheduleTask(final ScheduledTask job) throws SchedulerExcepti scheduler.rescheduleJob(triggerName, triggerGroup, trigger); } } - + QuartzUtils.initializeTaskRuntimeValues(jobName, jobGroup); } /** - * + * * Removes the job and all associated triggers from both schedulers the standard and the sequential - * + * * @param jobName * @param jobGroup * @return true if the job is found and removed in at least one of the schedulers, false otherwise - * @throws SchedulerException - * + * @throws SchedulerException + * */ public static boolean removeJob(final String jobName, final String jobGroup) throws SchedulerException { final Scheduler scheduler = getScheduler(); @@ -521,14 +522,14 @@ public static boolean removeJob(final String jobName, final String jobGroup) thr } /** - * + * * Removes the job and all associated triggers from the standard jobs scheduler - * + * * @param jobName * @param jobGroup * @return true if the job is found and removed in at least one of the schedulers, false otherwise - * @throws SchedulerException - * + * @throws SchedulerException + * */ public static boolean removeStandardJob(final String jobName, final String jobGroup) throws SchedulerException { final Scheduler scheduler = getScheduler(); @@ -596,14 +597,14 @@ public static boolean deleteJobDB(final String jobName, final String jobGroup) t } /** - * + * * Removes the job and all associated triggers from the sequential jobs scheduler - * + * * @param jobName * @param jobGroup * @return true if the job is found and removed in at least one of the schedulers, false otherwise - * @throws SchedulerException - * + * @throws SchedulerException + * */ public static boolean removeSequentialJob(final String jobName, final String jobGroup) throws SchedulerException { final Scheduler scheduler = getScheduler(); @@ -621,13 +622,13 @@ public static boolean removeSequentialJob(final String jobName, final String job private static boolean removeJob(final String jobName, final String jobGroup, final Scheduler scheduler) throws SchedulerException { return scheduler.deleteJob(jobName, jobGroup); } - + /** * Pauses a job and all it associated triggers from all schedulers * @param jobName * @param jobGroup * @return - * @throws SchedulerException + * @throws SchedulerException */ public static void pauseJob(final String jobName, final String jobGroup) throws SchedulerException { final Scheduler scheduler = getScheduler(); @@ -635,27 +636,27 @@ public static void pauseJob(final String jobName, final String jobGroup) throws } - + /** * Pauses a job and all it associated triggers from the standard schedulers * @param jobName * @param jobGroup * @return - * @throws SchedulerException + * @throws SchedulerException */ private static void pauseJob(final String jobName, final String jobGroup, final Scheduler scheduler) throws SchedulerException { scheduler.pauseJob(jobName, jobGroup); } - - + + /** * Pauses a job and all it associated triggers from all schedulers * @param jobName * @param jobGroup * @return - * @throws SchedulerException + * @throws SchedulerException */ public static void resumeJob(String jobName, String jobGroup) throws SchedulerException { final Scheduler scheduler = getScheduler(); @@ -673,13 +674,13 @@ public static void resumeJob(String jobName, String jobGroup) throws SchedulerEx private static void resumeJob(final String jobName, final String jobGroup, final Scheduler scheduler) throws SchedulerException { scheduler.resumeJob(jobName, jobGroup); } - + /** * Pauses a trigger from all schedulers * @param triggerName * @param triggerGroup * @return - * @throws SchedulerException + * @throws SchedulerException */ public static void pauseTrigger(final String triggerName, final String triggerGroup) throws SchedulerException { final Scheduler scheduler = getScheduler(); @@ -708,13 +709,13 @@ private static void pauseTrigger(final String triggerName, final String triggerG public static Trigger getTrigger(final String triggerName, final String triggerGroup) throws SchedulerException { return getScheduler() .getTrigger(triggerName, triggerGroup); } - + /** * Resumes a trigger from all schedulers * @param triggerName * @param triggerGroup * @return - * @throws SchedulerException + * @throws SchedulerException */ public static void resumeTrigger(final String triggerName, final String triggerGroup) throws SchedulerException { final Scheduler scheduler = getScheduler(); @@ -733,20 +734,20 @@ public static void resumeTrigger(final String triggerName, final String triggerG private static void resumeTrigger(final String triggerName, final String triggerGroup, final Scheduler scheduler) throws SchedulerException { scheduler.resumeTrigger(triggerName, triggerGroup); } - + /** * Temporarily pauses all schedulers from executing future triggers - * @throws SchedulerException + * @throws SchedulerException */ public static void pauseSchedulers () throws SchedulerException { final Scheduler scheduler = getScheduler(); scheduler.standby(); } - + /** * Temporarily pauses all schedulers from executing future triggers - * @throws SchedulerException + * @throws SchedulerException */ public static void pauseStandardSchedulers () throws SchedulerException { final Scheduler scheduler = getScheduler(); @@ -819,69 +820,106 @@ public static boolean isJobRunning(final Scheduler scheduler, return false; } } - + /** * job comparision util * @param job1 * @param job2 * @return */ - private static boolean isSameJob(JobDetail job1, JobDetail job2){ - - - - - - + public static boolean isSameJob(JobDetail job1, JobDetail job2){ + + + + + + try{ Map m1 = job1.getJobDataMap(); Map m2 = job2.getJobDataMap(); - + for(String key : m1.keySet()){ if(m2.get(key) == null ){ return false; - + } else if (m1.get(key) instanceof String[] && m2.get(key) instanceof String[]){ - + String[] x = (String[]) m1.get(key); String[] y = (String[]) m2.get(key); Arrays.sort(x); Arrays.sort(y); - - - + + + for(int i=0;i currentlyExecutingJobs = new ArrayList<>(); + currentlyExecutingJobs.addAll(QuartzUtils.getScheduler().getCurrentlyExecutingJobs()); + + JobDetail existingJobDetail = QuartzUtils.getScheduler().getJobDetail("CleanUpFieldReferencesJob", "CleanUpFieldReferencesJob_Group"); + + if (existingJobDetail != null) { + for (JobExecutionContext jec : currentlyExecutingJobs) { + + final JobDetail runningJobDetail = jec.getJobDetail(); + //if there is a CleanUpFieldReferencesJob running + if (existingJobDetail.equals(runningJobDetail) || isSameJob(existingJobDetail, runningJobDetail)) { + JobDataMap jobDataMap = runningJobDetail.getJobDataMap(); + Map> jobDetail = (Map>) jobDataMap.get("trigger_job_detail"); + //checks on the running jobs if there is any job with a field with the same name + boolean doesFieldExists = jobDetail.values() + .stream() + .filter(value -> value.get("field") != null) + .map(filteredField -> ((Field) filteredField.get("field")).name()) + .anyMatch(fieldName -> fieldName.equals(field.name())); + + if (doesFieldExists) { + throw new DotDataException("Field variable '" + field.name() + "' cannot be recreated while the CleanUpFieldReferencesJob is running. Please wait until finish"); + } + } + } + } + } catch (SchedulerException e){ + throw new DotDataException(e); + } + } + + } diff --git a/dotCMS/src/main/java/com/dotmarketing/quartz/job/CleanUpFieldReferencesJob.java b/dotCMS/src/main/java/com/dotmarketing/quartz/job/CleanUpFieldReferencesJob.java index 7994b1982f98..64119b301c90 100644 --- a/dotCMS/src/main/java/com/dotmarketing/quartz/job/CleanUpFieldReferencesJob.java +++ b/dotCMS/src/main/java/com/dotmarketing/quartz/job/CleanUpFieldReferencesJob.java @@ -1,5 +1,6 @@ package com.dotmarketing.quartz.job; +import com.dotcms.api.system.event.message.SystemMessageEventUtil; import com.dotcms.business.WrapInTransaction; import com.dotcms.contenttype.business.ContentTypeAPI; import com.dotcms.contenttype.exception.NotFoundInDbException; @@ -53,10 +54,10 @@ public void run(final JobExecutionContext jobContext) throws JobExecutionExcepti final JobDataMap jobDataMap = jobContext.getJobDetail().getJobDataMap(); if (jobDataMap.containsKey(EXECUTION_DATA)) { - //This bit is here to continue to support the integration-tests + //This bit is here to continue to support the integration-tests executionData = (Map) jobDataMap.get(EXECUTION_DATA); } else { - //But the `executionData` must be grabbed frm the persisted job detail. Through the trigger name. + //But the `executionData` must be grabbed frm the persisted job detail. Through the trigger name. final Trigger trigger = jobContext.getTrigger(); executionData = getExecutionData(trigger, CleanUpFieldReferencesJob.class); } @@ -103,6 +104,7 @@ public void run(final JobExecutionContext jobContext) throws JobExecutionExcepti Logger.error(CleanUpFieldReferencesJob.class, "Error cleaning up field references. Field velocity var: " + field.variable(), e); } + SystemMessageEventUtil.getInstance().pushSimpleTextEvent("CleanUpFieldReferencesJob finished for field: " + field.variable(), user.getUserId()); Logger.info(CleanUpFieldReferencesJob.class,String.format("CleanUpFieldReferencesJob ::: finished for field `%s`.",field.variable())); } diff --git a/dotcms-integration/src/test/java/com/dotcms/contenttype/business/FieldAPIImplIntegrationTest.java b/dotcms-integration/src/test/java/com/dotcms/contenttype/business/FieldAPIImplIntegrationTest.java index c8d92c2d9d7e..15c011fe3b67 100644 --- a/dotcms-integration/src/test/java/com/dotcms/contenttype/business/FieldAPIImplIntegrationTest.java +++ b/dotcms-integration/src/test/java/com/dotcms/contenttype/business/FieldAPIImplIntegrationTest.java @@ -15,10 +15,15 @@ import com.dotmarketing.exception.DotDataException; import com.dotmarketing.exception.DotSecurityException; import com.dotmarketing.portlets.languagesmanager.model.Language; +import com.dotmarketing.quartz.QuartzUtils; +import com.dotmarketing.quartz.job.CleanUpFieldReferencesJob; import com.liferay.portal.model.User; +import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Test; +import org.junit.jupiter.api.Assertions; import org.junit.runner.RunWith; +import org.quartz.SchedulerException; import javax.enterprise.context.ApplicationScoped; import java.util.List; @@ -26,8 +31,7 @@ import java.util.stream.Collectors; import static com.dotcms.util.CollectionsUtils.list; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; @ApplicationScoped @RunWith(DataProviderWeldRunner.class) @@ -79,7 +83,7 @@ public void shouldSaveFields() throws DotDataException, DotSecurityException { final Field column = new FieldDataGen() .name("fields-1"). - type(ColumnField.class) + type(ColumnField.class) .sortOrder(1) .contentTypeId(contentType.id()) .velocityVarName("fields1" + current) @@ -236,6 +240,70 @@ public void cleanUpUniqueFieldTableAfterDeleteField() throws DotDataException { } } + /** + * Method to test: {@link FieldAPIImpl#delete(Field, User)} + * Given Scenario: Big amount of contents, delete a field of the contents type to trigger CleanUpFieldReferencesJob, and while it is running try to create a new field with the same name + * ExpectedResult: Throw an exception indicating that the field cannot be created while the job is still running + * + */ + @Test + public void createSameFieldNameWhileCleaningUpField(){ + try { + final ContentType contentType = new ContentTypeDataGen() + .nextPersisted(); + final Language language = new LanguageDataGen().nextPersisted(); + + final Field field = new FieldDataGen() + .name("creationDate") + .velocityVarName("creationDate") + .contentTypeId(contentType.id()) + .type(TextField.class) + .nextPersisted(); + + final Host host = new SiteDataGen().nextPersisted(); + + final long langId = language.getId(); + + for (int i = 0; i < 1000; i++) { + new ContentletDataGen(contentType) + .host(host) + .languageId(langId) + .nextPersisted(); + } + + + Thread deleteThread = new Thread(() -> { + try { + APILocator.getContentTypeFieldAPI().delete(field); + QuartzUtils.startSchedulers(); + } catch (SchedulerException | DotDataException e) { + throw new RuntimeException(e); + } + }); + + deleteThread.start(); + + Thread.sleep(500); + + + Exception e = Assertions.assertThrows(Exception.class, () -> { + Field newField = new FieldDataGen() + .name("creationDate") + .velocityVarName("creationDate") + .contentTypeId(contentType.id()) + .type(TextField.class) + .next(); + APILocator.getContentTypeFieldAPI().save(newField, APILocator.systemUser(), false); + }); + + Assert.assertEquals("Field variable 'creationDate' cannot be recreated while the CleanUpFieldReferencesJob is running. Please wait until finish", e.getMessage()); + + deleteThread.join(); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + } + private static void checkExtraTableCount(final ContentType contentType, final int countExpected) throws DotDataException { final List> results = new DotConnect().setSQL("SELECT * FROM unique_fields WHERE supporting_values->>'contentTypeId' = ?")