Global and game chats have distinct flood controls.

Split the configuration value into two. Add a tracking object to ChatFilter per user which currently contains the two different last message times. Removed the last message times from the User object. Show chat error messages only in the tab that caused them, not both tabs.

WebContent/js/cah.ajax.handlers.js

@@ -128,6 +128,12 @@ cah.ajax.SuccessHandlers[cah.$.AjaxOperation.CHAT] = function(data) {
   // pass
 };
 
+cah.ajax.ErrorHandlers[cah.$.AjaxOperation.CHAT] = function(data, req) {
+  cah.log.status_with_game(req[cah.$.AjaxRequest.GAME_ID], "Error: "
+      + cah.$.ErrorCode_msg[data[cah.$.AjaxResponse.ERROR_CODE]], "error")
+};
+cah.ajax.ErrorHandlers[cah.$.AjaxOperation.GAME_CHAT] = cah.ajax.ErrorHandlers[cah.$.AjaxOperation.CHAT];
+
 cah.ajax.SuccessHandlers[cah.$.AjaxOperation.GAME_CHAT] = function(data) {
   // pass
 };

build.properties.example

@@ -10,10 +10,15 @@ pyx.insecure_id_allowed=true
 pyx.id_code_salt=
 # comma-separated listed of IP addresses (v4 or v6) from which users are considered admins.
 pyx.admin_addrs=127.0.0.1,::1
-# this many messages in that many seconds is considered chatting too fast.
-pyx.flood_count=4
+# this many messages to global chat in that many seconds is considered chatting too fast.
+pyx.global_flood_count=3
 # seconds
-pyx.flood_time=30
+pyx.global_flood_time=25
+# same but for game chats
+pyx.game_flood_count=5
+# seconds
+pyx.game_flood_time=30
+
 
 # for production use, use postgres
 #hibernate.dialect=org.hibernate.dialect.PostgreSQLDialect

src/main/filtered-resources/WEB-INF/pyx.properties

@@ -6,8 +6,10 @@ pyx.server.broadcast_connects_and_disconnects=${pyx.broadcast_connects_and_disco
 pyx.server.global_chat_enabled=${pyx.global_chat_enabled}
 pyx.server.id_code_salt=${pyx.id_code_salt}
 pyx.server.admin_addrs=${pyx.admin_addrs}
-pyx.chat.flood_count=${pyx.flood_count}
-pyx.chat.flood_time=${pyx.flood_time}
+pyx.chat.global.flood_count=${pyx.global_flood_count}
+pyx.chat.global.flood_time=${pyx.global_flood_time}
+pyx.chat.game.flood_count=${pyx.game_flood_count}
+pyx.chat.game.flood_time=${pyx.game_flood_time}
 pyx.build=${buildNumber}
 
 # this is NOT allowed to be changed during a reload, as metrics depend on previous events

src/main/java/net/socialgamer/cah/data/User.java

@@ -25,10 +25,7 @@ package net.socialgamer.cah.data;
 
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.Date;
-import java.util.LinkedList;
-import java.util.List;
 import java.util.concurrent.PriorityBlockingQueue;
 
 import javax.annotation.Nullable;
@@ -77,8 +74,6 @@ public class User {
 
   private final ReadableUserAgent agent;
 
-  private final List<Long> lastMessageTimes = Collections.synchronizedList(new LinkedList<Long>());
-
   /**
    * Reset when this user object is no longer valid, most likely because it pinged out.
    */
@@ -342,8 +337,4 @@ public class User {
       currentGame = null;
     }
   }
-
-  public List<Long> getLastMessageTimes() {
-    return lastMessageTimes;
-  }
 }

src/main/java/net/socialgamer/cah/util/ChatFilter.java

@@ -23,7 +23,13 @@
 
 package net.socialgamer.cah.util;
 
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
 import java.util.Properties;
+import java.util.TreeMap;
+import java.util.WeakHashMap;
 import java.util.concurrent.TimeUnit;
 
 import org.apache.log4j.Logger;
