Skip to content

Instantly share code, notes, and snippets.

@anuchandy
Last active May 4, 2020 21:45
Show Gist options
  • Save anuchandy/59ac7c338d3cfee746937f1ba3caa3d7 to your computer and use it in GitHub Desktop.
Save anuchandy/59ac7c338d3cfee746937f1ba3caa3d7 to your computer and use it in GitHub Desktop.

@JonathanGiles, @srnagar, @alzimmermsft, and I were having some discussion on restricting users from instantiating a category of model classes. This writeup summarizes the discussion so far and the proposal.

The general pattern we follow across client SDK is:

  1. The client classes are in com.azure.<service> package.
  2. The model classes are in com.azure.<service>.models package.

There are cases wherein we want to hide specific methods in models from the user, for example,

  1. Hiding constructor and certain setters of the model.
  2. Entirely immutable models.
  3. Hiding a subset of getters.

It's easy to make a model pure immutable if the serializer is the only one instantiating that model. Since serializer uses reflection to create and populate model properties, we can make the Ctr private and have no setters in it.

Below is such a model that is pure immutable.

public final class Person {
    @JsonProperty(value = "name", required = true)
    private String name;
    @JsonProperty(value = "age", required = true)
    private Integer age;

    private Person() {}

    public String getName() {
        return name;
    }

    public Integer getAge() {
        return age;
    }
}

Many models in swagger have all properties declared as read-only; above representation can be used for such models. These models cannot be directly created by SDK client types or by the user.

There is another set of models that SDK client types want to instantiate directly (i.e., using the new operator and without reflection), but we don't want users to instantiate these models.

Or in general, we want to hide certain methods (the constructor is one of them) from the user. To simplify the discussion below we refer only the constructor method but the same is applicable for any methods.

Since the client types are in com.azure.<service> package and model types are in com.azure.<service>.models package, today the model constructors have public visibility so that client type can create them.

We want to explore the options available so that we can restrict users from instantiating these models.

There are two options we explored:

Option1:

Move these models to the same package as client type i.e., com.azure.<service> package and have a package-private constructor. This approach has the following drawbacks:

  1. It will pollute the package, and SDK implementors/contributors will have a hard time navigating through the code.
  2. Customers will see that some model types coming from com.azure.<service>.models while few from com.azure.<service>.
  3. The Javadoc of model types is not centralized in one place.

Option2:

Declare these models as a contract - either as an interface or abstract class and keep them in com.azure.<service>.models package. We will have the implementation of such a contract.

interface is preferred over abstract class given it is clean and easy to mock by test frameworks.

The implementation of these models can reside:

  1. Either in com.azure.<service>.implementation.models package, with the public constructor.
  2. Or in com.azure.<service> with package-private constructor.

The preferred option is 1; this the pattern we've been following for client types as well i.e., all implementations stay under implementation package.

The implementation class name should have a suffixed Impl. For example: If the model type name is PageBlobItem, then the implementation will be PageBlobItemImpl.

example:

Contract type that user consumes:
// com.azure.storage.blob.models

public interface PageBlobItem {
  String getFirst();
  String getSecond();
 }
Internal implementation of the contract:
// com.azure.storage.blob.implementation.models

final class PageBlobItemImpl implements PageBlobItem {
  private final String first;
  private final String second;
 
  PageBlobItemImpl(String first, String second) {
     this.first = first;
     this.second = second;
  }
 
  @Override
  public String getFirst() { return first; }
 
  @Override
  public String getSecond() { return second; }
}

Proposal:

The proposed solution is option 2.

Final decision:

We agreed on option 2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment