Skip to content

Commit 56e980e

Browse files
steveloughransteveloughran
committed
HADOOP-19161. style, javadocs and more equality tests
Change-Id: Iec42cb156abf0b8ee96bc507d17ce8a8506fd428
1 parent af2dd8d commit 56e980e

File tree

2 files changed

+96
-35
lines changed

2 files changed

+96
-35
lines changed

‎hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/impl/FlagSet.java‎

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,14 @@
1818

1919
package org.apache.hadoop.fs.impl;
2020

21+
import java.util.Arrays;
2122
import java.util.EnumSet;
2223
import java.util.Map;
2324
import java.util.Objects;
2425
import java.util.Set;
2526
import java.util.concurrent.atomic.AtomicBoolean;
2627
import java.util.stream.Collectors;
28+
import javax.annotation.Nullable;
2729

2830
import org.apache.hadoop.conf.Configuration;
2931
import org.apache.hadoop.fs.StreamCapabilities;
@@ -64,8 +66,13 @@ public final class FlagSet<E extends Enum<E>> implements StreamCapabilities {
6466
* @param prefix prefix (with trailing ".") for path capabilities probe
6567
* @param flags flags. A copy of these are made.
6668
*/
67-
private FlagSet(final Class<E> enumClass, final String prefix, final EnumSet<E> flags) {
68-
this.flags = EnumSet.copyOf(flags);
69+
private FlagSet(final Class<E> enumClass,
70+
final String prefix,
71+
@Nullable final EnumSet<E> flags) {
72+
73+
this.flags = flags != null
74+
? EnumSet.copyOf(flags)
75+
: EnumSet.noneOf(enumClass);
6976
this.namesToValues = mapEnumNamesToValues(prefix, enumClass);
7077
}
7178

@@ -164,13 +171,17 @@ public String toString() {
164171

165172
/**
166173
* Equality is based on the set.
167-
* @param o
168-
* @return
174+
* @param o other object
175+
* @return true iff the flags are equal.
169176
*/
170177
@Override
171178
public boolean equals(final Object o) {
172-
if (this == o) {return true;}
173-
if (o == null || getClass() != o.getClass()) {return false;}
179+
if (this == o) {
180+
return true;
181+
}
182+
if (o == null || getClass() != o.getClass()) {
183+
return false;
184+
}
174185
FlagSet<?> flagSet = (FlagSet<?>) o;
175186
return Objects.equals(flags, flagSet.flags);
176187
}
@@ -210,6 +221,23 @@ public static <E extends Enum<E>> FlagSet<E> createFlagSet(
210221
return new FlagSet<>(enumClass, prefix, flags);
211222
}
212223

224+
/**
225+
* Create a FlagSet from a list of enum values.
226+
* @param enumClass class of enum
227+
* @param prefix prefix (with trailing ".") for path capabilities probe
228+
* @param enabled varags list of flags to enable.
229+
* @param <E> enum type
230+
* @return a mutable FlagSet
231+
*/
232+
public static <E extends Enum<E>> FlagSet<E> createFlagSet(
233+
final Class<E> enumClass,
234+
final String prefix,
235+
final E... enabled) {
236+
final FlagSet<E> flagSet = new FlagSet<>(enumClass, prefix, null);
237+
Arrays.stream(enabled).forEach(flagSet::enable);
238+
return flagSet;
239+
}
240+
213241
/**
214242
* Build a FlagSet from a comma separated list of values.
215243
* Case independent.

‎hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/impl/TestFlagSet.java‎

Lines changed: 62 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,17 @@
3535
/**
3636
* Unit tests for {@link FlagSet} class.
3737
*/
38-
public class TestFlagSet extends AbstractHadoopTestBase {
38+
public final class TestFlagSet extends AbstractHadoopTestBase {
3939

40-
public static final String KEY = "key";
40+
private static final String KEY = "key";
41+
42+
private static final String KEYDOT = KEY + ".";
4143

4244
/**
4345
* Flagset used in tests and assertions.
4446
*/
4547
private FlagSet<SimpleEnum> flagSet =
46-
createFlagSet(SimpleEnum.class, KEY + ".", noneOf(SimpleEnum.class));
48+
createFlagSet(SimpleEnum.class, KEYDOT, noneOf(SimpleEnum.class));
4749

4850
/**
4951
* Simple Enums.
@@ -54,7 +56,7 @@ private enum SimpleEnum { a, b, c }
5456
* Test that an entry can be enabled and disabled.
5557
*/
5658
@Test
57-
public void testEntryEnableDisable() throws Throwable {
59+
public void testEntryEnableDisable() {
5860
Assertions.assertThat(flagSet.flags()).isEmpty();
5961
assertDisabled(SimpleEnum.a);
6062
flagSet.enable(SimpleEnum.a);
@@ -67,7 +69,7 @@ public void testEntryEnableDisable() throws Throwable {
6769
* Test the setter.
6870
*/
6971
@Test
70-
public void testSetMethod() throws Throwable {
72+
public void testSetMethod() {
7173
Assertions.assertThat(flagSet.flags()).isEmpty();
7274
flagSet.set(SimpleEnum.a, true);
7375
assertEnabled(SimpleEnum.a);
@@ -105,12 +107,12 @@ public void testMutability() throws Throwable {
105107
public void testToString() throws Throwable {
106108
// empty
107109
assertStringValue("{}");
108-
assertConfigurationString("");
110+
assertConfigurationStringMatches("");
109111

110112
// single value
111113
flagSet.enable(SimpleEnum.a);
112114
assertStringValue("{a}");
113-
assertConfigurationString("a");
115+
assertConfigurationStringMatches("a");
114116

115117
// add a second value.
116118
flagSet.enable(SimpleEnum.b);
@@ -130,7 +132,7 @@ private void assertStringValue(final String expected) {
130132
/**
131133
* Assert the configuration string form matches that expected.
132134
*/
133-
public void assertConfigurationString(final String expected) {
135+
public void assertConfigurationStringMatches(final String expected) {
134136
Assertions.assertThat(flagSet.toConfigurationString())
135137
.describedAs("Configuration string of %s", flagSet)
136138
.isEqualTo(expected);
@@ -141,23 +143,34 @@ public void assertConfigurationString(final String expected) {
141143
* Multiple entries must be parsed, whitespace trimmed.
142144
*/
143145
@Test
144-
public void testConfEntry() throws Throwable {
145-
final Configuration conf = mkConf("a\t,\nc ");
146-
flagSet = buildFlagSet(SimpleEnum.class, conf, KEY, true);
146+
public void testConfEntry() {
147+
flagSet = flagSetFromConfig("a\t,\nc ", true);
147148
assertFlagSetMatches(flagSet, SimpleEnum.a, SimpleEnum.c);
148149
assertHasCapability(KEY + ".a");
149150
assertHasCapability(KEY + ".c");
150151
assertLacksCapability(KEY + ".b");
151152
}
152153

154+
/**
155+
* Create a flagset from a configuration string.
156+
* @param string configuration string.
157+
* @param ignoreUnknown should unknown values be ignored?
158+
* @return a flagset
159+
*/
160+
private static FlagSet<SimpleEnum> flagSetFromConfig(final String string,
161+
final boolean ignoreUnknown) {
162+
final Configuration conf = mkConf(string);
163+
final FlagSet<SimpleEnum> fs = buildFlagSet(SimpleEnum.class, conf, KEY, ignoreUnknown);
164+
return fs;
165+
}
166+
153167
/**
154168
* Test parsing from a configuration file,
155169
* where an entry is unknown; the builder is set to ignoreUnknown.
156170
*/
157171
@Test
158-
public void testConfEntryWithUnknownIgnored() throws Throwable {
159-
final Configuration conf = mkConf("a, unknown");
160-
flagSet = buildFlagSet(SimpleEnum.class, conf, KEY, true);
172+
public void testConfEntryWithUnknownIgnored() {
173+
flagSet = flagSetFromConfig("a, unknown", true);
161174
assertFlagSetMatches(flagSet, SimpleEnum.a);
162175
assertHasCapability(KEY + ".a");
163176
assertLacksCapability(KEY + ".b");
@@ -168,9 +181,8 @@ public void testConfEntryWithUnknownIgnored() throws Throwable {
168181
* the same entry is duplicated.
169182
*/
170183
@Test
171-
public void testDuplicateConfEntry() throws Throwable {
172-
final Configuration conf = mkConf("a,\ta,\na");
173-
flagSet = buildFlagSet(SimpleEnum.class, conf, KEY, true);
184+
public void testDuplicateConfEntry() {
185+
flagSet = flagSetFromConfig("a,\ta,\na\"", true);
174186
assertFlagSetMatches(flagSet, SimpleEnum.a);
175187
assertHasCapability(KEY + ".a");
176188
}
@@ -181,7 +193,7 @@ public void testDuplicateConfEntry() throws Throwable {
181193
@Test
182194
public void testConfUnknownFailure() throws Throwable {
183195
intercept(IllegalArgumentException.class, () ->
184-
buildFlagSet(SimpleEnum.class, mkConf("a, unknown"), KEY, false));
196+
flagSetFromConfig("a, unknown", false));
185197
}
186198

187199
/**
@@ -220,44 +232,66 @@ private void assertLacksCapability(final String capability) {
220232
*/
221233
@Test
222234
public void testFlagSetStarEntry() {
223-
final Configuration conf = mkConf("*");
224-
flagSet = buildFlagSet(SimpleEnum.class, conf, KEY, false);
235+
flagSet = flagSetFromConfig("*", false);
225236
assertFlags(SimpleEnum.a, SimpleEnum.b, SimpleEnum.c);
226237
assertHasCapability(KEY + ".a");
227238
assertHasCapability(KEY + ".b");
228239
}
229240

230241
@Test
231242
public void testRoundTrip() {
232-
final FlagSet<SimpleEnum> s1 = createFlagSet(SimpleEnum.class, KEY + ".", allOf(SimpleEnum.class));
243+
final FlagSet<SimpleEnum> s1 = createFlagSet(SimpleEnum.class, KEYDOT, allOf(SimpleEnum.class));
233244
final FlagSet<SimpleEnum> s2 = roundTrip(s1);
234245
Assertions.assertThat(s1.flags()).isEqualTo(s2.flags());
235246
assertFlagSetMatches(s2, SimpleEnum.a, SimpleEnum.b, SimpleEnum.c);
236247
}
237248

238249
@Test
239250
public void testEmptyRoundTrip() {
240-
final FlagSet<SimpleEnum> s1 = createFlagSet(SimpleEnum.class, KEY + ".", noneOf(SimpleEnum.class));
251+
final FlagSet<SimpleEnum> s1 = createFlagSet(SimpleEnum.class, KEYDOT, noneOf(SimpleEnum.class));
241252
final FlagSet<SimpleEnum> s2 = roundTrip(s1);
242-
Assertions.assertThat(s1.flags()).isEqualTo(s2.flags());
253+
Assertions.assertThat(s1.flags())
254+
.isEqualTo(s2.flags());
243255
Assertions.assertThat(s2.isEmpty())
244256
.describedAs("empty flagset %s", s2)
245257
.isTrue();
246258
assertFlagSetMatches(flagSet);
247259
}
248260

249261
@Test
250-
public void testSetIsClone() throws Throwable {
262+
public void testSetIsClone() {
251263
final EnumSet<SimpleEnum> flags = noneOf(SimpleEnum.class);
252-
final FlagSet<SimpleEnum> s1 = createFlagSet(SimpleEnum.class, KEY + ".", flags);
264+
final FlagSet<SimpleEnum> s1 = createFlagSet(SimpleEnum.class, KEYDOT, flags);
253265
s1.enable(SimpleEnum.b);
254266

255267
// set a source flag
256268
flags.add(SimpleEnum.a);
257269

258270
// verify the derived flagset is unchanged
259271
assertFlagSetMatches(s1, SimpleEnum.b);
272+
}
260273

274+
@Test
275+
public void testEquality() {
276+
final FlagSet<SimpleEnum> s1 = createFlagSet(SimpleEnum.class, KEYDOT, SimpleEnum.a);
277+
final FlagSet<SimpleEnum> s2 = createFlagSet(SimpleEnum.class, KEYDOT, SimpleEnum.a);
278+
// make one of them immutable
279+
s2.makeImmutable();
280+
Assertions.assertThat(s1)
281+
.describedAs("s1 == s2")
282+
.isEqualTo(s2);
283+
Assertions.assertThat(s1.hashCode())
284+
.describedAs("hashcode of s1 == hashcode of s2")
285+
.isEqualTo(s2.hashCode());
286+
}
287+
288+
@Test
289+
public void testInequality() {
290+
final FlagSet<SimpleEnum> s1 = createFlagSet(SimpleEnum.class, KEYDOT, noneOf(SimpleEnum.class));
291+
final FlagSet<SimpleEnum> s2 = createFlagSet(SimpleEnum.class, KEYDOT, SimpleEnum.a, SimpleEnum.b);
292+
Assertions.assertThat(s1)
293+
.describedAs("s1 == s2")
294+
.isNotEqualTo(s2);
261295
}
262296

263297
/**
@@ -272,7 +306,7 @@ private FlagSet<SimpleEnum> roundTrip(FlagSet<SimpleEnum> flagset) {
272306
}
273307

274308
/**
275-
* Assert a flag is enabled.
309+
* Assert a flag is enabled in the {@link #flagSet} field.
276310
* @param flag flag to check
277311
*/
278312
private void assertEnabled(final SimpleEnum flag) {
@@ -282,7 +316,7 @@ private void assertEnabled(final SimpleEnum flag) {
282316
}
283317

284318
/**
285-
* Assert a flag is disabled.
319+
* Assert a flag is disabled in the {@link #flagSet} field.
286320
* @param flag flag to check
287321
*/
288322
private void assertDisabled(final SimpleEnum flag) {
@@ -291,9 +325,8 @@ private void assertDisabled(final SimpleEnum flag) {
291325
.isFalse();
292326
}
293327

294-
295328
/**
296-
* Assert that a set of flags are enabled.
329+
* Assert that a set of flags are enabled in the {@link #flagSet} field.
297330
* @param flags flags which must be set.
298331
*/
299332
private void assertFlags(final SimpleEnum... flags) {

0 commit comments

Comments
 (0)