CLDR-18927 non-TC orgs should tally by count, not just latest time (#5248)

Author: srl295

tools/cldr-code/src/main/java/org/unicode/cldr/util/Counter.java

@@ -48,6 +48,7 @@ private static final class RWLong implements Comparable<RWLong> {
         public long value;
         private final int forceUnique;
         public long time;
+        public int participants;
 
         {
             synchronized (RWLong.class) { // make thread-safe
@@ -71,6 +72,16 @@ public int compareTo(RWLong that) {
         public String toString() {
             return String.valueOf(value);
         }
+
+        /** return the number of times participate() was counted */
+        public final int getParticipants() {
+            return participants;
+        }
+
+        /** add one to the participant coutn */
+        void participate() {
+            participants++;
+        }
     }
 
     public Counter<T> add(T obj, long countValue) {
@@ -85,7 +96,10 @@ public Counter<T> add(T obj, long countValue, long time) {
         RWLong count = map.get(obj);
         if (count == null) map.put(obj, count = new RWLong());
         count.value += countValue;
-        count.time = time;
+        if (countValue != 0) {
+            count.time = time;
+        }
+        count.participate();
         return this;
     }
 
@@ -106,6 +120,12 @@ public long get(T obj) {
         return count == null ? 0 : count.value;
     }
 
+    /** returns the number of times 'add' was called */
+    public int getParticipation(T obj) {
+        RWLong count = map.get(obj);
+        return count == null ? 0 : count.getParticipants();
+    }
+
     /**
      * Get the time, or 0
      *

tools/cldr-code/src/main/java/org/unicode/cldr/util/VoteResolver.java

@@ -579,8 +579,8 @@ public int hashCode() {
      * @param <T>
      */
     static class MaxCounter<T> extends Counter<T> {
-        public MaxCounter(boolean b) {
-            super(b);
+        public MaxCounter(boolean naturalOrdering) {
+            super(naturalOrdering);
         }
 
         /** Add, but only to bring up to the maximum value. */
@@ -589,6 +589,8 @@ public MaxCounter<T> add(T obj, long countValue, long time) {
             long value = getCount(obj);
             if ((value <= countValue)) {
                 super.add(obj, countValue - value, time); // only add the difference!
+            } else {
+                super.add(obj, 0, time); // just add a participant
             }
             return this;
         }
@@ -780,19 +782,23 @@ public Counter<T> getTotals(EnumSet<Organization> conflictedOrganizations) {
                         }
                     }
                 }
-                // This is deprecated, but preserve it until the method is removed.
-                /*
-                 * TODO: explain the above comment, and follow through. What is deprecated (orgToAdd, or getOrgVote)?
-                 * Preserve until which method is removed (getOrgVote)?
-                 */
+
+                // temporarily make the top voted value this org's value.
+                // TODO: may not be needed, see below
                 orgToAdd.put(org, value);
 
-                // We add the max vote for each of the organizations choices
-                long maxCount = 0;
+                /** does this org vote by time (TC) or by number of votes (others)? */
+                final boolean votesByTime = orgVotesByTime(org);
+
                 T considerItem = null;
                 long considerCount = 0;
-                long maxtime = 0;
                 long considerTime = 0;
+                int considerParticipation = 0;
+
+                // We add the max vote for each of the organizations choices
+                long maxCount = 0;
+                long maxtime = 0;
+
                 for (T item : items.keySet()) {
                     if (DEBUG) {
                         System.out.println(
@@ -802,6 +808,7 @@ public Counter<T> getTotals(EnumSet<Organization> conflictedOrganizations) {
                     }
                     long count = items.getCount(item);
                     long time = items.getTime(item);
+                    int participation = votesByTime ? 0 : items.getParticipation(item);
                     if (count > maxCount) {
                         maxCount = count;
                         maxtime = time;
@@ -823,9 +830,21 @@ public Counter<T> getTotals(EnumSet<Organization> conflictedOrganizations) {
                                             + "MAXCOUNT: "
                                             + maxCount);
                         }
-                        considerCount = items.getCount(considerItem);
-                        considerTime = items.getTime(considerItem);
-                    } else if ((time > maxtime) && (count == maxCount)) {
+                        considerCount = items.getCount(considerItem); // TODO: == maxCount == count?
+                        considerTime = items.getTime(considerItem); // TODO: == maxtime == time?
+                    } else if (!votesByTime
+                            && (count == maxCount)
+                            && (participation > considerParticipation)) {
+                        // tell the 'losing' item
+                        if (considerItem != null) {
+                            annotateTranscript(
+                                    "---- Org is not voting for '%s' with %d votes: there is an item '%s' with %d votes",
+                                    considerItem, considerParticipation, item, participation);
+                        }
+                        considerItem = item;
+                    } else if ((time > maxtime)
+                            && (count == maxCount)
+                            && (votesByTime || (participation == considerParticipation))) {
                         maxtime = time;
                         // tell the 'losing' item
                         if (considerItem != null) {
@@ -844,10 +863,12 @@ public Counter<T> getTotals(EnumSet<Organization> conflictedOrganizations) {
                                             + new Timestamp(considerTime));
                         }
                     }
+                    considerParticipation = participation;
                 }
                 annotateTranscript(
                         "--- %s vote is for '%s' with strength %d",
                         org.getDisplayName(), considerItem, considerCount);
+                // TODO: is this ever not reached if there is a value?
                 orgToAdd.put(org, considerItem);
                 totals.add(considerItem, considerCount, considerTime);
 
@@ -906,21 +927,18 @@ public String toString() {
         }
 
         /**
-         * This is now deprecated, since the organization may have multiple votes.
+         * Get the winning vote for this organization
          *
          * @param org
          * @return
-         * @deprecated
          */
-        @Deprecated
         public T getOrgVote(Organization org) {
             return orgToAdd.get(org);
         }
 
-        public T getOrgVoteRaw(Organization orgOfUser) {
-            return orgToAdd.get(orgOfUser);
-        }
-
+        /**
+         * @return all possible votes from the org, including ones which are disputed.
+         */
         public Map<T, Long> getOrgToVotes(Organization org) {
             Map<T, Long> result = new LinkedHashMap<>();
             MaxCounter<T> counter = orgToVotes.get(org);
@@ -1912,17 +1930,36 @@ public EnumSet<Organization> getConflictedOrganizations() {
      * @return
      */
     public T getOrgVote(Organization org) {
+        if (!resolved) {
+            resolveVotes();
+        }
         return organizationToValueAndVote.getOrgVote(org);
     }
 
+    /**
+     * @return a map of all votes for this organization
+     */
     public Map<T, Long> getOrgToVotes(Organization org) {
+        if (!resolved) {
+            resolveVotes();
+        }
         return organizationToValueAndVote.getOrgToVotes(org);
     }
 
     public Map<String, Long> getNameTime() {
+        if (!resolved) {
+            resolveVotes();
+        }
         return organizationToValueAndVote.getNameTime();
     }
 
+    /**
+     * @return true if this organization's votes are ordered by time (as with TC orgs)
+     */
+    private static boolean orgVotesByTime(Organization org) {
+        return (org.isTCOrg());
+    }
+
     /**
      * Get a String representation of this VoteResolver. This is sent to the client as
      * "voteResolver.raw" and is used only for debugging.
@@ -2172,7 +2209,7 @@ public VoteStatus getStatusForOrganization(Organization orgOfUser) {
             // If the value is provisional, it needs more votes.
             return VoteStatus.provisionalOrWorse;
         }
-        T orgVote = organizationToValueAndVote.getOrgVoteRaw(orgOfUser);
+        T orgVote = organizationToValueAndVote.getOrgVote(orgOfUser);
         if (!equalsOrgVote(winningValue, orgVote)) {
             // We voted and lost
             return VoteStatus.losing;

tools/cldr-code/src/test/java/org/unicode/cldr/unittest/TestHelper.java

@@ -316,7 +316,10 @@ public enum TestUser {
         microsoftV(134, Organization.microsoft, Level.vetter),
         ibmE(114, Organization.ibm, Level.manager),
         ibmT(129, Organization.ibm, Level.tc),
-        unaffiliatedS2(802, Organization.unaffiliated, Level.guest);
+        unaffiliatedS2(802, Organization.unaffiliated, Level.guest),
+        bretonV1(901, Organization.breton, Level.vetter),
+        bretonV2(902, Organization.breton, Level.vetter),
+        bretonV3(903, Organization.breton, Level.vetter);
 
         public static final Map<Integer, VoterInfo> TEST_USERS;
         public final Integer voterId;

tools/cldr-code/src/test/java/org/unicode/cldr/util/TestVoteResolver.java

@@ -19,6 +19,56 @@
  */
 public class TestVoteResolver {
 
+    @Test
+    void testNonTcOrgByCountNotTime() {
+        final VoteResolver<String> vr = getStringResolver();
+        vr.enableTranscript();
+        vr.setLocale(
+                CLDRLocale.getInstance("br"), null); // NB: pathHeader is needed for annotations
+        vr.setBaseline("Inez Bouvet", Status.unconfirmed);
+        vr.setBaileyValue("BV");
+
+        // Three dates
+        final Date t0 = new Date(1500000000000L);
+        final Date t1 = new Date(1600000000000L);
+        final Date t2 = new Date(1700000000000L);
+
+        // earlier, but majority vote
+        vr.add("1 Bouvet", TestHelper.TestUser.bretonV1.voterId, null, t0);
+        vr.add("1 Bouvet", TestHelper.TestUser.bretonV2.voterId, null, t1);
+        // latest, but minority vote
+        vr.add("2 Bouvet", TestHelper.TestUser.bretonV3.voterId, null, t2);
+
+        boolean printTranscript = true;
+        try {
+            assertAll(
+                    "Verify org's vote",
+                    () ->
+                            assertEquals(
+                                    "1 Bouvet",
+                                    vr.getOrgVote(Organization.breton),
+                                    "breton's vote"),
+                    () ->
+                            assertEquals(
+                                    4,
+                                    vr.getOrgToVotes(Organization.breton).get("1 Bouvet"),
+                                    "breton's vote for '1 Bouvet'"),
+                    () -> assertEquals("1 Bouvet", vr.getWinningValue(), "for the winning value"),
+                    () ->
+                            assertEquals(
+                                    VoteResolver.VoteStatus.ok,
+                                    vr.getStatusForOrganization(Organization.breton)));
+            printTranscript = false; // none of the assertions failed, so no need to print
+        } finally {
+            if (printTranscript) {
+                // if there were errors, print out the transcript for debugging.
+                vr.isDisputed(); // to cause calculation
+                System.err.println(vr.getTranscript());
+                System.err.println(vr.toString());
+            }
+        }
+    }
+
     @Test
     void testDisputed() {
         final VoteResolver<String> vr = getStringResolver();
@@ -95,16 +145,19 @@ void testExplanations() {
         vr.add("bafut", TestHelper.TestUser.unaffiliatedS.voterId);
 
         vr.enableTranscript(); // Should be recalculated from here.
+        final String vrString = vr.toString(); // NB:  toString() modifies the transcript!
         assertAll(
                 "Verify the outcome",
                 () -> assertEquals("bambara", vr.getWinningValue()),
                 () -> assertEquals(Status.provisional, vr.getWinningStatus()));
         final String transcriptText = vr.getTranscript();
-        System.out.println(transcriptText);
-        System.out.println(vr.toString()); // NB:  toString() modifies the transcript!
         assertTrue(
                 transcriptText.contains("earlier than 'bassa'"),
-                () -> "Transcript did not match expectations:\n" + transcriptText);
+                () ->
+                        "Transcript did not match expectations:\n"
+                                + transcriptText
+                                + "\n"
+                                + vrString);
     }
 
     private VoteResolver<String> getStringResolver() {