Configuration concept and implementation

Question

I created an API to load, save and read user configuration in my application. I've a Configuration interface which provides the basic methods to read, save and read configuration data and then in the application I've the implementation JsonConfiguration.

I tried to keep the Configuration interface as generic as possible. So I didn't make any assumption about how the settings are stored, it can be plain text as well as something else.

This is my Configuration interface:

public interface Configuration {
  /**
   * Called when the configuration should load the settings
   *
   * @param inputStream In this stream you can found the settings of
   *                    the user.
   *                    An implementation can decide what is inside this
   *                    stream.
   *                    Example: If an implementation
   *                    uses JSON to save and read settings, it should
   *                    assume that this stream contains a valid json.
   *                    If it doesn't, it is allowed to throw
   *                    any exception since it's not
   *                    their job to convert the stream to your
   *                    format.
   *                    Do not close the stream. You don't own it.
   */
  void load(@NotNull final InputStream inputStream);

  /**
   * Called when the configuration should save ALL the settings
   *
   * @param outputStream Write in this stream the settings of the
   *                     user in the form of the implementation.
   *                     Do not close the stream. You don't own it.
   *
   * @throws IOException Throw if something went wrong during the
   *                     save process
   */
  void save(@NotNull final OutputStream outputStream) throws IOException;

  /**
   * @param name The setting name
   * @return The setting as integer (if it's an integer)
   *         or NumberFormatException if not
   */
  @NotNull
  OptionalInt getAsInt(@NotNull final String name);

  @NotNull
  OptionalDouble getAsDouble(@NotNull final String name);

  @NotNull
  OptionalLong getAsLong(@NotNull final String name);

  @NotNull
  Optional<String> getAsString(@NotNull final String name);

  @NotNull
  Optional<Boolean> getAsBoolean(@NotNull final String name);

  /**
   * Set the value
   *
   * @param name The name of the setting
   * @param value The value to save
   * @param <T> The type of the value
   */
  <T> void set(@NotNull final String name, @NotNull final T value);
}

(I've removed documentation from getAsX since it's the same.) (Yes, I know there is no getAsFloat, I will add it after the review.)

As you can see, the concept of save/load and read are mixed in the same interface. My first question is, should I move the two concepts in two interfaces? Maybe using a Factory to load and save the settings and Configuration will only handle the reading of the data.

This is the implementation, JsonConfiguration, which stores data in a JSON format:

public class JsonConfiguration implements Configuration {
  private Map<String, ConfigurationSection> configurations = new HashMap<>();

  @Override
  public void load(@NotNull final InputStream inputStream) {
    final JsonParser parser = new JsonParser();
    final JsonObject root = parser.parse(new InputStreamReader(inputStream)).getAsJsonObject();

    for (final Map.Entry<String, JsonElement> entry : root.entrySet()) {
      final String confName = entry.getKey();

      final ConfigurationSection configuration = new JsonConfigurationSection();
      configuration.load(new ByteArrayInputStream(
              entry
                  .getValue()
                  .toString()
                  .getBytes(StandardCharsets.UTF_8))
      );

      configurations.put(confName, configuration);
    }
  }

  @Override
  public void save(@NotNull final OutputStream outputStream) throws IOException {
    final Type type = new TypeToken<Map<String, Configuration>>() {}.getType();
    final OutputStreamWriter out = new OutputStreamWriter(outputStream);
    final JsonWriter writer = new JsonWriter(out);

    new GsonBuilder()
        .registerTypeAdapter(JsonConfigurationSection.class, new JsonConfigurationSectionSerializer())
        .create()
        .toJson(configurations, type, writer);

    writer.flush();
    out.flush();
  }

  @NotNull
  private <T> Optional<T> get(@NotNull final String name) {
    final int dotSeparatorPosition = name.indexOf('.');

    if (dotSeparatorPosition == -1) {
      throw new IllegalArgumentException(name + " is not a correct user preference setting name");
    }

    final String section = name.substring(0, dotSeparatorPosition);
    final String element = name.substring(dotSeparatorPosition + 1);

    if (!configurations.containsKey(section)) {
      throw new IllegalArgumentException("Section " + section + " doesn't exists.");
    }

    return configurations.get(section).get(element);
  }

