From: Tony Tkacik Date: Tue, 10 Dec 2013 14:21:07 +0000 (+0100) Subject: Bug 211 - Fixed codec generation when transitive dependencies (parents) are not available X-Git-Tag: jenkins-controller-bulk-release-prepare-only-2-1~214^2 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=75b3060af970364e7b856a16c47f5555d568d2f4 Bug 211 - Fixed codec generation when transitive dependencies (parents) are not available - Classloader for each encountered generated class is added to javassist ClassPool if not already there, to make sure we have full visibility into supertype hierarchy of class Signed-off-by: Tony Tkacik Change-Id: I760b6d53df2e07a9ed5e73fecd655999061cc3be --- diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/util/JavassistUtils.xtend b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/util/JavassistUtils.xtend index 802e7acb5b..74efffafc6 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/util/JavassistUtils.xtend +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/util/JavassistUtils.xtend @@ -12,9 +12,16 @@ import javassist.LoaderClassPath import javassist.ClassClassPath import java.util.concurrent.locks.Lock import java.util.concurrent.locks.ReentrantLock +import org.slf4j.LoggerFactory +import java.util.HashMap +import java.util.WeakHashMap class JavassistUtils { + private static val LOG = LoggerFactory.getLogger(JavassistUtils); + + private val loaderClassPaths = new WeakHashMap(); + ClassPool classPool @Property @@ -95,14 +102,27 @@ class JavassistUtils { try { return pool.get(cls.name) } catch (NotFoundException e) { - pool.appendClassPath(new LoaderClassPath(cls.classLoader)); + appendClassLoaderIfMissing(cls.classLoader) try { return pool.get(cls.name) - } catch (NotFoundException ef) { + LOG.warn("Appending ClassClassPath for {}",cls); pool.appendClassPath(new ClassClassPath(cls)); + return pool.get(cls.name) } } } + + def void appendClassLoaderIfMissing(ClassLoader loader) { + if(loaderClassPaths.containsKey(loader)) { + return; + } + val ctLoader = new LoaderClassPath(loader); + classPool.appendClassPath(ctLoader); + } + + def void ensureClassLoader(Class child) { + appendClassLoaderIfMissing(child.classLoader); + } } diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/dom/serializer/impl/LazyGeneratedCodecRegistry.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/dom/serializer/impl/LazyGeneratedCodecRegistry.java index 39bd0816f5..4b672f1140 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/dom/serializer/impl/LazyGeneratedCodecRegistry.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/dom/serializer/impl/LazyGeneratedCodecRegistry.java @@ -389,8 +389,10 @@ public class LazyGeneratedCodecRegistry implements // public void onModuleContextAdded(SchemaContext schemaContext, Module module, ModuleContext context) { pathToType.putAll(context.getChildNodes()); qnamesToIdentityMap.putAll(context.getIdentities()); - for(Entry identity : context.getIdentities().entrySet()) { - typeToQname.put(new ReferencedTypeImpl(identity.getValue().getPackageName(), identity.getValue().getName()),identity.getKey()); + for (Entry identity : context.getIdentities().entrySet()) { + typeToQname.put( + new ReferencedTypeImpl(identity.getValue().getPackageName(), identity.getValue().getName()), + identity.getKey()); } captureCases(context.getCases(), schemaContext); } @@ -947,12 +949,12 @@ public class LazyGeneratedCodecRegistry implements // @Override public Class deserialize(QName input) { Type type = qnamesToIdentityMap.get(input); - if(type == null) { + if (type == null) { return null; } ReferencedTypeImpl typeref = new ReferencedTypeImpl(type.getPackageName(), type.getName()); WeakReference softref = typeToClass.get(typeref); - if(softref == null) { + if (softref == null) { return null; } return softref.get(); @@ -963,12 +965,12 @@ public class LazyGeneratedCodecRegistry implements // checkArgument(BaseIdentity.class.isAssignableFrom(input)); bindingClassEncountered(input); QName qname = identityQNames.get(input); - if(qname != null) { + if (qname != null) { return qname; } ConcreteType typeref = Types.typeForClass(input); qname = typeToQname.get(typeref); - if(qname != null) { + if (qname != null) { identityQNames.put(input, qname); } return qname; @@ -980,4 +982,26 @@ public class LazyGeneratedCodecRegistry implements // return serialize((Class) input); } } + + public boolean isCodecAvailable(Class cls) { + if (containerCodecs.containsKey(cls)) { + return true; + } + if (identifierCodecs.containsKey(cls)) { + return true; + } + if (choiceCodecs.containsKey(cls)) { + return true; + } + if (caseCodecs.containsKey(cls)) { + return true; + } + if (augmentableCodecs.containsKey(cls)) { + return true; + } + if (augmentationCodecs.containsKey(cls)) { + return true; + } + return false; + } } \ No newline at end of file diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/dom/serializer/impl/RuntimeGeneratedMappingServiceImpl.xtend b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/dom/serializer/impl/RuntimeGeneratedMappingServiceImpl.xtend index 13975cad4c..853e62aa38 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/dom/serializer/impl/RuntimeGeneratedMappingServiceImpl.xtend +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/dom/serializer/impl/RuntimeGeneratedMappingServiceImpl.xtend @@ -170,7 +170,9 @@ class RuntimeGeneratedMappingServiceImpl implements BindingIndependentMappingSer */ return; } - + if(registry.isCodecAvailable(class1)) { + return; + } val ref = Types.typeForClass(class1); getSchemaWithRetry(ref); } @@ -242,31 +244,6 @@ class RuntimeGeneratedMappingServiceImpl implements BindingIndependentMappingSer } } - private def getTypeDefinition(Type type) { - val typeDef = typeToDefinition.get(type); - if (typeDef !== null) { - return typeDef; - } - return type.getTypeDefInFuture.get(); - } - - private def Future getTypeDefInFuture(Type type) { - val future = SettableFuture.create() - promisedTypeDefinitions.put(type, future); - return future; - } - - private def void updatePromisedTypeDefinitions(GeneratedTypeBuilder builder) { - val futures = promisedTypeDefinitions.get(builder); - if (futures === null || futures.empty) { - return; - } - for (future : futures) { - future.set(builder); - } - promisedTypeDefinitions.removeAll(builder); - } - private def getSchemaWithRetry(Type type) { val typeDef = typeToSchemaNode.get(type); if (typeDef !== null) { diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/dom/serializer/impl/TransformerGenerator.xtend b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/dom/serializer/impl/TransformerGenerator.xtend index b2d25af885..87c3286f95 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/dom/serializer/impl/TransformerGenerator.xtend +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/dom/serializer/impl/TransformerGenerator.xtend @@ -51,6 +51,11 @@ import org.opendaylight.yangtools.yang.model.util.ExtendedType import org.opendaylight.yangtools.yang.model.util.EnumerationType import static com.google.common.base.Preconditions.* import org.opendaylight.yangtools.yang.model.api.SchemaPath +import javassist.CtMethod +import javassist.CannotCompileException +import java.util.concurrent.locks.Lock +import java.util.concurrent.Callable +import org.opendaylight.controller.sal.binding.impl.util.ClassLoaderUtils class TransformerGenerator { @@ -91,7 +96,7 @@ class TransformerGenerator { @Property var GeneratorListener listener; - + public static val CLASS_TYPE = Types.typeForClass(Class); public new(ClassPool pool) { @@ -276,7 +281,7 @@ class TransformerGenerator { implementsType(BINDING_CODEC) method(Object, "toDomStatic", QName, Object) [ modifiers = PUBLIC + FINAL + STATIC - body = ''' + bodyChecked = ''' { «QName.name» _resultName; if($1 != null) { @@ -298,7 +303,7 @@ class TransformerGenerator { ] method(Object, "fromDomStatic", QName, Object) [ modifiers = PUBLIC + FINAL + STATIC - body = ''' + bodyChecked = ''' { if($2 == null){ return null; @@ -319,7 +324,7 @@ class TransformerGenerator { ''' ] method(Object, "serialize", Object) [ - body = ''' + bodyChecked = ''' { java.util.Map.Entry _input = (java.util.Map.Entry) $1; «QName.name» _localQName = («QName.name») _input.getKey(); @@ -329,7 +334,7 @@ class TransformerGenerator { ''' ] method(Object, "deserialize", Object) [ - body = ''' + bodyChecked = ''' return fromDomStatic(QNAME,$1); ''' ] @@ -357,7 +362,7 @@ class TransformerGenerator { staticField(it, IDENTITYREF_CODEC, BindingCodec) method(Object, "toDomStatic", QName, Object) [ modifiers = PUBLIC + FINAL + STATIC - body = ''' + bodyChecked = ''' { «QName.name» _resultName = «QName.name».create($1,QNAME.getLocalName()); java.util.List _childNodes = new java.util.ArrayList(); @@ -368,7 +373,7 @@ class TransformerGenerator { ''' ] method(Object, "serialize", Object) [ - body = ''' + bodyChecked = ''' { java.util.Map.Entry _input = (java.util.Map.Entry) $1; «QName.name» _localName = QNAME; @@ -381,10 +386,10 @@ class TransformerGenerator { ] method(Object, "fromDomStatic", QName, Object) [ modifiers = PUBLIC + FINAL + STATIC - body = deserializeBody(type, node) + bodyChecked = deserializeBody(type, node) ] method(Object, "deserialize", Object) [ - body = ''' + bodyChecked = ''' { //System.out.println("«type.name»#deserialize: " +$1); java.util.Map.Entry _input = (java.util.Map.Entry) $1; @@ -418,10 +423,10 @@ class TransformerGenerator { implementsType(BINDING_CODEC) method(Object, "toDomStatic", QName, Object) [ modifiers = PUBLIC + FINAL + STATIC - body = serializeBodyFacade(typeSpec, node) + bodyChecked = serializeBodyFacade(typeSpec, node) ] method(Object, "serialize", Object) [ - body = ''' + bodyChecked = ''' { java.util.Map.Entry _input = (java.util.Map.Entry) $1; «QName.name» _localName = QNAME; @@ -434,10 +439,10 @@ class TransformerGenerator { ] method(Object, "fromDomStatic", QName, Object) [ modifiers = PUBLIC + FINAL + STATIC - body = deserializeBody(typeSpec, node) + bodyChecked = deserializeBody(typeSpec, node) ] method(Object, "deserialize", Object) [ - body = ''' + bodyChecked = ''' return fromDomStatic(QNAME,$1); ''' ] @@ -468,7 +473,7 @@ class TransformerGenerator { implementsType(BINDING_CODEC) method(Object, "toDomStatic", QName, Object) [ modifiers = PUBLIC + FINAL + STATIC - body = ''' + bodyChecked = ''' { //System.out.println("Qname " + $1); //System.out.println("Value " + $2); @@ -485,7 +490,7 @@ class TransformerGenerator { ''' ] method(Object, "serialize", Object) [ - body = ''' + bodyChecked = ''' { java.util.Map.Entry _input = (java.util.Map.Entry) $1; «QName.name» _localName = QNAME; @@ -498,7 +503,7 @@ class TransformerGenerator { ] method(Object, "fromDomStatic", QName, Object) [ modifiers = PUBLIC + FINAL + STATIC - body = ''' + bodyChecked = ''' { «QName.name» _localQName = QNAME; @@ -522,7 +527,7 @@ class TransformerGenerator { ''' ] method(Object, "deserialize", Object) [ - body = ''' + bodyChecked = ''' return fromDomStatic(QNAME,$1); ''' ] @@ -553,7 +558,7 @@ class TransformerGenerator { implementsType(BINDING_CODEC) method(List, "toDomStatic", QName, Object) [ modifiers = PUBLIC + FINAL + STATIC - body = ''' + bodyChecked = ''' { if($2 == null) { return null; @@ -572,13 +577,13 @@ class TransformerGenerator { ''' ] method(Object, "serialize", Object) [ - body = ''' + bodyChecked = ''' throw new «UnsupportedOperationException.name»("Direct invocation not supported."); ''' ] method(Object, "fromDomStatic", QName, Map) [ modifiers = PUBLIC + FINAL + STATIC - body = ''' + bodyChecked = ''' { «BINDING_CODEC.name» _codec = («BINDING_CODEC.name») «COMPOSITE_TO_CASE».get($2); if(_codec != null) { @@ -589,7 +594,7 @@ class TransformerGenerator { ''' ] method(Object, "deserialize", Object) [ - body = ''' + bodyChecked = ''' throw new «UnsupportedOperationException.name»("Direct invocation not supported."); ''' ] @@ -822,17 +827,10 @@ class TransformerGenerator { val ret = ctCls.toClassImpl(inputType.classLoader, inputType.protectionDomain) return ret as Class, Object>>; } - var hasBinding = false; - try { - val bindingCodecClass = loadClassWithTCCL(BINDING_CODEC.name); - hasBinding = bindingCodecClass !== null; - } catch (ClassNotFoundException e) { - hasBinding = false; - } - val hasYangBinding = hasBinding + val ctCls = createClass(typeSpec.codecClassName) [ //staticField(Map,"AUGMENTATION_SERIALIZERS"); - if (hasYangBinding) { + if (inputType.isYangBindingAvailable) { implementsType(BINDING_CODEC) staticField(it, INSTANCE_IDENTIFIER_CODEC, BindingCodec) staticField(it, IDENTITYREF_CODEC, BindingCodec) @@ -840,7 +838,8 @@ class TransformerGenerator { } method(Object, "toDomValue", Object) [ modifiers = PUBLIC + FINAL + STATIC - body = ''' + val ctSpec = typeSpec.asCtClass; + bodyChecked = ''' { //System.out.println("«inputType.simpleName»#toDomValue: "+$1); @@ -857,7 +856,7 @@ class TransformerGenerator { ''' ] method(Object, "serialize", Object) [ - body = ''' + bodyChecked = ''' { return toDomValue($1); } @@ -865,7 +864,7 @@ class TransformerGenerator { ] method(Object, "fromDomValue", Object) [ modifiers = PUBLIC + FINAL + STATIC - body = ''' + bodyChecked = ''' { //System.out.println("«inputType.simpleName»#fromDomValue: "+$1); @@ -879,7 +878,7 @@ class TransformerGenerator { ''' ] method(Object, "deserialize", Object) [ - body = '''{ + bodyChecked = '''{ return fromDomValue($1); } ''' @@ -898,18 +897,37 @@ class TransformerGenerator { } + def boolean isYangBindingAvailable(Class class1) { + try { + val bindingCodecClass = class1.classLoader.loadClass(BINDING_CODEC.name); + return bindingCodecClass !== null; + } catch (ClassNotFoundException e) { + return false; + } + } + private def createDummyImplementation(Class object, GeneratedTransferObject typeSpec) { log.info("Generating Dummy DOM Codec for {} with {}", object, object.classLoader) return createClass(typeSpec.codecClassName) [ - //staticField(Map,"AUGMENTATION_SERIALIZERS"); - implementsType(BINDING_CODEC) - implementsType(BindingDeserializer.asCtClass) + if (object.isYangBindingAvailable) { + implementsType(BINDING_CODEC) + staticField(it, INSTANCE_IDENTIFIER_CODEC, BindingCodec) + staticField(it, IDENTITYREF_CODEC, BindingCodec) + implementsType(BindingDeserializer.asCtClass) + } + //implementsType(BindingDeserializer.asCtClass) method(Object, "toDomValue", Object) [ modifiers = PUBLIC + FINAL + STATIC - body = '''return null;''' + bodyChecked = '''{ + if($1 == null) { + return null; + } + return $1.toString(); + + }''' ] method(Object, "serialize", Object) [ - body = ''' + bodyChecked = ''' { return toDomValue($1); } @@ -917,10 +935,10 @@ class TransformerGenerator { ] method(Object, "fromDomValue", Object) [ modifiers = PUBLIC + FINAL + STATIC - body = '''return null;''' + bodyChecked = '''return null;''' ] method(Object, "deserialize", Object) [ - body = '''{ + bodyChecked = '''{ return fromDomValue($1); } ''' @@ -952,7 +970,7 @@ class TransformerGenerator { //implementsType(BINDING_CODEC) method(Object, "toDomValue", Object) [ modifiers = PUBLIC + FINAL + STATIC - body = '''{ + bodyChecked = '''{ if($1 == null) { return null; } @@ -967,13 +985,13 @@ class TransformerGenerator { ''' ] method(Object, "serialize", Object) [ - body = ''' + bodyChecked = ''' return toDomValue($1); ''' ] method(Object, "fromDomValue", Object) [ modifiers = PUBLIC + FINAL + STATIC - body = ''' + bodyChecked = ''' { if($1 == null) { return null; @@ -989,7 +1007,7 @@ class TransformerGenerator { ''' ] method(Object, "deserialize", Object) [ - body = ''' + bodyChecked = ''' return fromDomValue($1); ''' ] @@ -1200,7 +1218,7 @@ class TransformerGenerator { private def dispatch serializeValue(Type signature, String property) { if (INSTANCE_IDENTIFIER == signature) { return '''«INSTANCE_IDENTIFIER_CODEC».serialize(«property»)''' - }else if (CLASS_TYPE.equals(signature)) { + } else if (CLASS_TYPE.equals(signature)) { return '''(«QName.resolvedName») «IDENTITYREF_CODEC».serialize(«property»)''' } return '''«property»'''; @@ -1316,6 +1334,21 @@ class TransformerGenerator { throw exception; } + private def setBodyChecked(CtMethod method, String body) { + try { + method.setBody(body); + } catch (CannotCompileException e) { + log.error("Cannot compile method: {}#{} {}, Reason: {} Body: {}", method.declaringClass, method.name, + method.signature, e.message, body) + throw e; + } + } + + private def V withClassLoaderAndLock(ClassLoader cls, Lock lock, Callable function) throws Exception { + appendClassLoaderIfMissing(cls); + ClassLoaderUtils.withClassLoaderAndLock(cls, lock, function); + } + } @Data diff --git a/opendaylight/md-sal/sal-binding-dom-it/src/test/java/org/opendaylight/controller/sal/binding/test/bugfix/DOMCodecBug02Test.java b/opendaylight/md-sal/sal-binding-dom-it/src/test/java/org/opendaylight/controller/sal/binding/test/bugfix/DOMCodecBug02Test.java index 848fb6b190..c311161c44 100644 --- a/opendaylight/md-sal/sal-binding-dom-it/src/test/java/org/opendaylight/controller/sal/binding/test/bugfix/DOMCodecBug02Test.java +++ b/opendaylight/md-sal/sal-binding-dom-it/src/test/java/org/opendaylight/controller/sal/binding/test/bugfix/DOMCodecBug02Test.java @@ -7,9 +7,13 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; +import javassist.ClassPool; + +import org.junit.Ignore; import org.junit.Test; import org.opendaylight.controller.md.sal.common.api.TransactionStatus; import org.opendaylight.controller.sal.binding.test.AbstractDataServiceTest; +import org.opendaylight.controller.sal.binding.test.util.BindingBrokerTestFactory; import org.opendaylight.controller.sal.binding.api.data.DataModificationTransaction; import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.config.rev130819.flows.Flow; import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.Node; @@ -23,6 +27,9 @@ import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.common.RpcResult; import org.opendaylight.yangtools.yang.model.api.SchemaContext; +import com.google.common.util.concurrent.ListeningExecutorService; +import com.google.common.util.concurrent.MoreExecutors; + import static org.junit.Assert.*; public class DOMCodecBug02Test extends AbstractDataServiceTest { @@ -54,15 +61,32 @@ public class DOMCodecBug02Test extends AbstractDataServiceTest { private static final NodeRef NODE_REF = new NodeRef(NODE_INSTANCE_ID_BA); /** - * + * This test is ignored, till found out better way to test generation + * of classes without leaking of instances from previous run * * @throws Exception */ + + public void setUp() { + ListeningExecutorService executor = MoreExecutors.sameThreadExecutor(); + BindingBrokerTestFactory factory = new BindingBrokerTestFactory(); + factory.setExecutor(executor); + factory.setClassPool(new ClassPool()); + factory.setStartWithParsedSchema(getStartWithSchema()); + testContext = factory.getTestContext(); + testContext.start(); + + baDataService = testContext.getBindingDataBroker(); + biDataService = testContext.getDomDataBroker(); + dataStore = testContext.getDomDataStore(); + mappingService = testContext.getBindingToDomMappingService(); + }; + @Test public void testSchemaContextNotAvailable() throws Exception { ExecutorService testExecutor = Executors.newFixedThreadPool(1); - + testContext.loadYangSchemaFromClasspath(); Future>> future = testExecutor.submit(new Callable>>() { @Override public Future> call() throws Exception { @@ -74,11 +98,10 @@ public class DOMCodecBug02Test extends AbstractDataServiceTest { } }); - testContext.loadYangSchemaFromClasspath(); + RpcResult result = future.get().get(); assertEquals(TransactionStatus.COMMITED, result.getResult()); - Nodes nodes = checkForNodes(); assertNotNull(nodes);