refactor UserSessionService

- use finite color list to show visually distinct color on concurrent users edit
- no mermory leak after migrate from session id to editor client id

zanata-war/src/main/java/org/zanata/webtrans/client/presenter/TargetContentsPresenter.java

@@ -418,14 +418,14 @@ private void updateEditorTranslatorList(TransUnitId selectedTransUnitId, Person
       {
          for (ToggleEditor editor : currentEditors)
          {
-            editor.addTranslator(person.getName(), sessionService.getColor(editorClientId.getValue()));
+            editor.addTranslator(person.getName(), sessionService.getColor(editorClientId));
          }
       }
       else
       {
          for (ToggleEditor editor : currentEditors)
          {
-            editor.removeTranslator(person.getName(), sessionService.getColor(editorClientId.getValue()));
+            editor.removeTranslator(person.getName(), sessionService.getColor(editorClientId));
          }
       }
    }

zanata-war/src/main/java/org/zanata/webtrans/client/presenter/WorkspaceUsersPresenter.java

@@ -187,7 +187,7 @@ public void onSuccess(PublishWorkspaceChatResult result)
 
    public void addTranslator(EditorClientId editorClientId, Person person, TransUnit selectedTransUnit)
    {
-      String color = sessionService.getColor(editorClientId.getValue());
+      String color = sessionService.getColor(editorClientId);
 
       UserPanelSessionItem item = sessionService.getUserPanel(editorClientId);
       if (item == null)
@@ -204,9 +204,4 @@ public void addTranslator(EditorClientId editorClientId, Person person, TransUni
       sessionService.updateTranslatorStatus(editorClientId, selectedTransUnit);
    }
 
-   public int getTranslatorsSize()
-   {
-      return sessionService.getTranslatorsSize();
-   }
-
 }

zanata-war/src/main/java/org/zanata/webtrans/client/service/UserSessionService.java

@@ -21,17 +21,23 @@
 package org.zanata.webtrans.client.service;
 
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 import net.customware.gwt.presenter.client.EventBus;
 
 import org.zanata.webtrans.client.events.TransUnitEditEvent;
 import org.zanata.webtrans.client.events.TransUnitEditEventHandler;
+import org.zanata.webtrans.client.ui.DistinctColor;
 import org.zanata.webtrans.shared.auth.EditorClientId;
 import org.zanata.webtrans.shared.auth.EditorClientId;
 import org.zanata.webtrans.shared.model.TransUnit;
 import org.zanata.webtrans.shared.model.UserPanelSessionItem;
 
+import com.allen_sauer.gwt.log.client.Log;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
 import com.google.gwt.canvas.dom.client.CssColor;
 import com.google.gwt.user.client.Random;
 import com.google.inject.Inject;
@@ -45,18 +51,13 @@
 public class UserSessionService implements TransUnitEditEventHandler
 {
    private final HashMap<EditorClientId, UserPanelSessionItem> userSessionMap;
-
-   public final HashMap<String, String> colorListMap;
-
-   private static final int DARK_BOUND = 100;
-   private static final int BRIGHT_BOUND = 400;
+   private final DistinctColor distinctColor;
 
    @Inject
-   public UserSessionService(final EventBus eventBus)
+   public UserSessionService(final EventBus eventBus, DistinctColor distinctColor)
    {
-      userSessionMap = new HashMap<EditorClientId, UserPanelSessionItem>();
-
-      colorListMap = new HashMap<String, String>();
+      this.distinctColor = distinctColor;
+      userSessionMap = Maps.newHashMap();
 
       eventBus.addHandler(TransUnitEditEvent.getType(), this);
    }
@@ -75,11 +76,6 @@ public void updateTranslatorStatus(EditorClientId editorClientId, TransUnit sele
       }
    }
 