  @NotNull
  @Override
  public OptionalInt getAsInt(@NotNull final String name) {
    final Optional<String> optional = get(name);
    return !optional.isPresent() ? OptionalInt.empty() : OptionalInt.of(Integer.parseInt(optional.get()));
  }

  @NotNull
  @Override
  public OptionalDouble getAsDouble(@NotNull final String name) {
    final Optional<String> optional = get(name);
    return !optional.isPresent() ? OptionalDouble.empty() : OptionalDouble.of(Double.parseDouble(optional.get()));
  }

  @NotNull
  @Override
  public OptionalLong getAsLong(@NotNull final String name) {
    final Optional<String> optional = get(name);
    return !optional.isPresent() ? OptionalLong.empty() : OptionalLong.of(Long.parseLong(optional.get()));
  }

  @NotNull
  @Override
  public Optional<String> getAsString(@NotNull final String name) {
    return get(name);
  }

  @NotNull
  @Override
  public Optional<Boolean> getAsBoolean(@NotNull final String name) {
    final Optional<String> optional = get(name);
    return !optional.isPresent() ? Optional.empty() : Optional.of(Boolean.parseBoolean(optional.get()));
  }

  /**
   * Sets the value of a section. The name
   * is composed of: sectionName.entryName
   *
   * If the section doesn't exists it will
   * be created.
   *
   * If the entryName doesn't exists it will
   * be created.
   *
   * @param name The name of the setting
   * @param value The value to save
   * @param <T> The type of the value
   * @throws IllegalArgumentException If the name passed is not of the format: sectionName.entryName
   */
  @Override
  public <T> void set(@NotNull final String name,
                      @NotNull final T value) {
    final int dotSeparatorPosition = name.indexOf('.');

    if (dotSeparatorPosition == -1) {
      throw new IllegalArgumentException(name + " is not a correct user preference setting name");
    }

    final String sectionName = name.substring(0, dotSeparatorPosition);
    final String element = name.substring(dotSeparatorPosition + 1);

    ConfigurationSection section = configurations.get(sectionName);

    if (section == null) {
      section = new JsonConfigurationSection();

      configurations.put(sectionName, section);
    }

    section.set(element, value.toString());
  }
}

Since I don't make any assumption in the Configuration interface, the concept of "sections" exists only in the implementation and the user access an entry in a section using the notation sectionName.entryName.

The interface ConfigurationSection is without documentation:

public interface ConfigurationSection {
  void load(@NotNull final InputStream inputStream);
  void save(@NotNull final OutputStream outputStream) throws IOException;
  <T> Optional<T> get(@NotNull final String name);
  <T> void set(@NotNull final String name, @NotNull final T value);
}

and the implementation:

public class JsonConfigurationSection implements ConfigurationSection {
  @NotNull
  private ConcurrentMap<String, Object> values = new ConcurrentHashMap<>();

  @Override
  public void load(@NotNull final InputStream inputStream) {
    values = new ConcurrentHashMap<>();

    final JsonParser parser = new JsonParser();
    final JsonObject root = parser.parse(new InputStreamReader(inputStream)).getAsJsonObject();

    for (final Map.Entry<String, JsonElement> entry : root.entrySet()) {
      final String key = entry.getKey();
      final JsonElement value = entry.getValue();

      if (value.isJsonPrimitive()) {
        final JsonPrimitive primitive = value.getAsJsonPrimitive();
        values.put(key, primitive.getAsString());
      } else if (value.isJsonNull()) {
        throw new IllegalArgumentException("null is not a valid parameter");
      } else if (value.isJsonArray()) {
        throw new UnsupportedOperationException("Arrays not supported yet");
      } else if (value.isJsonObject()) {
        throw new UnsupportedOperationException("Objects not supported yet");
      }
    }
  }

