Skip to content

Dev: Antipatterns in webSpoon

Hiromu Hota edited this page Nov 25, 2018 · 2 revisions

Due to the differences between RWT and SWT, there are some anti-patterns that should be avoided when developing webSpoon and (webSpoon-) compatible plugins.

Avoid sharing session-unique instances among sessions

The singleton pattern is a design pattern where only one instance of a class is created. This pattern can be seen here and there in Spoon's source code. An example is GUIResource, which mainly manages colors, fonts, and images. The following code snippet (excerpt of here) illustrates how it is ensured that GUIResource can only be instantiated once. The resources (colors, fonts, and images) are accessed through GUIResource.getInstance().

public class GUIResource {
  private static GUIResource guiResource;

  // Making the constructor private prevents instantiation from outside
  private GUIResource( Display display ) {
  ...
  }

  public static final GUIResource getInstance() {
    if ( guiResource != null ) {
      return guiResource;
    }
    guiResource = new GUIResource( PropsUI.getDisplay() );
    return guiResource;
  }
}

Spoon is another example that takes the singleton pattern (not strictly as it can be instantiated more than once). If you look at the following code snippet (excerpt of here), you would notice that Display is a member field of Spoon. This means that Spoon can only manages a single instance of Display. The singleton pattern for Spoon class is nothing wrong for Spoon that servers only a single user, but makes troubles for webSpoon where Display is instantiated for each session.

public class Spoon extends ApplicationWindow implements AddUndoPositionInterface,
    ..., PartitionSchemasProvider {
  private static Spoon staticSpoon;
  private Display display;
  public static Spoon getInstance() {
    return staticSpoon;
  }
}

Fortunately, RAP/RWT provides SingletonUtil as a remedy. Spoon.getInstance() in the following code will return an instance of Spoon that is unique to a session. As a result, each instance of Display can be referenced by their corresponding instance of Spoon.

public class Spoon {
  private Display display;
  public static Spoon getInstance() {
    return SingletonUtil.getSessionInstance( Spoon.class );
  }
}

The rule-of-thumb is that such a session-unique instance should accessed only by the corresponding session and not by any other. Let's look at examples below how this rule can be violated.

Example: use of the static modifier for session-unique instances

The code below violates the rule, but how?

public class RepositoryOpenSaveDialog extends ThinDialog {
  private static final Image LOGO = GUIResource.getInstance().getImageLogoSmall();
}

Let us assume that GUIResource has already been adapted to RAP/RWT and GUIResource.getInstance() returns a session-unique instance of GUIResource. An instance of Image returned by GUIResource.getInstance().getImageLogoSmall() also becomes session-unique. When RepositoryOpenSaveDialog is instantiated, say in session A, LOGO is instantiated and assigned with an instance of Image unique to session A. By the static modifier, LOGO keeps the reference to that particular instance ever after and returns that instance when accessed in any later sessions.

Sharing session-unique instance of Image is not necessarily harmful especially when the image is inherently common to all sessions (e.g., the kettle small logo). However, issues like #92 happen when the session A gets terminated and the instance of Image is disposed, but accessed by another session. To resolve these issues, remove the static modifier as follows:

public class RepositoryOpenSaveDialog extends ThinDialog {
  private final Image LOGO = GUIResource.getInstance().getImageLogoSmall();
}

Example: caching session-unique instances in plugins

PDI plugins, even in webSpoon, have application scope instead of session scope and only one instance is instantiated for each one of these plugins (= effectively singleton). Let's take a look at a modified snippet of HadoopClusterViewTreeExtension.

public class HadoopClusterViewTreeExtension implements ExtensionPointInterface {
  private Spoon spoon = null;
  private Image hadoopClusterImage = null;

  public HadoopClusterViewTreeExtension() {
    spoon = Spoon.getInstance();
    hadoopClusterImage = getHadoopClusterImage( spoon.getDisplay() );
  }

  private void refreshNamedClusterSubtree( SelectionTreeExtension selectionTreeExtension ) {
    for ( NamedCluster namedCluster : namedClusters ) {
      createTreeItem( tiNcTitle, namedCluster.getName(), hadoopClusterImage );
    }
  }
}

The constructor assigns a session-unique Spoon and (effectively) session-unique Image to its member fields. These member fields could be technically re-assigned but never in Spoon. In webSpoon, this code causes issues such as #23 and a non-reported issue resolved by adbb00a. To resolve the issues, stop caching session-unique instances in the member fields and retrieve them on-demand. The code below is the corrected one.

public class HadoopClusterViewTreeExtension implements ExtensionPointInterface {
  // private Spoon spoon = null;
  // private Image hadoopClusterImage = null;

  public HadoopClusterViewTreeExtension() {
    // spoon = Spoon.getInstance();
    // hadoopClusterImage = getHadoopClusterImage( spoon.getDisplay() );
  }

  private void refreshNamedClusterSubtree( SelectionTreeExtension selectionTreeExtension ) {
    for ( NamedCluster namedCluster : namedClusters ) {
      createTreeItem( tiNcTitle, namedCluster.getName(),
        getHadoopClusterImage( Spoon.getInstance().getDisplay() ) ) );
    }
  }
}

Avoid use of unimplemented SWT APIs

Example: drawing on an Image

SWT can paint on any widgets or image, while RAP/RWT can only paint on Canvas widget. SWT supports the following codes:

  Image image = new Image( device, width, height );
  GC gc = new GC( image );
  gc.drawRectangle( 0, 0, 10, 20 );

but RWT does not. You can see an example of how this limitation has been overcame in pdi-dataservice-server-plugin.