Skip to content

Commit 54f7c0d

Browse files
fabrizzio-dotCMSfabrizzio-dotCMS
authored
fix(Content Import) Unique Field Better Messages + adjustments in general Refs: dotCMS#32033 (dotCMS#33014)
### Proposed Changes * Removing unnecessary fields that were added to the context of some exceptions * Changes in wording and messages sent by our Import API * Adjustments in the ImportAPITest accordingly * UniqueFieldValueDuplicatedException extended to have the additional payload required by the ImportAPI
1 parent e764479 commit 54f7c0d

File tree

13 files changed

+470
-153
lines changed

13 files changed

+470
-153
lines changed

‎dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/field/FieldHandlerStrategyFactory.java‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,6 @@ else if (value instanceof String && APILocator.getTempFileAPI().isTempResource((
241241
tempFileOptional.get().file);
242242
} else {
243243
throw DotBinaryFieldException.invalidTempFileBuilder(field.variable(), value)
244-
.fieldName(field.name())
245244
.fieldType(field.typeName())
246245
.expectedFormat("Valid temporary file")
247246
.addContext("tempFileResource", value.toString())
@@ -251,7 +250,6 @@ else if (value instanceof String && APILocator.getTempFileAPI().isTempResource((
251250
}
252251
} catch (IOException e) {
253252
throw DotBinaryFieldException.ioErrorBuilder(field.variable(), value)
254-
.fieldName(field.name())
255253
.fieldType(field.typeName())
256254
.expectedFormat("File or temporary file resource")
257255
.addContext("providedType", value != null ? value.getClass().getSimpleName() : "null")
@@ -308,7 +306,6 @@ public void parseDate(final Contentlet contentlet,
308306
} catch (Exception e) {
309307

310308
throw DotDateFieldException.conversionErrorBuilder(field.variable(), value)
311-
.fieldName(field.name())
312309
.fieldType(field.typeName())
313310
.acceptedFormats(dateFormats)
314311
.addContext("errorMessage", e.getMessage())
@@ -321,7 +318,6 @@ public void parseDate(final Contentlet contentlet,
321318
} else if (field.required() && value == null) {
322319

323320
throw DotDateFieldException.invalidTypeBuilder(field.variable(), value)
324-
.fieldName(field.name())
325321
.fieldType(field.typeName())
326322
.expectedFormat("String or Date")
327323
.addContext("providedType", value != null ? value.getClass().getSimpleName() : "null")
@@ -368,7 +364,6 @@ private void floatStrategy(final Contentlet contentlet, final Field field, final
368364
contentlet.getMap().put(field.variable(), stringValue);
369365
}
370366
throw DotNumericFieldException.floatFieldBuilder(field.variable(), value)
371-
.fieldName(field.name())
372367
.fieldType(field.typeName())
373368
.build();
374369
}
@@ -399,7 +394,6 @@ private void integerStrategy(final Contentlet contentlet, final Field field, fin
399394
} catch (Exception e) {
400395
//If we throw this exception here... the contentlet will never get to the validateContentlet Method
401396
throw DotNumericFieldException.longFieldBuilder(field.variable(), value)
402-
.fieldName(field.name())
403397
.fieldType(field.typeName())
404398
.build();
405399
}

‎dotCMS/src/main/java/com/dotcms/contenttype/business/UniqueFieldValueDuplicatedException.java‎

Lines changed: 129 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,149 @@
11
package com.dotcms.contenttype.business;
22

3+
import static com.dotmarketing.util.FieldNameUtils.convertFieldClassName;
4+
5+
import com.dotmarketing.util.FieldNameUtils;
6+
import com.dotmarketing.util.importer.ImportLineValidationCodes;
7+
import com.dotmarketing.util.importer.exception.ImportLineError;
8+
import java.util.HashMap;
39
import java.util.List;
10+
import java.util.Map;
11+
import java.util.Optional;
412

513
/**
6-
* Throw if try to insert a duplicated register in unique_fiedls table
14+
* Throw if try to insert a duplicated register in unique_fields table
715
*/
8-
public class UniqueFieldValueDuplicatedException extends Exception{
16+
public class UniqueFieldValueDuplicatedException extends Exception implements ImportLineError {
17+
918

10-
private List<String> contentletsIDS;
19+
private final List<String> contentletsIDS;
20+
private final String field;
21+
private final String value;
22+
private final String fieldType;
23+
private final String contentType;
24+
private final Map<String, String> additionalContext;
1125

1226
public UniqueFieldValueDuplicatedException(String message) {
13-
super(message);
27+
this(message, null);
1428
}
1529

1630
public UniqueFieldValueDuplicatedException(String message, List<String> contentletsIDS) {
1731
super(message);
1832
this.contentletsIDS = contentletsIDS;
33+
this.field = null;
34+
this.value = null;
35+
this.fieldType = null;
36+
this.contentType = null;
37+
this.additionalContext = new HashMap<>();
38+
}
39+
40+
/**
41+
* Enhanced constructor for Builder pattern
42+
*/
43+
private UniqueFieldValueDuplicatedException(String message, List<String> contentletsIDS,
44+
String field, Object value,
45+
String fieldType, String contentType,
46+
Map<String, String> additionalContext) {
47+
super(message);
48+
this.contentletsIDS = contentletsIDS;
49+
this.field = field;
50+
this.value = value == null ? "" : value.toString();
51+
this.fieldType = fieldType;
52+
this.contentType = contentType;
53+
this.additionalContext = new HashMap<>(additionalContext);
1954
}
2055

2156
public List<String> getContentlets() {
2257
return contentletsIDS;
2358
}
59+
60+
@Override
61+
public Optional<String> getField() {
62+
return Optional.ofNullable(field);
63+
}
64+
65+
@Override
66+
public Optional<String> getValue() {
67+
return Optional.ofNullable(value);
68+
}
69+
70+
@Override
71+
public String getCode() {
72+
return ImportLineValidationCodes.DUPLICATE_UNIQUE_VALUE.name();
73+
}
74+
75+
@Override
76+
public Optional<Map<String, ?>> getContext() {
77+
Map<String, String> context = new HashMap<>(additionalContext);
78+
if (fieldType != null) {
79+
context.put("fieldType", convertFieldClassName(fieldType));
80+
}
81+
if (contentType != null) {
82+
context.put("contentType", contentType);
83+
}
84+
if (contentletsIDS != null && !contentletsIDS.isEmpty()) {
85+
context.put("duplicatedContentlets", String.join(", ", contentletsIDS));
86+
}
87+
return context.isEmpty() ? Optional.empty() : Optional.of(context);
88+
}
89+
90+
/**
91+
* Builder for creating UniqueFieldValueDuplicatedException with enhanced context information
92+
*/
93+
public static class Builder {
94+
private final String message;
95+
private final String field;
96+
private final Object value;
97+
private final String contentType;
98+
private String fieldType;
99+
private List<String> contentletsIDS;
100+
private final Map<String, String> additionalContext = new HashMap<>();
101+
102+
private Builder(String message, String field, Object value, String contentType) {
103+
this.message = message;
104+
this.field = field;
105+
this.value = value;
106+
this.contentType = contentType;
107+
}
108+
109+
/**
110+
* Set the type of the field (e.g., "text", "textarea", etc.)
111+
*/
112+
public Builder fieldType(String fieldType) {
113+
this.fieldType = fieldType;
114+
return this;
115+
}
116+
117+
/**
118+
* Set the list of contentlet IDs that have the duplicated value
119+
*/
120+
public Builder contentletIds(List<String> contentletIds) {
121+
this.contentletsIDS = contentletIds;
122+
return this;
123+
}
124+
125+
/**
126+
* Add additional context information
127+
*/
128+
public Builder addContext(String key, String value) {
129+
this.additionalContext.put(key, value);
130+
return this;
131+
}
132+
133+
/**
134+
* Build the exception with enhanced context
135+
*/
136+
public UniqueFieldValueDuplicatedException build() {
137+
return new UniqueFieldValueDuplicatedException(message, contentletsIDS, field, value,
138+
fieldType, contentType, additionalContext);
139+
}
140+
}
141+
142+
/**
143+
* Create a Builder with a pre-computed message
144+
*/
145+
public static Builder builder(String message, String field, Object value, String contentType) {
146+
return new Builder(message, field, value, contentType);
147+
}
148+
24149
}

‎dotCMS/src/main/java/com/dotcms/contenttype/business/uniquefields/ESUniqueFieldValidationStrategy.java‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import com.dotcms.content.elasticsearch.util.ESUtils;
44
import com.dotcms.contenttype.business.UniqueFieldValueDuplicatedException;
5+
import com.dotcms.contenttype.transform.field.LegacyFieldTransformer;
56
import com.dotmarketing.portlets.contentlet.model.Contentlet;
67
import com.dotcms.contenttype.model.field.DataTypes;
78
import com.dotcms.contenttype.model.field.Field;
@@ -84,7 +85,9 @@ public void innerValidate(final Contentlet contentlet, final Field uniqueField,
8485
"unique content Inode '%s' was not found. ES Index might need to be reindexed.",
8586
uniqueField.variable(), contentletSearch.getInode());
8687
Logger.warn(this, errorMsg);
87-
throw new DotContentletValidationException(errorMsg);
88+
throw DotContentletValidationException.builder(errorMsg)
89+
.addUniqueField(new LegacyFieldTransformer(uniqueField).asOldField(),
90+
null != fieldValue ? fieldValue.toString() : "N/A").build();
8891
}
8992

9093
final Map<String, Object> uniqueContentMap = uniqueContent.getMap();
@@ -110,16 +113,28 @@ public void innerValidate(final Contentlet contentlet, final Field uniqueField,
110113
final String duplicatedValueMessage = String.format("The value %s for the field %s in the Content type %s is duplicated",
111114
fieldValue, uniqueField.variable(), contentType.variable());
112115

113-
throw new UniqueFieldValueDuplicatedException(duplicatedValueMessage,
114-
contentlets.stream().map(ContentletSearch::getIdentifier).collect(Collectors.toList()));
116+
throw UniqueFieldValueDuplicatedException.builder(
117+
duplicatedValueMessage,
118+
uniqueField.variable(),
119+
fieldValue,
120+
contentType.variable())
121+
.fieldType(uniqueField.dataType().toString())
122+
.contentletIds(contentlets.stream().map(ContentletSearch::getIdentifier).collect(Collectors.toList()))
123+
.build();
115124
}
116125
}
117126
} else {
118127
final String duplicatedValueMessage = String.format("The value %s for the field %s in the Content type %s is duplicated",
119128
fieldValue, uniqueField.variable(), contentType.variable());
120129

121-
throw new UniqueFieldValueDuplicatedException(duplicatedValueMessage,
122-
contentlets.stream().map(ContentletSearch::getIdentifier).collect(Collectors.toList()));
130+
throw UniqueFieldValueDuplicatedException.builder(
131+
duplicatedValueMessage,
132+
uniqueField.variable(),
133+
fieldValue,
134+
contentType.variable())
135+
.fieldType(uniqueField.dataType().toString())
136+
.contentletIds(contentlets.stream().map(ContentletSearch::getIdentifier).collect(Collectors.toList()))
137+
.build();
123138
}
124139
}
125140
}
@@ -154,7 +169,6 @@ private List<ContentletSearch> getContentletFromES(Contentlet contentlet, Field
154169
.append(ESUtils.sha256(contentlet.getContentType().variable()
155170
+ StringPool.PERIOD + uniqueField.variable(), fieldValue,
156171
contentlet.getLanguageId()));
157-
158172
final List<ContentletSearch> contentlets = new ArrayList<>();
159173
try {
160174
contentlets.addAll(
@@ -175,6 +189,6 @@ private List<ContentletSearch> getContentletFromES(Contentlet contentlet, Field
175189

176190
private boolean getUniquePerSiteConfig(final com.dotcms.contenttype.model.field.Field field) {
177191
return field.fieldVariableValue(UNIQUE_PER_SITE_FIELD_VARIABLE_NAME)
178-
.map(value -> Boolean.valueOf(value)).orElse(false);
192+
.map(Boolean::valueOf).orElse(false);
179193
}
180194
}

‎dotCMS/src/main/java/com/dotcms/contenttype/business/uniquefields/extratable/DBUniqueFieldValidationStrategy.java‎

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
package com.dotcms.contenttype.business.uniquefields.extratable;
22

3+
import static com.dotcms.content.elasticsearch.business.ESContentletAPIImpl.UNIQUE_PER_SITE_FIELD_VARIABLE_NAME;
4+
import static com.dotcms.contenttype.business.uniquefields.extratable.UniqueFieldCriteria.CONTENTLET_IDS_ATTR;
5+
import static com.dotcms.contenttype.business.uniquefields.extratable.UniqueFieldCriteria.FIELD_VALUE_ATTR;
6+
import static com.dotcms.contenttype.business.uniquefields.extratable.UniqueFieldCriteria.UNIQUE_PER_SITE_ATTR;
7+
import static com.dotmarketing.util.Constants.DONT_RESPECT_FRONT_END_ROLES;
8+
import static com.liferay.util.StringPool.BLANK;
9+
310
import com.dotcms.business.CloseDBIfOpened;
411
import com.dotcms.business.WrapInTransaction;
512
import com.dotcms.contenttype.business.UniqueFieldValueDuplicatedException;
@@ -21,23 +28,15 @@
2128
import com.dotmarketing.util.UtilMethods;
2229
import com.google.common.annotations.VisibleForTesting;
2330
import com.liferay.portal.model.User;
24-
import org.postgresql.util.PGobject;
25-
26-
import javax.enterprise.context.ApplicationScoped;
27-
import javax.inject.Inject;
2831
import java.io.IOException;
2932
import java.util.HashMap;
3033
import java.util.List;
3134
import java.util.Map;
3235
import java.util.Optional;
3336
import java.util.stream.Collectors;
34-
35-
import static com.dotcms.content.elasticsearch.business.ESContentletAPIImpl.UNIQUE_PER_SITE_FIELD_VARIABLE_NAME;
36-
import static com.dotcms.contenttype.business.uniquefields.extratable.UniqueFieldCriteria.CONTENTLET_IDS_ATTR;
37-
import static com.dotcms.contenttype.business.uniquefields.extratable.UniqueFieldCriteria.FIELD_VALUE_ATTR;
38-
import static com.dotcms.contenttype.business.uniquefields.extratable.UniqueFieldCriteria.UNIQUE_PER_SITE_ATTR;
39-
import static com.dotmarketing.util.Constants.DONT_RESPECT_FRONT_END_ROLES;
40-
import static com.liferay.util.StringPool.BLANK;
37+
import javax.enterprise.context.ApplicationScoped;
38+
import javax.inject.Inject;
39+
import org.postgresql.util.PGobject;
4140

4241
/**
4342
* This implementation of the {@link UniqueFieldValidationStrategy} checks the uniqueness of a given
@@ -324,7 +323,14 @@ private static void throwsDuplicatedException(final UniqueFieldCriteria uniqueFi
324323
uniqueFieldCriteria.contentType().variable());
325324

326325
Logger.error(DBUniqueFieldValidationStrategy.class, duplicatedValueMessage);
327-
throw new UniqueFieldValueDuplicatedException(duplicatedValueMessage);
326+
327+
throw UniqueFieldValueDuplicatedException.builder(
328+
duplicatedValueMessage,
329+
uniqueFieldCriteria.field().variable(),
330+
uniqueFieldCriteria.value(),
331+
uniqueFieldCriteria.contentType().variable())
332+
.fieldType(uniqueFieldCriteria.field().typeName())
333+
.build();
328334
}
329335

330336
@WrapInTransaction
@@ -344,7 +350,13 @@ public void recalculate(final Field field, final boolean uniquePerSite) throws U
344350
"'%s': %s", field.variable(), field.contentTypeId(), ExceptionUtil.getErrorMessage(e));
345351
Logger.error(this, errorMsg, e);
346352
}
347-
throw new UniqueFieldValueDuplicatedException(errorMsg);
353+
throw UniqueFieldValueDuplicatedException.builder(
354+
errorMsg,
355+
field.variable(),
356+
"N/A",
357+
field.contentTypeId())
358+
.fieldType(field.typeName())
359+
.build();
348360
}
349361
}
350362

0 commit comments

Comments
 (0)