From fab919714fbb4e663635371bb27d0666e16eb300 Mon Sep 17 00:00:00 2001 From: Ruslan Kashapov Date: Wed, 23 Nov 2022 08:05:12 +0200 Subject: [PATCH] Generate binding-dom-codec classes into separate packages We are namespace-squatting on the codegen namespace, adding various magic suffixes to ensure uniqueness. This is not perfect, because compile-time codegen does not have a completely free reign over its namespace. This is also problematic if these classes were delivered through a separate jar -- it would cause split packages, which are ugly. Allocate three new namespaces and dump codec classes there, thus ensuring no overlap, squatting or similar. JIRA: MDSAL-784 Change-Id: I68b8db09ffc9286bb40118545619760b9060a082 Signed-off-by: Ruslan Kashapov Signed-off-by: Robert Varga --- binding/mdsal-binding-dom-codec/pom.xml | 5 ++ .../codec/impl/CodecDataObjectGenerator.java | 4 +- .../binding/dom/codec/impl/CodecPackage.java | 57 +++++++++++++++++++ .../impl/DataObjectStreamerGenerator.java | 2 +- .../codec/impl/NotificationCodecContext.java | 4 +- .../codec/impl/OpaqueNodeCodecContext.java | 2 +- .../dom/codec/impl/CodecClassLoaderTest.java | 53 +++++++++++++++++ .../binding/loader/BindingClassLoader.java | 23 +++----- 8 files changed, 128 insertions(+), 22 deletions(-) create mode 100644 binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecPackage.java create mode 100644 binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecClassLoaderTest.java diff --git a/binding/mdsal-binding-dom-codec/pom.xml b/binding/mdsal-binding-dom-codec/pom.xml index 6e16f67d20..889b9711e2 100644 --- a/binding/mdsal-binding-dom-codec/pom.xml +++ b/binding/mdsal-binding-dom-codec/pom.xml @@ -116,5 +116,10 @@ mdsal-binding-runtime-spi test + + org.junit.jupiter + junit-jupiter-params + test + diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecDataObjectGenerator.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecDataObjectGenerator.java index 62776c833c..9d00f9adc4 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecDataObjectGenerator.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecDataObjectGenerator.java @@ -263,7 +263,7 @@ abstract class CodecDataObjectGenerator> implements static > Class generate(final BindingClassLoader loader, final Class bindingInterface, final ImmutableMap simpleProperties, final Map, PropertyInfo> daoProperties, final Method keyMethod) { - return loader.generateClass(bindingInterface, "codecImpl", + return CodecPackage.CODEC.generateClass(loader, bindingInterface, new Reusable<>(BB_CDO, simpleProperties, daoProperties, keyMethod)); } @@ -271,7 +271,7 @@ abstract class CodecDataObjectGenerator> implements final BindingClassLoader loader, final Class bindingInterface, final ImmutableMap simpleProperties, final Map, PropertyInfo> daoProperties, final Method keyMethod) { - return loader.generateClass(bindingInterface, "codecImpl", + return CodecPackage.CODEC.generateClass(loader, bindingInterface, new Reusable<>(BB_ACDO, simpleProperties, daoProperties, keyMethod)); } diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecPackage.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecPackage.java new file mode 100644 index 0000000000..0d3c7d9d78 --- /dev/null +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecPackage.java @@ -0,0 +1,57 @@ +/* + * Copyright (c) 2022 PANTHEON.tech, s.r.o. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ +package org.opendaylight.mdsal.binding.dom.codec.impl; + +import static com.google.common.base.Preconditions.checkArgument; +import static java.util.Objects.requireNonNull; + +import org.eclipse.jdt.annotation.NonNull; +import org.opendaylight.mdsal.binding.loader.BindingClassLoader; +import org.opendaylight.mdsal.binding.loader.BindingClassLoader.ClassGenerator; +import org.opendaylight.mdsal.binding.spec.naming.BindingMapping; + +/** + * Centralized registry of Java package names used by classes generated by codec components. + */ +enum CodecPackage { + /** + * Package holding {@link CodecDataObject}s, {@link CodecOpaqueObject}s and similar. + */ + CODEC("org.opendaylight.yang.rt.v1.obj"), + /** + * Package holding {@link DataObjectStreamer}s. + */ + STREAMER("org.opendaylight.yang.rt.v1.stream"), + /** + * Package holding @link EventInstantAware} specializations of {@code notification} objects. + */ + EVENT_AWARE("org.opendaylight.yang.rt.v1.eia"); + + private static final int PACKAGE_PREFIX_LENGTH = BindingMapping.PACKAGE_PREFIX.length(); + + private String packagePrefix; + + CodecPackage(final String packagePrefix) { + this.packagePrefix = requireNonNull(packagePrefix); + } + + @NonNull Class generateClass(final BindingClassLoader loader, final Class bindingInterface, + final ClassGenerator generator) { + return loader.generateClass(bindingInterface, createFQCN(bindingInterface), generator); + } + + @NonNull Class getGeneratedClass(final BindingClassLoader loader, final Class bindingInterface) { + return loader.getGeneratedClass(bindingInterface, createFQCN(bindingInterface)); + } + + private @NonNull String createFQCN(final Class bindingInterface) { + final var ifName = bindingInterface.getName(); + checkArgument(ifName.startsWith(BindingMapping.PACKAGE_PREFIX), "Unrecognized interface %s", bindingInterface); + return packagePrefix + ifName.substring(PACKAGE_PREFIX_LENGTH); + } +} diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/DataObjectStreamerGenerator.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/DataObjectStreamerGenerator.java index 1161b5ca34..06db0fe200 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/DataObjectStreamerGenerator.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/DataObjectStreamerGenerator.java @@ -196,7 +196,7 @@ final class DataObjectStreamerGenerator> impleme throw new UnsupportedOperationException("Schema type " + schema.getClass() + " is not supported"); } - return loader.generateClass(type, "streamer", + return CodecPackage.STREAMER.generateClass(loader, type, // FIXME: cast to GeneratedType: we really should adjust getTypeWithSchema() new DataObjectStreamerGenerator<>(registry, (GeneratedType) typeAndSchema.javaType(), (DataNodeContainer) schema, type, startEvent)); diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/NotificationCodecContext.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/NotificationCodecContext.java index af0071b99d..a964068d52 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/NotificationCodecContext.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/NotificationCodecContext.java @@ -73,9 +73,9 @@ final class NotificationCodecContext super(DataContainerCodecPrototype.from(key, schema, factory)); final Class bindingClass = getBindingClass(); - final Class awareClass = factory().getLoader().generateClass(bindingClass, "eventInstantAware", + final Class awareClass = CodecPackage.EVENT_AWARE.generateClass(factory().getLoader(), bindingClass, (loader, fqcn, bindingInterface) -> { - final Class codecImpl = loader.getGeneratedClass(bindingClass, "codecImpl"); + final Class codecImpl = CodecPackage.CODEC.getGeneratedClass(loader, bindingClass); return GeneratorResult.of(new ByteBuddy() .subclass(codecImpl, ConstructorStrategy.Default.NO_CONSTRUCTORS) diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/OpaqueNodeCodecContext.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/OpaqueNodeCodecContext.java index fd4ac8a7ea..fe850b87c1 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/OpaqueNodeCodecContext.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/OpaqueNodeCodecContext.java @@ -155,7 +155,7 @@ abstract class OpaqueNodeCodecContext> extends ValueNo } private static MethodHandle createImpl(final BindingClassLoader rootLoader, final Class bindingClass) { - final Class proxyClass = rootLoader.generateClass(bindingClass, "codecImpl", + final Class proxyClass = CodecPackage.CODEC.generateClass(rootLoader, bindingClass, (loader, fqcn, bindingInterface) -> GeneratorResult.of(TEMPLATE .name(fqcn) .implement(bindingInterface) diff --git a/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecClassLoaderTest.java b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecClassLoaderTest.java new file mode 100644 index 0000000000..a4b36e9631 --- /dev/null +++ b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/CodecClassLoaderTest.java @@ -0,0 +1,53 @@ +/* + * Copyright (c) 2022 PANTHEON.tech s.r.o. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ +package org.opendaylight.mdsal.binding.dom.codec.impl; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +import java.util.stream.Stream; +import net.bytebuddy.ByteBuddy; +import net.bytebuddy.implementation.FixedValue; +import net.bytebuddy.matcher.ElementMatchers; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.opendaylight.mdsal.binding.loader.BindingClassLoader; +import org.opendaylight.mdsal.binding.loader.BindingClassLoader.GeneratorResult; +import org.opendaylight.yang.gen.v1.urn.opendaylight.yang.union.test.rev220428.Top; + +public class CodecClassLoaderTest { + + private final BindingClassLoader codecClassLoader = BindingClassLoader.create(CodecClassLoaderTest.class, null); + + @ParameterizedTest(name = "Generate class within namespace: {0}") + @MethodSource("generateClassWithinNamespaceArgs") + void generateClassWithinNamespace(final CodecPackage pkg, final String expectedClassName) { + final Class generated = pkg.generateClass(codecClassLoader, Top.class, + (loader, fqcn, bindingInterface) -> GeneratorResult.of(new ByteBuddy() + .subclass(Object.class) + .name(fqcn) + .method(ElementMatchers.isToString()) + .intercept(FixedValue.value("test")) + .make())); + assertNotNull(generated); + assertEquals(expectedClassName, generated.getName()); + + final Class stored = pkg.getGeneratedClass(codecClassLoader, Top.class); + assertEquals(generated, stored); + } + + private static Stream generateClassWithinNamespaceArgs() { + final String common = "urn.opendaylight.yang.union.test.rev220428.Top"; + return Stream.of( + Arguments.of(CodecPackage.CODEC, "org.opendaylight.yang.rt.v1.obj." + common), + Arguments.of(CodecPackage.STREAMER, "org.opendaylight.yang.rt.v1.stream." + common), + Arguments.of(CodecPackage.EVENT_AWARE, "org.opendaylight.yang.rt.v1.eia." + common) + ); + } +} diff --git a/binding/mdsal-binding-loader/src/main/java/org/opendaylight/mdsal/binding/loader/BindingClassLoader.java b/binding/mdsal-binding-loader/src/main/java/org/opendaylight/mdsal/binding/loader/BindingClassLoader.java index 63db1f90f1..2a75829d5d 100644 --- a/binding/mdsal-binding-loader/src/main/java/org/opendaylight/mdsal/binding/loader/BindingClassLoader.java +++ b/binding/mdsal-binding-loader/src/main/java/org/opendaylight/mdsal/binding/loader/BindingClassLoader.java @@ -143,31 +143,28 @@ public abstract sealed class BindingClassLoader extends ClassLoader } /** - * The name of the target class is formed through concatenation of the name of a {@code bindingInterface} and - * specified {@code suffix}. + * Generate a class which is related to specified compile-type-generated interface. * * @param Type of generated class * @param bindingInterface Binding compile-time-generated interface - * @param suffix Suffix to use + * @param fqcn Fully-Qualified Class Name of the generated class * @param generator Code generator to run * @return A generated class object * @throws NullPointerException if any argument is null */ - public final Class generateClass(final Class bindingInterface, final String suffix, + public final @NonNull Class generateClass(final Class bindingInterface, final String fqcn, final ClassGenerator generator) { - return findClassLoader(requireNonNull(bindingInterface)).doGenerateClass(bindingInterface, suffix, generator); + return findClassLoader(requireNonNull(bindingInterface)).doGenerateClass(bindingInterface, fqcn, generator); } - public final @NonNull Class getGeneratedClass(final Class bindingInterface, final String suffix) { + public final @NonNull Class getGeneratedClass(final Class bindingInterface, final String fqcn) { final var loader = findClassLoader(requireNonNull(bindingInterface)); - final var fqcn = generatedClassName(bindingInterface, suffix); - final Class ret; synchronized (loader.getClassLoadingLock(fqcn)) { ret = loader.findLoadedClass(fqcn); } - checkArgument(ret != null, "Failed to find generated class %s for %s of %s", fqcn, suffix, bindingInterface); + checkArgument(ret != null, "Failed to find generated class %s for %s", fqcn, bindingInterface); return ret; } @@ -190,10 +187,8 @@ public abstract sealed class BindingClassLoader extends ClassLoader */ abstract @NonNull BindingClassLoader findClassLoader(@NonNull Class bindingClass); - private Class doGenerateClass(final Class bindingInterface, final String suffix, + private @NonNull Class doGenerateClass(final Class bindingInterface, final String fqcn, final ClassGenerator generator) { - final var fqcn = generatedClassName(bindingInterface, suffix); - synchronized (getClassLoadingLock(fqcn)) { // Attempt to find a loaded class final var existing = findLoadedClass(fqcn); @@ -255,8 +250,4 @@ public abstract sealed class BindingClassLoader extends ClassLoader } } } - - private static String generatedClassName(final Class bindingInterface, final String suffix) { - return bindingInterface.getName() + "$$$" + suffix; - } } -- 2.36.6