Page History

Motivation

The main motivation behind this refactor work was is to increate the readability/quality of some of the ServerResource interface implementations, like the CitrixResourceBase class source-code. The source-code had way too many lines, with a big if/else block, containing in total 64 ifs, inside the executeRequest() method. Each if condition had a call to an execute() method, most of the times a private one, inside the CitrixResourceBase.

In addition to the rewrite of the private/protected execute() methods, we also wanted to add more unit tests. Below details about before/after number of lines and test coverage.

Number of Lines

• Before: 7340
• After: 4940

Unit Test Coverage

• Module: cloud-plugin-hypervisor-xenserver 
  • Before: 2%
  • After: 9.7%
• Number of Unit Tests
  • Before: 4
  • After: 104

Design

, LibvirtComputingResource and NiciraNvpResource classes. There are several other implementations of ServerResource, but they won't be tackled on the 4.6.0 release of Apache CloudStack.

In order to fulfil our goal a number of points have to be worked on:

• Reduce amount of lines in the implementations.
• Reduce the amount of private methods in the implementations.
• Increase loosely-coupling of the command classes used by the implementations
• Increase unit test coverage

In order to give an idea on how the refactor took place for those ServerResource implementation, some extra documentation, including code snippets and diagrams, is available below. Although the diagrams and code snippets use the CitrixResrouceBase as example, the same approach has been successfully applied to LibvirtComputingResource and NiciraNvpresource. 

Design

The ServerResource.Just like with the other ServerResource implementations (e.g. LibVirt, Bare-metal, etc), the executeRequest() method expects a Command object, which is defined in the Cloud Core module. Since we could not change this structure, in order to avoid a major change in the whole API layer. We , we added a CommandWrapper to deal with the behaviour expected by each ServerResourse sub-class. In that sense, the whole Citrix implementation is XenServer, Libvirt and Nicira implementations are completely independent and doesndon't change anything in the existing commands. Only extensions of the CommandWrapper<T extends Command, A extends Answer, R extends ServerResource> class will be enough to have the commands decoupled from the ServerResource implementation.

Changes in the Cloud Core module

In order to have a smooth refactor, we added the following resources to the Cloud-Core module:

• CommandWrapper<T extends Command, A extends Answer, R extends ServerResource>
  • An abstract class that will be extended by each CommandWrapper implementation.

...

• • This class uses Generics in order to cope with implementations of the following objects only
    • Command
    • Answer
    • ServerResource
• RequestWrapper
  • An abstract class used to hold a map containing the key/value pairs of ServerResources/CommandWrappers.
  • This class is also responsible by retrieving CommandWrappers implementations based on given Commands.
  • Extensions of this class will be responsible by filling the ServerResources/CommandWrappers map.
• ResourceWrapper
  
  • Annotation used to make the CommandWrappers. More details in the Reflections & Annotations section.

By using the 3 objects mentioned above we no longer need the 143  if/else block that were part of the 3 refactored classes; which also led to 143 private methods. Instead, we can simply load a command from the wrapper with the snippet below:

Instead of having 64 ifs to deal with the variety of API commands, no we simply have:

final CitrixRequestWrapper wrapper = CitrixRequestWrapper.getInstance();
try {
    return wrapper.execute(cmd, this);
} catch (final Exception e) {
    return Answer.createUnsupportedCommandAnswer(cmd);
}

The diagram below depicts the relationship between the CitrixResourceBase and the new classes. The same can be applied to LibvirtComputingResource and NiciraNvpResource. Future implementations will follow the same model.

Please keep in mind that It's just an exhibit of what has been done: we did not want to add all the new classes to the diagram.

...

The image is a bit too large, but one can download the original in order to have a better idea of the implementation.

Reflections & Annotations

In order to keep the maintenance of the ServerResource implementations easy, we also added the use of Reflections & Annotations in the current code. What kind of benefits does it bring?

...

@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
public @interface ResourceWrapper {
    Class<? extends Command> handles();
}

Loading CommandWrappers annotated with the ResourceWrapper will need three simple steps:

1. On your RequestWrapper implementation (e.g. CitrixRequestWrapper), do the following:

   1. Load the CommadWrappers that are annotated

      Code Block
      language java theme Eclipse title Load annotations
      Reflections baseWrappers = new Reflections("com.cloud.hypervisor.xenserver.resource.wrapper.xenbase") Set<Class<? extends CommandWrapper>> baseSet = baseWrappers.getSubTypesOf(CommandWrapper.class);

   2. Fill the resources

      Code Block
      language java theme Eclipse title Filling resources
      final Hashtable<Class<? extends Command>, CommandWrapper> citrixCommands = processAnnotations(baseSet); resources.put(CitrixResourceBase.class, citrixCommands);

The resources map is defined in the RequestWrapper abstract class.

When defining In order to define a new wrapper class for a given Command, we have to follow the following:

...