  @Override
  public void save(@NotNull final OutputStream outputStream) throws IOException {
    final Type type = new TypeToken<Map<String, Object>>() {}.getType();
    final OutputStreamWriter out = new OutputStreamWriter(outputStream, StandardCharsets.UTF_8);
    final JsonWriter writer = new JsonWriter(out);

    new Gson().toJson(values, type, writer);

    writer.flush();
    out.flush();
  }

  @NotNull
  @Override
  public <T> Optional<T> get(@NotNull final String name) {
    return Optional.ofNullable((T) values.get(name));
  }

  @Override
  public <T> void set(@NotNull final String name,
                      @Nullable final T value) {
    values.put(name, value);
  }

  @NotNull
  Map<String, Object> getValues() {
    return Collections.unmodifiableMap(values);
  }
}

As you can see from load everything is stored as String in the map (so yes, values can be changed from <String, Object> to <String, String> but my plan is to support arrays and objects soon... so).

ConfigurationSection interface and Configuration are pretty similar as interfaces, but have different concepts.

My application has a Application interface with a getConfiguration method so Plugins/Application use it to access a Configuration object and read settings. The fact that the concept of save/load is of the Boot class only (which prepare/load etc the application) it could be an incentive to use a factory instead of the current interface (so I hide the two things).

I'm planning to add a Transformer concept to convert a configuration format to another, but it's just an idea for now.

My questions:

1. As I said, the concepts save/load and read are mixed together in one interface, make sense separate them and implement a Factory to load and save settings?
2. I want to remove the throws IOException from save because it's not consistent with load which doesn't throw IOException.
3. Can I improve the load of JsonConfigurationSection? The ifs seems okay, but maybe could be improved.
4. Any comment? I've read how other languages/framework does it but...

Answer 1

1. As I said, the concepts save/load and read are mixed together in one interface, make sense separate them and implement a Factory to load and save settings?

Wells, Java's Properties provide similar load/store functionality as well, so I think this is fine.

2. I want to remove the throws IOException from save because it's not consistent with load which doesn't throw IOException.

As mentioned by yourself in the lengthy comments, a custom Exception class foremost may be more helpful to wrap underlying reasons such as IOException (or SQLException, UnknownHostException etc.). The question then becomes whether you want to stick with checked Exception classes or not.

If you prefer checked Exception classes, then I think both save/load methods should throw it for consistency. As helpfully explained here, here and here, checked Exceptions may be used to indicate to the method's callers that they should be able to reasonably recover from these scenarios. For your implementation, I think a checked, custom Exception class has its merits for users of your library to explicitly handle cases where their configuration could not be initialized properly.

3. Can I improve the load of JsonConfigurationSection? The ifs seems okay, but maybe could be improved.

You may want to consider the fail-fast approach here:

for (final Map.Entry<String, JsonElement> entry : root.entrySet()) {
    final JsonElement value = entry.getValue();
    if (value.isJsonNull()) {
        throw new IllegalArgumentException("null is not a valid parameter");
    } else if (value.isJsonArray()) {
        throw new UnsupportedOperationException("Arrays not supported yet");
    } else if (value.isJsonObject()) {
        throw new UnsupportedOperationException("Objects not supported yet");
    } else if (value.isJsonPrimitive()) {
        // note: inlined entry.getKey() and value.getAsJsonPrimitive()
        values.put(entry.getKey(), value.getAsJsonPrimitive().getAsString());
    }
}

It may not be apparent now, but once your library gains new features, e.g. for array/object support, you can gradually remove the checks from the top and append the implementations (or preferably method calls to the new features) below.

4. Any comment? I've read how other languages/framework does it but...

When you are converting your Optional<String> instance to one of the OptionalInt/OptionalLong, you can rely on the map().orElse() chained methods to better convey the conversion:

return optional.map(v -> OptionalInt.of(Integer.parseInt(v)))
                .orElse(OptionalInt.empty());

I also don't think you really need an Optional<Boolean> case... do you really need a tri-state configuration choice, true, false and null? Wouldn't it be easier to just say true or false?

You also mentioned...

Since I don't make any assumption in the Configuration interface, the concept of "sections" exists only in the implementation...

Do you intend to support a tree hierarchy of configuration in future versions? That may be worth pondering about...