Cleanup enumeration static factory methods 15/100815/4
authorRobert Varga <robert.varga@pantheon.tech>
Tue, 26 Apr 2022 12:47:23 +0000 (14:47 +0200)
committerRobert Varga <nite@hq.sk>
Tue, 26 Apr 2022 15:46:44 +0000 (15:46 +0000)
We have some amount of inconsistency around enumeration returns. Improve
the situation by making forName() return a nullable value and add
ofName()/ofValue() methods which throw IAE.

While we are modifying the template, also add proper @Nullable and
@NonNull annotations.

JIRA: MDSAL-754
Change-Id: I07fe4b408174ba830bcc1369ed70597ea2538a85
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/EnumTemplate.xtend
binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/CodeHelpers.java
binding/yang-binding/src/test/java/org/opendaylight/yangtools/yang/binding/CodeHelpersTest.java
model/ietf/rfc8294-ietf-routing-types/src/main/java/org/opendaylight/yang/gen/v1/urn/ietf/params/xml/ns/yang/ietf/routing/types/rev171204/IetfRoutingUtils.java
model/ietf/rfc8519-ietf-ethertypes/src/main/java/org/opendaylight/yang/gen/v1/urn/ietf/params/xml/ns/yang/ietf/ethertypes/rev190304/EthertypeUtils.java

index c71bef1b87187ed9ccf9e0cdf2ff8ed42e274b42..82b36fa69bccae0699a7d2b6b98517edeec7eb54 100644 (file)
@@ -12,7 +12,6 @@ import static org.opendaylight.mdsal.binding.model.ri.Types.STRING;
 
 import com.google.common.collect.ImmutableMap
 import com.google.common.collect.ImmutableMap.Builder
-import java.util.Optional
 import org.opendaylight.mdsal.binding.model.api.Enumeration
 import org.opendaylight.mdsal.binding.model.api.GeneratedType
 
@@ -87,16 +86,16 @@ class EnumTemplate extends BaseTemplate {
                 VALUE_MAP = vb.build();
             }
 
-            private final «STRING.importedName» name;
+            private final «STRING.importedNonNull» name;
             private final int value;
 