-   public int getTranslatorsSize()
-   {
-      return userSessionMap.size();
-   }
-
    public UserPanelSessionItem getUserPanel(EditorClientId editorClientId)
    {
       return userSessionMap.get(editorClientId);
@@ -93,53 +89,16 @@ public void addUser(EditorClientId editorClientId, UserPanelSessionItem item)
    public void removeUser(EditorClientId editorClientId)
    {
       userSessionMap.remove(editorClientId);
-      // FIXME remove from colorListMap (memory leak)
+      distinctColor.releaseColor(editorClientId);
    }
 
    public Map<EditorClientId, UserPanelSessionItem> getUserSessionMap()
    {
       return userSessionMap;
    }
 
-   // TODO what we pass is really editorClientId
-   // Should we be passing sessionId?  See also memory leak above.
-   public String getColor(String sessionId)
-   {
-      if (colorListMap.containsKey(sessionId))
-      {
-         return colorListMap.get(sessionId);
-      }
-
-      String color = null;
-
-      while (colorListMap.containsValue(color) || color == null || color.equals("rgb(0,0,0)") || color.equals("rgb(255,255,255)"))
-      {
-         color = generateNewColor();
-      }
-
-      colorListMap.put(sessionId, color);
-
-      return color;
-   }
-
-   private String generateNewColor()
+   public String getColor(EditorClientId editorClientId)
    {
-      int total = 0;
-
-      int rndRedColor = 0;
-      int rndGreenColor = 0;
-      int rndBlueColor = 0;
-
-      while (total < DARK_BOUND || total > BRIGHT_BOUND)
-      {
-         rndRedColor = Random.nextInt(255) / 2;
-         rndGreenColor = Random.nextInt(255);
-         rndBlueColor = Random.nextInt(255);
-         total = rndRedColor + rndGreenColor + rndBlueColor;
-      }
-
-      CssColor randomColor = CssColor.make(rndRedColor, rndGreenColor, rndBlueColor);
-      return randomColor.value();
+      return distinctColor.getOrCreateColor(editorClientId);
    }
-
 }

zanata-war/src/main/java/org/zanata/webtrans/client/ui/DistinctColor.java

@@ -0,0 +1,12 @@
+package org.zanata.webtrans.client.ui;
+
+import org.zanata.webtrans.shared.auth.EditorClientId;
+import com.google.inject.ImplementedBy;
+
+@ImplementedBy(DistinctColorListImpl.class)
+public interface DistinctColor
+{
+   String getOrCreateColor(EditorClientId editorClientId);
+
+   void releaseColor(EditorClientId editorClientId);
+}

zanata-war/src/main/java/org/zanata/webtrans/client/ui/DistinctColorListImpl.java

@@ -0,0 +1,96 @@
+package org.zanata.webtrans.client.ui;
+
+import java.util.HashMap;
+import java.util.List;
+
+import org.zanata.webtrans.shared.auth.EditorClientId;
+import com.allen_sauer.gwt.log.client.Log;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Maps;
+import com.google.gwt.canvas.dom.client.CssColor;
+import com.google.inject.Singleton;
+
+/**
+ * Referencing from http://eleanormaclure.files.wordpress.com/2011/03/colour-coding.pdf.
+ * We store 22 visually distinct color and cycle through them.
+ * If we ever get more than 22 concurrent users on one row, we will have two users share same color.
+ *
+ * @author Patrick Huang <a href="mailto:pahuang@redhat.com">pahuang@redhat.com</a>
+ */
+@Singleton
+public class DistinctColorListImpl implements DistinctColor
+{
+   private int index = 0;
+   private List<String> colorList;
+   private final HashMap<EditorClientId, String> colorMap;
+
+
+   public DistinctColorListImpl()
+   {
+      // @formatter:off
+      colorList = ImmutableList.<String>builder()
+            .add(distinctColor(240, 163, 255))
+            .add(distinctColor(0, 117, 220))
+            .add(distinctColor(153, 63, 0))
+            .add(distinctColor(76, 0, 92))
+            .add(distinctColor(25, 25, 25))
+            .add(distinctColor(0, 92, 49))
+            .add(distinctColor(43, 206, 72))
+            .add(distinctColor(255, 204, 153))
+            .add(distinctColor(128, 128, 128))
+            .add(distinctColor(148, 255, 181))
+            .add(distinctColor(143, 124, 0))
+            .add(distinctColor(157, 204, 0))
+            .add(distinctColor(194, 0, 136))
+            .add(distinctColor(0, 51, 128))
+            .add(distinctColor(255, 164, 5))
+            .add(distinctColor(66, 102, 0))
+            .add(distinctColor(255, 0, 16))
+            .add(distinctColor(94, 241, 242))
+            .add(distinctColor(0, 153, 143))
+            .add(distinctColor(224, 255, 102))
+            .add(distinctColor(116, 10, 255))
+            .add(distinctColor(153, 0, 0))
+            .add(distinctColor(255, 255, 128))
+            .add(distinctColor(255, 255, 0))
+            .add(distinctColor(255, 80, 5))
+            .build();
+      // @formatter:on
+      colorMap = Maps.newHashMap();
+   }
+
+   private static String distinctColor(int rndRedColor, int rndGreenColor, int rndBlueColor)
+   {
+      return CssColor.make(rndRedColor, rndGreenColor, rndBlueColor).value();
+   }
+
+   @Override
+   public String getOrCreateColor(EditorClientId editorClientId)
+   {
+      if (colorMap.containsKey(editorClientId))
+      {
+         return colorMap.get(editorClientId);
+      }
+
+      String color = nextColor();
+      colorMap.put(editorClientId, color);
+      Log.info("put new color for [" + editorClientId.hashCode() + "]" + color);
+      return color;
+   }
+
+   @Override
+   public void releaseColor(EditorClientId editorClientId)
+   {
+      colorMap.remove(editorClientId);
+   }
+
+   private String nextColor()
+   {
+      if (index == colorList.size())
+      {
+         index = 0;
+      }
+      return colorList.get(index++);
+   }
+
+}

