From c2e78da031e0183447fd29092e9c8da45e318feb Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Tue, 26 Apr 2022 14:47:23 +0200 Subject: [PATCH] Cleanup enumeration static factory methods 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 --- .../java/api/generator/EnumTemplate.xtend | 44 ++++++++++++++----- .../yangtools/yang/binding/CodeHelpers.java | 32 ++++++++++++++ .../yang/binding/CodeHelpersTest.java | 20 +++++++++ .../types/rev171204/IetfRoutingUtils.java | 30 ++++++------- .../ethertypes/rev190304/EthertypeUtils.java | 7 ++- 5 files changed, 102 insertions(+), 31 deletions(-) diff --git a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/EnumTemplate.xtend b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/EnumTemplate.xtend index c71bef1b87..82b36fa69b 100644 --- a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/EnumTemplate.xtend +++ b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/EnumTemplate.xtend @@ -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); + } } ''' diff --git a/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/CodeHelpers.java b/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/CodeHelpers.java index 028543b2cd..75e625fc1f 100644 --- a/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/CodeHelpers.java +++ b/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/CodeHelpers.java @@ -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 @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 @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. * diff --git a/binding/yang-binding/src/test/java/org/opendaylight/yangtools/yang/binding/CodeHelpersTest.java b/binding/yang-binding/src/test/java/org/opendaylight/yangtools/yang/binding/CodeHelpersTest.java index a3d81ea2d2..f39c5062ce 100644 --- a/binding/yang-binding/src/test/java/org/opendaylight/yangtools/yang/binding/CodeHelpersTest.java +++ b/binding/yang-binding/src/test/java/org/opendaylight/yangtools/yang/binding/CodeHelpersTest.java @@ -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)); + } } diff --git a/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 b/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 index efda958985..e61e356ab6 100644 --- a/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 +++ b/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 @@ -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) { diff --git a/model/ietf/rfc8519-ietf-ethertypes/src/main/java/org/opendaylight/yang/gen/v1/urn/ietf/params/xml/ns/yang/ietf/ethertypes/rev190304/EthertypeUtils.java b/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 3f826d45d9..e65a2a881a 100644 --- a/model/ietf/rfc8519-ietf-ethertypes/src/main/java/org/opendaylight/yang/gen/v1/urn/ietf/params/xml/ns/yang/ietf/ethertypes/rev190304/EthertypeUtils.java +++ b/model/ietf/rfc8519-ietf-ethertypes/src/main/java/org/opendaylight/yang/gen/v1/urn/ietf/params/xml/ns/yang/ietf/ethertypes/rev190304/EthertypeUtils.java @@ -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 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)); } /** -- 2.36.6