-            private «enums.name»(int value, «STRING.importedName» name) {
+            private «enums.name»(int value, «STRING.importedNonNull» name) {
                 this.value = value;
                 this.name = name;
             }
 
             @«OVERRIDE.importedName»
-            public «STRING.importedName» getName() {
+            public «STRING.importedNonNull» getName() {
                 return name;
             }
 
@@ -106,25 +105,48 @@ class EnumTemplate extends BaseTemplate {
             }
 
             /**
-             * Return the enumeration member whose {@link #getName()} matches specified value.
+             * Return the enumeration member whose {@link #getName()} matches specified assigned name.
              *
              * @param name YANG assigned name
-             * @return corresponding «enums.name» item, if present
-             * @throws NullPointerException if name is null
+             * @return corresponding «enums.name» item, or {@code null} if no such item exists
+             * @throws NullPointerException if {@code name} is null
              */
-            public static «Optional.importedName»<«enums.name»> forName(«STRING.importedName» name) {
-                return «Optional.importedName».ofNullable(NAME_MAP.get(«JU_OBJECTS.importedName».requireNonNull(name)));
+            public static «enums.importedNullable» forName(«STRING.importedName» name) {
+                return NAME_MAP.get(«JU_OBJECTS.importedName».requireNonNull(name));
             }
 
             /**
              * Return the enumeration member whose {@link #getIntValue()} matches specified value.
              *
              * @param intValue integer value
-             * @return corresponding «enums.name» item, or null if no such item exists
+             * @return corresponding «enums.name» item, or {@code null} if no such item exists
              */
-            public static «enums.name» forValue(int intValue) {
+            public static «enums.importedNullable» forValue(int intValue) {
                 return VALUE_MAP.get(intValue);
             }
+
+            /**
+             * Return the enumeration member whose {@link #getName()} matches specified assigned name.
+             *
+             * @param name YANG assigned name
+             * @return corresponding «enums.name» item
+             * @throws NullPointerException if {@code name} is null
+             * @throws IllegalArgumentException if {@code name} does not match any item
+             */
+            public static «enums.importedNonNull» ofName(«STRING.importedName» name) {
+                return «CODEHELPERS.importedName».checkEnum(forName(name), name);
+            }
+
+            /**
+             * Return the enumeration member whose {@link #getIntValue()} matches specified value.
+             *
+             * @param intValue integer value
+             * @return corresponding «enums.name» item
+             * @throws IllegalArgumentException if {@code intValue} does not match any item
+             */
+            public static «enums.importedNonNull» ofValue(int intValue) {
+                return «CODEHELPERS.importedName».checkEnum(forValue(intValue), intValue);
+            }
         }
     '''
 
index 028543b2cd7cf216f7408f6935288660e512bf49..75e625fc1f11b33e97fc8461d540a9e1fe7e3112 100644 (file)
@@ -369,6 +369,38 @@ public final class CodeHelpers {
         return hash == 0 ? 31 : hash;
     }
 
+    /**
+     * Check that the specified {@link Enumeration} object is not {@code null}. This method is meant to be used with
+     * {@code ofName(String)} and {@code ofValue(int)} static factory methods.
+     *
+     * @param obj enumeration object, possibly null
+     * @param name User-supplied enumeration name
+     * @return Enumeration object
+     * @throws IllegalArgumentException if {@code obj} is null
+     */
+    public static <T extends Enumeration> @NonNull T checkEnum(final @Nullable T obj, final String name) {
+        if (obj == null) {
+            throw new IllegalArgumentException("\"" + name + "\" is not a valid name");
+        }
+        return obj;
+    }
+
+    /**
+     * Check that the specified {@link Enumeration} object is not {@code null}. This method is meant to be used with
+     * {@code ofName(String)} and {@code ofValue(int)} static factory methods.
+     *
+     * @param obj enumeration object, possibly null
+     * @param value User-supplied enumeration value
+     * @return Enumeration object
+     * @throws IllegalArgumentException if {@code obj} is null
+     */
+    public static <T extends Enumeration> @NonNull T checkEnum(final @Nullable T obj, final int value) {
+        if (obj == null) {
+            throw new IllegalArgumentException(value + " is not a valid value");
+        }
+        return obj;
+    }
+
     /**
      * Utility method for checking whether a target object is a compatible DataObject.
      *
index a3d81ea2d2fe2650134c12f64e25f3deafa2d8a1..f39c5062ceb839b93be8561427a2a87234303fba 100644 (file)
@@ -9,9 +9,11 @@ package org.opendaylight.yangtools.yang.binding;
 
 import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertThrows;
+import static org.mockito.Mockito.mock;
 
 import java.util.Collections;
 import java.util.List;
@@ -60,4 +62,22 @@ public class CodeHelpersTest {
             () -> CodeHelpers.checkSetFieldCast(CodeHelpersTest.class, "foo", Set.of(new Object())));
         assertThat(iae.getCause(), instanceOf(ClassCastException.class));
     }
+
+    @Test
+    public void testCheckEnumName() {
+        final var ex = assertThrows(IllegalArgumentException.class, () -> CodeHelpers.checkEnum(null, "xyzzy"));
+        assertEquals("\"xyzzy\" is not a valid name", ex.getMessage());
+
+        final var obj = mock(Enumeration.class);
+        assertSame(obj, CodeHelpers.checkEnum(obj, "xyzzy"));
+    }
+
+    @Test
+    public void testCheckEnumValue() {
+        final var ex = assertThrows(IllegalArgumentException.class, () -> CodeHelpers.checkEnum(null, 1234));
+        assertEquals("1234 is not a valid value", ex.getMessage());
+
+        final var obj = mock(Enumeration.class);
+        assertSame(obj, CodeHelpers.checkEnum(obj, 1234));
+    }
 }
index efda95898513455deb9b2eaba18af9274085b6d0..e61e356ab637a071c666d106b227584ef81c37ea 100644 (file)
@@ -63,9 +63,9 @@ public final class IetfRoutingUtils {
     }
 
     public static Ipv4MulticastSourceAddress ipv4MulticastSourceAddressFor(final String str) {
-        return Ipv4MulticastSourceAddress.Enumeration.forName(str)
-            .map(ENUMERATED_IPV4_MCAST_SRC::get)
-            .orElse(new Ipv4MulticastSourceAddress(new Ipv4Address(str)));
+        final var enumeration = Ipv4MulticastSourceAddress.Enumeration.forName(str);
+        return enumeration != null ? ipv4MulticastSourceAddressFor(enumeration)
+            : new Ipv4MulticastSourceAddress(new Ipv4Address(str));
     }
 
     public static Ipv4MulticastSourceAddress ipv4MulticastSourceAddressFor(
@@ -74,9 +74,9 @@ public final class IetfRoutingUtils {
     }
 
     public static Ipv6MulticastSourceAddress ipv6MulticastSourceAddressFor(final String str) {
-        return Ipv6MulticastSourceAddress.Enumeration.forName(str)
-            .map(ENUMERATED_IPV6_MCAST_SRC::get)
-            .orElse(new Ipv6MulticastSourceAddress(new Ipv6Address(str)));
+        final var enumeration = Ipv6MulticastSourceAddress.Enumeration.forName(str);
+        return enumeration != null ? ipv6MulticastSourceAddressFor(enumeration)
+            : new Ipv6MulticastSourceAddress(new Ipv6Address(str));
     }
 
     public static Ipv6MulticastSourceAddress ipv6MulticastSourceAddressFor(
@@ -85,9 +85,9 @@ public final class IetfRoutingUtils {
     }
 
     public static TimerValueMilliseconds timerValueMillisecondsFor(final String str) {
-        return TimerValueMilliseconds.Enumeration.forName(str)
-            .map(ENUMERATED_TIMERVAR_MS::get)
-            .orElse(new TimerValueMilliseconds(Uint32.valueOf(str)));
+        final var enumeration = TimerValueMilliseconds.Enumeration.forName(str);
+        return enumeration != null ? timerValueMillisecondsFor(enumeration)
+            : new TimerValueMilliseconds(Uint32.valueOf(str));
     }
 
     public static TimerValueMilliseconds timerValueMillisecondsFor(
@@ -95,10 +95,9 @@ public final class IetfRoutingUtils {
         return verifyNotNull(ENUMERATED_TIMERVAR_MS.get(requireNonNull(enumeration)));
     }
 
-
     public static TimerValueSeconds16 timerValueSeconds16For(final String str) {
-        return TimerValueSeconds16.Enumeration.forName(str).map(ENUMERATED_TIMERVAL_16::get)
-                .orElse(new TimerValueSeconds16(Uint16.valueOf(str)));
+        final var enumeration = TimerValueSeconds16.Enumeration.forName(str);
+        return enumeration != null ? timerValueSeconds16For(enumeration) : new TimerValueSeconds16(Uint16.valueOf(str));
     }
 
     public static TimerValueSeconds16 timerValueSeconds16For(
@@ -106,10 +105,9 @@ public final class IetfRoutingUtils {
         return verifyNotNull(ENUMERATED_TIMERVAL_16.get(requireNonNull(enumeration)));
     }
 
-    public static TimerValueSeconds32 timerValueSeconds32For(final String defaultValue) {
-        return TimerValueSeconds32.Enumeration.forName(defaultValue)
-            .map(ENUMERATED_TIMERVAL_32::get)
-            .orElse(new TimerValueSeconds32(Uint32.valueOf(defaultValue)));
+    public static TimerValueSeconds32 timerValueSeconds32For(final String str) {
+        final var enumeration = TimerValueSeconds32.Enumeration.forName(str);
+        return enumeration != null ? timerValueSeconds32For(enumeration) : new TimerValueSeconds32(Uint32.valueOf(str));
     }
 
     public static TimerValueSeconds32 timerValueSeconds32For(final TimerValueSeconds32.Enumeration enumeration) {
index 3f826d45d9002b76c4db8867858503c83db947d2..e65a2a881a329eee264d4f6f9f20da339b0bfa9f 100644 (file)
@@ -15,7 +15,6 @@ import com.google.common.base.CharMatcher;
 import java.util.Comparator;
 import java.util.EnumMap;
 import java.util.Objects;
-import java.util.Optional;
 import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.yangtools.yang.common.Uint16;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.ethertypes.rev190304.Ethertype.Enumeration;
@@ -52,9 +51,9 @@ public final class EthertypeUtils {
             // Fall through and interpret as a string
         }
 
-        final Optional<Enumeration> known = Enumeration.forName(defaultValue);
-        checkArgument(known.isPresent(), "Unknown ethertype %s", defaultValue);
-        return verifyNotNull(ENUM_ETHERTYPES.get(known.get()));
+        final var known = Enumeration.forName(defaultValue);
+        checkArgument(known != null, "Unknown ethertype %s", defaultValue);
+        return verifyNotNull(ENUM_ETHERTYPES.get(known));
     }
 
     /**