Multiple entries with same key immutable map error

Question

I have a below builder class which I am using from multithread application so I have made it thread safe. Just for simplicity, I am showing only few fields here to demonstrate the problem.

public final class ClientKey {
  private final long userId;
  private final int clientId;
  private final String processName;
  private final Map<String, String> parameterMap;

  private ClientKey(Builder builder) {
    this.userId = builder.userId;
    this.clientId = builder.clientId;
    this.processName = builder.processName;
    // initializing the required fields
    // and below line throws exception once I try to clone the `ClientKey` object
    builder.parameterMap.put("is_clientid", (clientId == 0) ? "false" : "true");
    this.parameterMap = builder.parameterMap.build();
  }

  public static class Builder {
    private final long userId;
    private final int clientId;
    private String processName;
    private ImmutableMap.Builder<String, String> parameterMap = ImmutableMap.builder();

    // this is for cloning
    public Builder(ClientKey key) {
      this.userId = key.userId;
      this.clientId = key.clientId;
      this.processName = key.processName;
      this.parameterMap =
          ImmutableMap.<String, String>builder().putAll(key.parameterMap);
    }

    public Builder(long userId, int clientId) {
      this.userId = userId;
      this.clientId = clientId;
    }

    public Builder parameterMap(Map<String, String> parameterMap) {
      this.parameterMap.putAll(parameterMap);
      return this;
    }

    public Builder processName(String processName) {
      this.processName = processName;
      return this;
    }

    public ClientKey build() {
      return new ClientKey(this);
    }
  }

  // getters
}

Below is how I make ClientKey and it works fine.

Map<String, String> testMap = new HashMap<String, String>();
testMap.put("hello", "world");
ClientKey keys = new ClientKey.Builder(12345L, 200).parameterMap(testMap).build();

Now when I try to clone the keys object as shown below, it throws exception.

ClientKey clonedKey = new ClientKey.Builder(keys).processName("hello").build();

It throws exception with error message as: java.lang.IllegalArgumentException: Multiple entries with same key: is_clientid=true and is_clientid=true

builder.parameterMap.put("is_clientid", (clientId == 0) ? "false" : "true");
// from below line exception is coming
this.parameterMap = builder.parameterMap.build();

How can I fix this problem? I want to make my map immutable but I also want to initialize with required fields as well and that I can only do it in the constructor of ClientKey class. And it throws exception while cloning the ClientKey object.

Answer 1

When you construct a ClientKey, the "is_clientid" key is put in the map. Therefore, if you call your ClientKey.Builder(ClientKey) constructor the putAll call will copy it to your new ImmutableMap.Builder instance. When you then build your cloned ClientKey, the ClientKey constructor will again try to add the same key to the map, which causes the exception.

The ImmutableMap.Builder could have been written in a different way, but it wasn't. If you want to use it, you'll have to live with it.

One solution is to not copy the entry with the "is_clientid" key to the new ImmutableMap.Builder in the constructor of your Builder. Instead of this.parameterMap = ImmutableMap.<String, String>builder().putAll(key.parameterMap); you write:

this.parameterMap = new ImmutableMap.Builder<>();
for (Map.Entry<String,String> entry : key.parameterMap.entrySet()) {
    if (!"is_clientid".equals(entry.getKey()) {
        this.parameterMap.put(entry.getKey(), entry.getValue());
    }
}

Another solution is to not use Guava's ImmutableMap.Builder, but a normal Java HashMap (it does not throw exception when you try to put a duplicate key in it, the old entry is simply overwritten). Then in your ClientKey constructor you write:

this.parameterMap = Collections.unmodifiableMap(builder.parameterMap);

You could also write:

this.parameterMap = ImmutableMap.copyOf(builder.parameterMap);

but this makes an entire copy of the map, which may take some time for very large maps.

A concluding remark: if all you want to do is copy a ClientKey, you do not need a builder; idiomatic Java would use a copy constructor or the clone() method (although the latter is discouraged by some).