Jump to: navigation, search

BestPractices/DI Guidelines

Dependency Injection (DI)

Please first understand about basic original XML instead annotation driven Using Blueprint in ODL.

Please also note the related Component Tests (with Guice) page.

This presentation from the ODL Summit 2016 has some content related to this topic.

Guidelines

  • We use constructor injection, so a typical pattern is:
   @Singleton
   public class YourThing {
   
      private final AnotherThing anotherThing;
   
      @Inject
      public YourThing(AnotherThing anotherThing) {
         this.anotherThing = anotherThing;
      }

For most typical fine grained inner bean instances in ODL, do NOT put a LOG.info("MyThing started()") in the constructor - that's pointless. The reason is that most of these classes, all those listeners and what not, really are just internal. There is no value in informing operators in a log that hundreds of internal objects have just been instantiated - so what?

  • If your class needs initialization, you can put that code either into the constructor (simpler) or into:
      @PostConstruct
      public void init() {
      }

Typically, just putting all set-up code into the constructor is simpler, easier to read, and thus recommended. Cases in which you have to put something into @PostConstruct because you cannot put it into a constructor exist, but are typically quite rare in OpenDaylight. If you do have a @PostConstruct with only a LOG.info("MyThing started!") - that's pointless; remove.

One such special case where a @PostConstruct as opposed to everything in constructor is useful (even required) is e.g. if you register something like a listener or Runnable from a constructor that runs the risk of being invoked before the constructor finished constructing the object, such as if you have to pass this to something like an executor.execute(this). The http://errorprone.info/bugpattern/ConstructorLeaksThis helps us to detect such problems.

  • If your class needs shutdown logic (e.g. to unregister listeners and the like), you can use:
       @PreDestroy
       public void destroy() {
       }

You really only should have a @PreDestroy method if you really have something else to close() or unregister() or shutdown() in it.

Do NOT use a @PreDestroy just to LOG.info("MyThing closed()") - that's pointless.

Your destroy(), or close() method typically should not @Override - the API interface should not extend AutoCloseable (or Closeable), because consumers of such API interfaces do not care about this - they will never close() your service (only Blueprint will, through @PreDestroy.

  • Use throws ModuleSetupRuntimeException if something can go wrong in your init() or destroy methods().
  • You can, if your class does not have another superclass, but do not have to, use AbstractLifecycle:
   @Singleton
   public class YourThing extends org.opendaylight.infrautils.inject.AbstractLifecycle {
   
       private final AnotherThing anotherThing;
   
       @Inject
       public YourThing(AnotherThing anotherThing) {
         this.anotherThing = anotherThing;
       }
       @Override
       protected void start() throws Exception {
       }
   
       @Override
       protected void stop() throws Exception {
       }

AbstractLifecycle (really Lifecycle) properly tracks State and has isRunning(), but in practice, for most of the fine grained inner bean instances, this isn't actually needed.

ODL Integration

Blueprint

In order to make the OSGi Blueprint (BP) implementation Apache Aries used in ODL honour @Inject etc. you activate Blueprint annotation support (which generates the BP XML file with <bean>), your project simply needs to add this to your pom.xml:

 <build>
   <plugins>
     <plugin>
       <groupId>org.apache.aries.blueprint</groupId>
       <artifactId>blueprint-maven-plugin</artifactId>
     </plugin>
   </plugins>
 </build> 

The generated XML file is moved to org/opendaylight/blueprint so it is loaded alongside other Blueprint descriptors (but note https://git.opendaylight.org/gerrit/#/c/54885/).

To use @javax.annotation.PostConstruct & @PreDestroy annotations, no new dependency is required, as those are already in the JDK RT.

To use @javax.inject.Inject annotations, add this dependency:

 <dependency>
   <groupId>javax.inject</groupId>
   <artifactId>javax.inject</artifactId>
   <optional>true</optional>
 </dependency> 

To use org.opendaylight.infrautils.inject.AbstractLifecycle, add both of these dependencies:

 <dependency>
   <groupId>org.opendaylight.infrautils</groupId>
   <artifactId>inject</artifactId>
   <version>${infrautils.version}</version>
 </dependency>
 <dependency>
   <groupId>javax.inject</groupId>
   <artifactId>javax.inject</artifactId>
   <optional>true</optional>
 </dependency> 

The javax.inject has to be repeated for to the <optional>true, as it cannot be inherited (neither transitively from infrautils.inject nor from odlparent's dependencyManagement).

In addition to this (above) Maven dependency, which is for compilation, you will typically in addition ALSO need to add a dependency to the Karaf feature named "odl-infrautils-inject" in the features.xml used by your project.

Note that you only need the infrautils.inject dependency if you actually use the AbstractLifecycle super class, which you may not, as it's an optional just for convenience (and if your @Singleton annotated classes already extend other ODL parent classes, then you cannot make them extend AbstractLifecycle). If you do not use AbstractLifecycle, then please do not add an unnecessary dependency to infrautils.inject without actually using anything from it, but depend on javax.inject directly instead - as that is much clearer.

Generate BP XML from annotations

You can use annotations instead of BP XML for OSGi service registration and lookup. This is just an alternative, and ultimately will do the exact same thing (blueprint-maven-plugin will just generate the BP XML for you, from the annotations). The reason for using in-line annotations in code rather than external XML files which implicitly reference code is that it's much clearer to read, as it clearly documents your intention - right there in the code. It's also much more refactoring friendly - should you ever change the code.

It's also much easier to write tests for code doing dependency injection with annotations.

Neon and later: @Reference & @Service (blueprint-maven-plugin-annotation)

as per https://wiki.opendaylight.org/view/Neon_platform_upgrade#Blueprint_declarations :

   <dependency>
     <groupId>org.apache.aries.blueprint</groupId>
     <artifactId>blueprint-maven-plugin-annotation</artifactId>
     <optional>true</optional>
   </dependency>

Now this is the equivalent of, and replaces, a <reference> element in BP XML:

   @Inject
   public SomeService(@Reference DataBroker dataBroker)

This is the equivalent of, and replaces, a <service> element in BP XML:

   @Singleton
   @Service(classes = ServiceInterface.class)
   public class ServiceImpl implements ServiceInterface {

pre-Neon: @OsgiService & @OsgiServiceProvider (pax-cdi-api)

   <dependency>
     <groupId>org.ops4j.pax.cdi</groupId>
     <artifactId>pax-cdi-api</artifactId>
     <optional>true</optional>
   </dependency>

Now this is the equivalent of, and replaces, a <reference> element in BP XML:

   @Inject
   public SomeService(final @OsgiService DataBroker dataBroker)

Careful with @OsgiServiceProvider: It does not seem to work well (for us in ODL); the problem is that gen. autowire.xml can contain repetitions of WRONG (bad) bean and service elements, if an impl project depends on other impl projects, which is what we have sometimes in ODL (e.g. for component tests, or for an impl that extends another impl). If despite of this you'd like to use @OsgiServiceProvider, then:

This is the equivalent of, and replaces, a <service> element in BP XML:

   @Singleton
   @OsgiServiceProvider(classes = ServiceInterface.class)
   public class ServiceImpl implements ServiceInterface {

If you use Component Tests with Guice, then you now have to add a annotatedWith(OsgiService.class):

   bind(DataTreeEventCallbackRegistrar).annotatedWith(OsgiService.class).to(DataTreeEventCallbackRegistrarImpl.class)

NB that, despite some information stating the opposite, it:

a) IS necessary to use BOTH @Singleton AND @OsgiServiceProvider (at least when using the blueprint-maven-plugin)

b) IS necessary to use classes to specify the Java interface(s) under which the export the service into the OSGi registry; as the default auto-export="interfaces" gen. in XML otherwise does not really seem to be always working reliably. (And it's perhaps safer to be explicit anyway, e.g. in case new interface are added after "implements, or in case the implementation extends some parent class.)

Inject DataBroker from mdsal (new) vs controller (old)

see https://git.opendaylight.org/gerrit/#/c/79294/7..8/coe/impl/src/main/java/org/opendaylight/netvirt/coe/listeners/NetworkPolicyListener.java and https://git.opendaylight.org/gerrit/#/c/79294/7..8/coe/impl/src/main/resources/OSGI-INF/blueprint/coe-renderer.xml

blueprint.xml

When migrating a project that is already using Blueprint to @Inject annotations (as opposed to migrating a project still on ODL Controller CSS to Blueprint), then simply remove remove all existing <bean> elements from the blueprint.xml file, as those will now be generated by the blueprint-maven-plugin based on annotations, but keep the other required elements other than <bean>:

  • reference: to get references to services
  • odl:rpc-service: to get references to RPCs
  • odl:rpc-implementation: to register RPCs
  • service ref: to announce services implemented. Ensure default naming is used as described in Nota_Bene

An example blueprint.xml:

<?xml version="1.0" encoding="UTF-8"?>
<!--
Copyright (C) 2016 Red Hat, Inc. 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
-->
<blueprint xmlns="http://www.osgi.org/xmlns/blueprint/v1.0.0"
           xmlns:odl="http://opendaylight.org/xmlns/blueprint/v1.0.0"
           odl:use-default-for-reference-types="true">

  <reference id="dataBroker"
             interface="org.opendaylight.controller.md.sal.binding.api.DataBroker"
             odl:type="default" />
  <reference id="notificationPublishService"
             interface="org.opendaylight.controller.md.sal.binding.api.NotificationPublishService" />
  <reference id="notificationService"
             interface="org.opendaylight.controller.md.sal.binding.api.NotificationService" />

  <odl:rpc-service id="idManagerService"
                   interface="org.opendaylight.yang.gen.v1.urn.opendaylight.genius.idmanager.rev160406.IdManagerService" />
  <odl:rpc-service id="odlArputilService"
                   interface="org.opendaylight.yang.gen.v1.urn.opendaylight.genius.arputil.rev160406.OdlArputilService" />
  <odl:rpc-service id="odlInterfaceRpcService"
                   interface="org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.rpcs.rev160406.OdlInterfaceRpcService" />
  <odl:rpc-service id="packetProcessingService"
                   interface="org.opendaylight.yang.gen.v1.urn.opendaylight.packet.service.rev130709.PacketProcessingService" />

  <odl:rpc-implementation ref="alivenessMonitor" />

  <service ref="mDSALManager"
           interface="org.opendaylight.genius.mdsalutil.interfaces.IMdsalApiManager" />

</blueprint>

Note how there are no <bean> elements anymore above; and e.g. ref="alivenessMonitor" refers to a <bean> that is not part of this hand-written BP XML but which is declared in the other BP XML file which got generated by the blueprint-maven-plugin based on annotations.

Tests

Please see Component Tests about how classes annotated as above are ideally suited for testing.

Nota Bene

  • @Singleton class annotation is NOT inherited (in CDI, BP and Mycila Guice Extensions; javax.inject.Singleton explicitly says so)
  • @PostConstruct & @PreDestroy method annotations are only inherited from methods in classes, not interfaces (in BP). If the annotated method is declared in a superclass, the method is called unless a subclass of the declaring class overrides the method without repeating the annotation (JSR 250). This is one of the reasons why in ODL's org.opendaylight.infrautils.inject.AbstractLifecycle @PostConstruct init() & @PreDestroy destroy() are final. See also the JavaDoc in AsyncDataTreeChangeListenerBase init() re. *Listener subclasses must repeat @PostConstruct on their own @Override init() methods. -- What slightly confuses this matter is that the blueprint-maven-plugin appear to not be fully JSR 250 compliant, and gen. XML which calls @PostConstruct annotated methods even if they are @Override in a subclass without repeating the annotation.
  • Please use only @javax.inject.Singleton in ODL instead of @Bean. It has the big advantage of being able to be used by other DI frameworks than BP, e.g. by Guice for Component Tests (which would not recognize the Blueprint specific @Bean annotation out-of-the-box; although it could be made to with additional work).
  • Do NOT use @Singleton in utility classes in *-api projects (only in *-impl). This is because if you do, because our blueprint-maven-plugin configuration in odlparent has <scanPath>org.opendaylight, it will scan the entire CP and all @Singleton on the CP get an entry in the gen. autowire.xml .. so all projects depending on said *-api project, whether they use your utility class or not, have a <bean> for it. But those projects that do not use the utility and have no Java import for it in any of their *.java will not get the Import-Package: for the utility package, because that gets added automatically by the maven-bundle-plugin (by its use of bnd), based on analysis of Java import. And this will fail the loading of that gen. autowire.xml by BP, with an "org.osgi.service.blueprint.container.ComponentDefinitionException: Unable to load class ... from recipe BeanRecipe[name='...'] Caused by: java.lang.ClassNotFoundException: ...", due to the missing Import-Package: in the MANIFEST.MF, because of OSGi bundle classloader isolation. While it would be possible to tweak the blueprint-maven-plugin <scanPath>, in each project (only scan package of that project, and if util from other project is needed, add its package to scanPath), that's error prone and a PITA to maintain, and more a kind of a workaround - BP was never really intended to be used like this! Sharing beans like this is not a good practice... better use an OSGi service for everything that is cross bundle. The rule of thumb is to either just use a utility class directly (using new) or make it a service if it is not part of your bundle. This issue was the root cause of the confusing SingleTransactionDataBroker issue on 2016.12.1 discussed in email thread "Genius CSIT Unstable" on genius-dev.
  • Recognize the default naming conventions for beans which is the class name with a lowercase first letter. This is mainly useful in cases where a class implements a service that is injected into other classes. The implementing service must announce it's service reference so that the other classes can use it. When migrating existing code to Blueprint you will likely hit this issue as the existing convention was to use the interface class name and lowercase it's first letter. An example is how in mdsalutil.xml. Notice the bean for the MDSALManager class:
<service ref="mDSALManager"
         interface="org.opendaylight.genius.mdsalutil.interfaces.IMdsalApiManager" />

The first letter must be lowercase to match the default autowire naming. If not, the following exception will be seen:

org.osgi.service.blueprint.container.ComponentDefinitionException: Unresolved ref/idref to component: mDSALManager

The solution is to ensure the default naming is used and simply use the class name with first letter lowercase. The consuming classes are free to use any name as the binding information is the interface:

<reference id="iMdsalApiManager"
           interface="org.opendaylight.genius.mdsalutil.interfaces.IMdsalApiManager" />

References