@@ -47,49 +53,55 @@ public class ChatFilter {
   private static final long DEFAULT_CHAT_FLOOD_TIME = TimeUnit.SECONDS.toMillis(30);
 
   private final Provider<Properties> propsProvider;
+  private final Map<User, FilterData> filterData = Collections.synchronizedMap(new WeakHashMap<>());
 
   public enum Result {
     OK, TOO_FAST, TOO_LONG, NO_MESSAGE
   }
 
+  private enum Scope {
+    global, game
+  }
+
   @Inject
   public ChatFilter(final Provider<Properties> propsProvider) {
     this.propsProvider = propsProvider;
   }
 
   public Result filterGlobal(final User user, final String message) {
-    final Result result = filterCommon(user, message);
+    final Result result = filterCommon(Scope.global, user, message);
     if (Result.OK != result) {
       return result;
     }
 
     // TODO
-    user.getLastMessageTimes().add(System.currentTimeMillis());
+
+    getMessageTimes(user, Scope.global).add(System.currentTimeMillis());
     return result;
   }
 
   public Result filterGame(final User user, final String message) {
-    final Result result = filterCommon(user, message);
+    final Result result = filterCommon(Scope.game, user, message);
     if (Result.OK != result) {
       return result;
     }
 
     // TODO
-    user.getLastMessageTimes().add(System.currentTimeMillis());
+
+    getMessageTimes(user, Scope.game).add(System.currentTimeMillis());
     return result;
   }
 
-  private Result filterCommon(final User user, final String message) {
+  private Result filterCommon(final Scope scope, final User user, final String message) {
     // TODO
 
-    // Intentionally leaving flood protection as per-user, rather than
-    // changing it to per-user-per-game.
-    if (user.getLastMessageTimes().size() >= getFloodCount()) {
-      final Long head = user.getLastMessageTimes().get(0);
-      if (System.currentTimeMillis() - head < getFloodTime()) {
+    final List<Long> messageTimes = getMessageTimes(user, scope);
+    if (messageTimes.size() >= getFloodCount(scope)) {
+      final Long head = messageTimes.get(0);
+      if (System.currentTimeMillis() - head < getFloodTime(scope)) {
         return Result.TOO_FAST;
       }
-      user.getLastMessageTimes().remove(0);
+      messageTimes.remove(0);
     }
 
     if (message.length() > Constants.CHAT_MAX_LENGTH) {
@@ -101,25 +113,52 @@ public class ChatFilter {
     return Result.OK;
   }
 
-  private int getFloodCount() {
+  private int getFloodCount(final Scope scope) {
     try {
-      return Integer.parseInt(propsProvider.get().getProperty("pyx.chat.flood_count",
+      return Integer.parseInt(propsProvider.get().getProperty(
+          String.format("pyx.chat.%s.flood_count", scope),
           String.valueOf(DEFAULT_CHAT_FLOOD_MESSAGE_COUNT)));
     } catch (final NumberFormatException e) {
-      LOG.warn(String.format("Unable to parse pyx.chat.flood_count as a number,"
-          + " using default of %d", DEFAULT_CHAT_FLOOD_MESSAGE_COUNT), e);
+      LOG.warn(String.format("Unable to parse pyx.chat.%s.flood_count as a number,"
+          + " using default of %d", scope, DEFAULT_CHAT_FLOOD_MESSAGE_COUNT), e);
       return DEFAULT_CHAT_FLOOD_MESSAGE_COUNT;
     }
   }
 
-  private long getFloodTime() {
+  private long getFloodTime(final Scope scope) {
     try {
       return TimeUnit.SECONDS.toMillis(Integer.parseInt(propsProvider.get().getProperty(
-          "pyx.chat.flood_time", String.valueOf(DEFAULT_CHAT_FLOOD_TIME))));
+          String.format("pyx.chat.%s.flood_time", scope),
+          String.valueOf(DEFAULT_CHAT_FLOOD_TIME))));
     } catch (final NumberFormatException e) {
-      LOG.warn(String.format("Unable to parse pyx.chat.flood_time as a number,"
-          + " using default of %d", DEFAULT_CHAT_FLOOD_TIME), e);
+      LOG.warn(String.format("Unable to parse pyx.chat.%s.flood_time as a number,"
+          + " using default of %d", scope, DEFAULT_CHAT_FLOOD_TIME), e);
       return DEFAULT_CHAT_FLOOD_TIME;
     }
   }
+
+  private List<Long> getMessageTimes(final User user, final Scope scope) {
+    FilterData data;
+    synchronized (filterData) {
+      data = filterData.get(user);
+      // we should only have to do this once per user...
+      if (null == data) {
+        LOG.trace(String.format("Created new FilterData for user %s", user.getNickname()));
+        data = new FilterData();
+        filterData.put(user, data);
+      }
+    }
+    return data.lastMessageTimes.get(scope);
+  }
+
+  private static class FilterData {
+    final Map<Scope, List<Long>> lastMessageTimes;
+
+    private FilterData() {
+      final Map<Scope, List<Long>> map = new TreeMap<>();
+      map.put(Scope.global, Collections.synchronizedList(new LinkedList<>()));
+      map.put(Scope.game, Collections.synchronizedList(new LinkedList<>()));
+      lastMessageTimes = Collections.unmodifiableMap(map);
+    }
+  }
 }