Skip to content

Commit 3cb5951

Browse files
committed
HDDS-14377. refactor OmLifeCycleConfiguration to align builder pattern in WithObjectID
1 parent 8ee502a commit 3cb5951

File tree

5 files changed

+66
-17
lines changed

5 files changed

+66
-17
lines changed

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmLifecycleConfiguration.java

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -312,14 +312,52 @@ public Builder setUpdateID(long uID) {
312312
}
313313

314314
@Override
315-
public OmLifecycleConfiguration buildObject() {
316-
return new OmLifecycleConfiguration(this);
315+
protected void validate() {
316+
super.validate();
317+
try {
318+
if (StringUtils.isBlank(volume)) {
319+
throw new OMException("Invalid lifecycle configuration: Volume cannot be blank.",
320+
OMException.ResultCodes.INVALID_REQUEST);
321+
}
322+
323+
if (StringUtils.isBlank(bucket)) {
324+
throw new OMException("Invalid lifecycle configuration: Bucket cannot be blank.",
325+
OMException.ResultCodes.INVALID_REQUEST);
326+
}
327+
328+
if (rules.isEmpty()) {
329+
throw new OMException("At least one rules needs to be specified in a lifecycle configuration.",
330+
OMException.ResultCodes.INVALID_REQUEST);
331+
}
332+
333+
if (rules.size() > LC_MAX_RULES) {
334+
throw new OMException("The number of lifecycle rules must not exceed the allowed limit of "
335+
+ LC_MAX_RULES + " rules", OMException.ResultCodes.INVALID_REQUEST);
336+
}
337+
338+
if (!hasNoDuplicateID()) {
339+
throw new OMException("Invalid lifecycle configuration: Duplicate rule IDs found.",
340+
OMException.ResultCodes.INVALID_REQUEST);
341+
}
342+
343+
for (OmLCRule rule : rules) {
344+
rule.valid(bucketLayout, creationTime);
345+
}
346+
} catch (OMException e) {
347+
throw new IllegalArgumentException(e.getMessage(), e);
348+
}
349+
}
350+
351+
private boolean hasNoDuplicateID() {
352+
return rules.size() == rules.stream()
353+
.map(OmLCRule::getId)
354+
.collect(Collectors.toSet())
355+
.size();
317356
}
318357

319-
public OmLifecycleConfiguration buildAndValid() throws OMException {
320-
OmLifecycleConfiguration omLifecycleConfiguration = buildObject();
321-
omLifecycleConfiguration.valid();
322-
return omLifecycleConfiguration;
358+
@Override
359+
protected OmLifecycleConfiguration buildObject() {
360+
return new OmLifecycleConfiguration(this);
323361
}
324362
}
325363
}

hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/OMLCUtils.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,16 @@ public final class OMLCUtils {
4747

4848
public static void assertOMException(Executable action, OMException.ResultCodes expectedResultCode,
4949
String expectedMessageContent) {
50-
OMException e = assertThrows(OMException.class, action);
50+
Exception thrown = assertThrows(Exception.class, action);
51+
OMException e;
52+
if (thrown instanceof OMException) {
53+
e = (OMException) thrown;
54+
} else if (thrown instanceof IllegalArgumentException
55+
&& thrown.getCause() instanceof OMException) {
56+
e = (OMException) thrown.getCause();
57+
} else {
58+
throw new AssertionError("Expected OMException but got: " + thrown.getClass().getName(), thrown);
59+
}
5160
assertEquals(expectedResultCode, e.getResult());
5261
assertTrue(e.getMessage().contains(expectedMessageContent),
5362
"Expected: " + expectedMessageContent + "\n Actual: " + e.getMessage());

hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmLCRule.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ public void testDuplicateRuleIDs() throws OMException {
185185
.setBucket("bucket")
186186
.setRules(rules);
187187

188-
assertOMException(() -> config.buildAndValid(), INVALID_REQUEST, "Duplicate rule IDs found");
188+
assertOMException(config::build, INVALID_REQUEST, "Duplicate rule IDs found");
189189
}
190190

191191
@Test

hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmLifeCycleConfiguration.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,14 +73,14 @@ public void testCreateInValidLCConfiguration() throws OMException {
7373
List<OmLCRule> rules = Collections.singletonList(rule);
7474

7575
OmLifecycleConfiguration.Builder lcc0 = getOmLifecycleConfiguration(null, "bucket", rules);
76-
assertOMException(() -> lcc0.buildAndValid(), INVALID_REQUEST, "Volume cannot be blank");
76+
assertOMException(lcc0::build, INVALID_REQUEST, "Volume cannot be blank");
7777

7878
OmLifecycleConfiguration.Builder lcc1 = getOmLifecycleConfiguration("volume", null, rules);
79-
assertOMException(() -> lcc1.buildAndValid(), INVALID_REQUEST, "Bucket cannot be blank");
79+
assertOMException(lcc1::build, INVALID_REQUEST, "Bucket cannot be blank");
8080

8181
OmLifecycleConfiguration.Builder lcc3 = getOmLifecycleConfiguration(
8282
"volume", "bucket", Collections.emptyList());
83-
assertOMException(() -> lcc3.buildAndValid(), INVALID_REQUEST,
83+
assertOMException(lcc3::build, INVALID_REQUEST,
8484
"At least one rules needs to be specified in a lifecycle configuration");
8585

8686
List<OmLCRule> rules4 = new ArrayList<>(
@@ -94,7 +94,7 @@ public void testCreateInValidLCConfiguration() throws OMException {
9494
rules4.add(r);
9595
}
9696
OmLifecycleConfiguration.Builder lcc4 = getOmLifecycleConfiguration("volume", "bucket", rules4);
97-
assertOMException(() -> lcc4.buildAndValid(), INVALID_REQUEST,
97+
assertOMException(lcc4::build, INVALID_REQUEST,
9898
"The number of lifecycle rules must not exceed the allowed limit of");
9999
}
100100

hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/S3LifecycleConfiguration.java

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -264,12 +264,14 @@ public OmLifecycleConfiguration toOmLifecycleConfiguration(OzoneBucket ozoneBuck
264264
builder.addRule(convertToOmRule(rule));
265265
}
266266

267-
return builder.buildAndValid();
268-
} catch (Exception ex) {
269-
if (ex instanceof IllegalStateException) {
270-
throw S3ErrorTable.newError(S3ErrorTable.INVALID_REQUEST, ozoneBucket.getName(), ex);
267+
return builder.build();
268+
} catch (IllegalArgumentException ex) {
269+
if (ex.getCause() instanceof OMException) {
270+
throw (OMException) ex.getCause();
271271
}
272-
throw ex;
272+
throw S3ErrorTable.newError(S3ErrorTable.INVALID_REQUEST, ozoneBucket.getName(), ex);
273+
} catch (IllegalStateException ex) {
274+
throw S3ErrorTable.newError(S3ErrorTable.INVALID_REQUEST, ozoneBucket.getName(), ex);
273275
}
274276
}
275277

0 commit comments

Comments
 (0)