From: Robert Varga Date: Wed, 13 Apr 2022 12:12:03 +0000 (+0200) Subject: Do not support unions with complex types X-Git-Tag: v9.0.0~132 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=yangtools.git;a=commitdiff_plain;h=93048afc8037fd8bb631150361b05e4ab482a623 Do not support unions with complex types Our falling back to string representation is actively hurtful, as we end up admitting the wrong type. We really should be treating union as a complex type and not handle it at all, but that may break users more than we want to in one release. JIRA: YANGTOOLS-1427 Change-Id: I944e5869a718a768799bf334e4291cd0eb861e98 Signed-off-by: Robert Varga --- diff --git a/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/BitsStringCodec.java b/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/BitsStringCodec.java index 509110cb84..d35f4b8a2d 100644 --- a/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/BitsStringCodec.java +++ b/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/BitsStringCodec.java @@ -18,7 +18,6 @@ import com.google.common.collect.ImmutableSet; import java.util.ArrayList; import java.util.List; import java.util.Set; -import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.yangtools.yang.data.api.codec.BitsCodec; import org.opendaylight.yangtools.yang.model.api.type.BitsTypeDefinition; import org.opendaylight.yangtools.yang.model.api.type.BitsTypeDefinition.Bit; @@ -36,13 +35,13 @@ public final class BitsStringCodec extends TypeDefinitionAwareCodec, private final ImmutableSet validBits; @SuppressWarnings("unchecked") - private BitsStringCodec(final @NonNull BitsTypeDefinition typeDef) { - super(typeDef, (Class>) (Class) Set.class); + private BitsStringCodec(final BitsTypeDefinition typeDef) { + super(requireNonNull(typeDef), (Class>) (Class) Set.class); validBits = ImmutableSet.copyOf(Collections2.transform(typeDef.getBits(), Bit::getName)); } public static BitsStringCodec from(final BitsTypeDefinition type) { - return new BitsStringCodec(requireNonNull(type)); + return new BitsStringCodec(type); } @Override diff --git a/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/BooleanStringCodec.java b/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/BooleanStringCodec.java index 643b4a8413..068297015c 100644 --- a/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/BooleanStringCodec.java +++ b/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/BooleanStringCodec.java @@ -20,12 +20,12 @@ import org.opendaylight.yangtools.yang.model.api.type.BooleanTypeDefinition; @Beta public final class BooleanStringCodec extends TypeDefinitionAwareCodec implements BooleanCodec { - private BooleanStringCodec(final @NonNull BooleanTypeDefinition typeDef) { - super(typeDef, Boolean.class); + private BooleanStringCodec(final BooleanTypeDefinition typeDef) { + super(requireNonNull(typeDef), Boolean.class); } - public static @NonNull BooleanStringCodec from(final BooleanTypeDefinition normalizedType) { - return new BooleanStringCodec(requireNonNull(normalizedType)); + public static @NonNull BooleanStringCodec from(final BooleanTypeDefinition typeDef) { + return new BooleanStringCodec(typeDef); } @Override diff --git a/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/DecimalStringCodec.java b/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/DecimalStringCodec.java index 35a5c87f8d..c3724ac142 100644 --- a/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/DecimalStringCodec.java +++ b/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/DecimalStringCodec.java @@ -22,11 +22,11 @@ import org.opendaylight.yangtools.yang.model.api.type.DecimalTypeDefinition; public final class DecimalStringCodec extends TypeDefinitionAwareCodec implements DecimalCodec { private DecimalStringCodec(final DecimalTypeDefinition typeDef) { - super(typeDef, Decimal64.class); + super(requireNonNull(typeDef), Decimal64.class); } public static @NonNull DecimalStringCodec from(final DecimalTypeDefinition type) { - return new DecimalStringCodec(requireNonNull(type)); + return new DecimalStringCodec(type); } @Override diff --git a/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/EmptyStringCodec.java b/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/EmptyStringCodec.java index 62655f9c6f..3522b2e3b6 100644 --- a/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/EmptyStringCodec.java +++ b/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/EmptyStringCodec.java @@ -9,7 +9,6 @@ package org.opendaylight.yangtools.yang.data.impl.codec; import static com.google.common.base.Preconditions.checkArgument; -import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.yangtools.yang.common.Empty; import org.opendaylight.yangtools.yang.data.api.codec.EmptyCodec; import org.opendaylight.yangtools.yang.model.api.type.EmptyTypeDefinition; @@ -29,7 +28,7 @@ final class EmptyStringCodec extends TypeDefinitionAwareCodec { private final ImmutableMap values; - private EnumStringCodec(final @NonNull EnumTypeDefinition typeDef) { - super(typeDef, String.class); + private EnumStringCodec(final EnumTypeDefinition typeDef) { + super(requireNonNull(typeDef), String.class); values = typeDef.getValues().stream() // Intern the String to get wide reuse .map(pair -> pair.getName().intern()) .collect(ImmutableMap.toImmutableMap(Functions.identity(), Functions.identity())); } - public static EnumStringCodec from(final EnumTypeDefinition normalizedType) { - return new EnumStringCodec(requireNonNull(normalizedType)); + public static EnumStringCodec from(final EnumTypeDefinition typeDef) { + return new EnumStringCodec(typeDef); } @Override diff --git a/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/TypeDefinitionAwareCodec.java b/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/TypeDefinitionAwareCodec.java index b563bff000..81f6d8f8e6 100644 --- a/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/TypeDefinitionAwareCodec.java +++ b/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/TypeDefinitionAwareCodec.java @@ -29,8 +29,18 @@ import org.opendaylight.yangtools.yang.model.api.type.Uint32TypeDefinition; import org.opendaylight.yangtools.yang.model.api.type.Uint64TypeDefinition; import org.opendaylight.yangtools.yang.model.api.type.Uint8TypeDefinition; import org.opendaylight.yangtools.yang.model.api.type.UnionTypeDefinition; +import org.slf4j.LoggerFactory; public abstract class TypeDefinitionAwareCodec> extends AbstractDataStringCodec { + private static final boolean ENABLE_UNION_CODEC = + !Boolean.getBoolean("org.opendaylight.yangtools.yang.data.impl.codec.disable-union"); + + static { + if (!ENABLE_UNION_CODEC) { + LoggerFactory.getLogger(TypeDefinitionAwareCodec.class).info("Support for unions is disabled"); + } + } + private final @NonNull Class inputClass; private final @Nullable T typeDefinition; @@ -45,6 +55,7 @@ public abstract class TypeDefinitionAwareCodec> e return inputClass; } + // FIXME: is this even useful? public Optional getTypeDefinition() { return Optional.ofNullable(typeDefinition); } @@ -78,8 +89,6 @@ public abstract class TypeDefinitionAwareCodec> e return AbstractIntegerStringCodec.from((Int64TypeDefinition) typeDefinition); } else if (typeDefinition instanceof StringTypeDefinition) { return StringStringCodec.from((StringTypeDefinition)typeDefinition); - } else if (typeDefinition instanceof UnionTypeDefinition) { - return UnionStringCodec.from((UnionTypeDefinition)typeDefinition); } else if (typeDefinition instanceof Uint8TypeDefinition) { return AbstractIntegerStringCodec.from((Uint8TypeDefinition) typeDefinition); } else if (typeDefinition instanceof Uint16TypeDefinition) { @@ -88,6 +97,8 @@ public abstract class TypeDefinitionAwareCodec> e return AbstractIntegerStringCodec.from((Uint32TypeDefinition) typeDefinition); } else if (typeDefinition instanceof Uint64TypeDefinition) { return AbstractIntegerStringCodec.from((Uint64TypeDefinition) typeDefinition); + } else if (ENABLE_UNION_CODEC && typeDefinition instanceof UnionTypeDefinition) { + return UnionStringCodec.from((UnionTypeDefinition)typeDefinition); } else { return null; } diff --git a/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/UnionStringCodec.java b/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/UnionStringCodec.java index 3053b1af96..d5a47c0e37 100644 --- a/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/UnionStringCodec.java +++ b/data/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/UnionStringCodec.java @@ -9,9 +9,12 @@ package org.opendaylight.yangtools.yang.data.impl.codec; import static java.util.Objects.requireNonNull; +import com.google.common.collect.ImmutableList; +import java.util.ArrayList; import java.util.Base64; +import java.util.List; +import org.eclipse.jdt.annotation.Nullable; import org.opendaylight.yangtools.yang.data.api.codec.UnionCodec; -import org.opendaylight.yangtools.yang.model.api.TypeDefinition; import org.opendaylight.yangtools.yang.model.api.type.UnionTypeDefinition; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -20,36 +23,49 @@ final class UnionStringCodec extends TypeDefinitionAwareCodec { private static final Logger LOG = LoggerFactory.getLogger(UnionStringCodec.class); - UnionStringCodec(final UnionTypeDefinition typeDef) { + private final ImmutableList> codecs; + + private UnionStringCodec(final UnionTypeDefinition typeDef, + final ImmutableList> codecs) { super(requireNonNull(typeDef), Object.class); + this.codecs = requireNonNull(codecs); } - static TypeDefinitionAwareCodec from(final UnionTypeDefinition normalizedType) { - return new UnionStringCodec(normalizedType); + static @Nullable TypeDefinitionAwareCodec from(final UnionTypeDefinition typeDef) { + final var types = typeDef.getTypes(); + final var builder = ImmutableList.>builderWithExpectedSize(types.size()); + for (var type : types) { + final var codec = from(type); + if (codec == null) { + LOG.debug("Cannot handle {} because of unhandled component {}", typeDef, type); + return null; + } + builder.add(codec); + } + return new UnionStringCodec(typeDef, builder.build()); } @Override - @SuppressWarnings("checkstyle:illegalCatch") protected Object deserializeImpl(final String stringRepresentation) { - for (final TypeDefinition type : getTypeDefinition().get().getTypes()) { - final TypeDefinitionAwareCodec typeAwareCodec = from(type); - if (typeAwareCodec == null) { - /* - * This is a type for which we have no codec (eg identity ref) so we'll say it's - * valid - */ - return stringRepresentation; - } - + List suppressed = null; + for (var codec : codecs) { try { - return typeAwareCodec.deserialize(stringRepresentation); - } catch (final Exception e) { - LOG.debug("Value {} did not matched representation for {}",stringRepresentation,type,e); + return codec.deserialize(stringRepresentation); + } catch (final IllegalArgumentException e) { // invalid - try the next union type. + LOG.debug("Value {} did not match codec {}", stringRepresentation, codec, e); + if (suppressed == null) { + suppressed = new ArrayList<>(); + } + suppressed.add(e); } } - throw new IllegalArgumentException("Invalid value \"" + stringRepresentation + "\" for union type."); + final var ex = new IllegalArgumentException("Invalid value \"" + stringRepresentation + "\" for union type."); + if (suppressed != null) { + suppressed.forEach(ex::addSuppressed); + } + throw ex; } @Override