zanata-war/src/test/java/org/zanata/webtrans/client/presenter/WorkspaceUsersPresenterTest.java

@@ -169,9 +169,9 @@ public void setNonEmptyUserList()
       UserPanelSessionItem mockItem2 = new UserPanelSessionItem(mockPanel2, person2);
       UserPanelSessionItem mockItem3 = new UserPanelSessionItem(mockPanel3, person3);
 
-      expect(mockSessionService.getColor(editorClientId1.getValue())).andReturn("color1");
-      expect(mockSessionService.getColor(editorClientId2.getValue())).andReturn("color2");
-      expect(mockSessionService.getColor(editorClientId3.getValue())).andReturn("color3");
+      expect(mockSessionService.getColor(editorClientId1)).andReturn("color1");
+      expect(mockSessionService.getColor(editorClientId2)).andReturn("color2");
+      expect(mockSessionService.getColor(editorClientId3)).andReturn("color3");
 
       expect(mockSessionService.getUserPanel(editorClientId1)).andReturn(mockItem1);
       expect(mockSessionService.getUserPanel(editorClientId2)).andReturn(mockItem2);

zanata-war/src/test/java/org/zanata/webtrans/client/service/ValidationServiceTest.java

@@ -37,6 +37,12 @@ public class ValidationServiceTest
    public void beforeMethod()
    {
       MockitoAnnotations.initMocks(this);
+      when(valMessages.printfVariablesValidatorName()).thenReturn("printf");
+      when(valMessages.positionalPrintfVariablesValidatorName()).thenReturn("positional");
+      when(valMessages.javaVariablesValidatorName()).thenReturn("java");
+      when(valMessages.xmlHtmlValidatorName()).thenReturn("xmlhtml");
+      when(valMessages.xmlEntityValidatorName()).thenReturn("entity");
+
       service = new ValidationService(eventBus, messages, valMessages);
 
       when(messages.notifyValidationError()).thenReturn("validation error");
@@ -49,11 +55,13 @@ public void beforeMethod()
    @Test
    public void onValidate()
    {
+      when(valMessages.varsAdded(Lists.newArrayList("%s"))).thenReturn("var added %s");
       RunValidationEvent event = new RunValidationEvent("source", "target %s", false);
       event.addWidget(validationMessagePanel);
+
       service.onValidate(event);
 
-      verify(validationMessagePanel).updateValidationWarning(Lists.<String>newArrayList());
+      verify(validationMessagePanel).updateValidationWarning(Lists.newArrayList("var added %s"));
    }
 
 }