Bug 1378967 - Pre: make our mapCursor implementation "lossy" r=sebastian draft
authorGrigory Kruglov <gkruglov@mozilla.com>
Thu, 13 Jul 2017 18:05:24 -0400
changeset 608596 2347b4fde0bc1e9f520a8a4485f25172ed57a70d
parent 607669 03bcd6d65af62c5e60a0ab9247ccce43885e707b
child 608597 17fa1d3506dc47b3353e3e884678fcec68759b9f
push id68346
push userbmo:gkruglov@mozilla.com
push dateThu, 13 Jul 2017 22:12:51 +0000
reviewerssebastian
bugs1378967
milestone56.0a1
Bug 1378967 - Pre: make our mapCursor implementation "lossy" r=sebastian It's helpful to be able to map a cursor to a list, conditionally processing every cursor entry. This patch considers 'null' result from func to be a "ignore this cursor entry" sentinel. We could achieve the same via map->filter, but that would require multiple iterations over a list. MozReview-Commit-ID: Ixmzyhd2RqM
mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightsRanking.java
mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/RankingUtils.java
mobile/android/tests/background/junit4/src/org/mozilla/gecko/activitystream/ranking/TestRankingUtils.java
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightsRanking.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/HighlightsRanking.java
@@ -22,17 +22,17 @@ import static java.util.Collections.sort
 import static org.mozilla.gecko.activitystream.ranking.RankingUtils.Action1;
 import static org.mozilla.gecko.activitystream.ranking.RankingUtils.Action2;
 import static org.mozilla.gecko.activitystream.ranking.RankingUtils.Func1;
 import static org.mozilla.gecko.activitystream.ranking.RankingUtils.Func2;
 import static org.mozilla.gecko.activitystream.ranking.RankingUtils.apply;
 import static org.mozilla.gecko.activitystream.ranking.RankingUtils.apply2D;
 import static org.mozilla.gecko.activitystream.ranking.RankingUtils.applyInPairs;
 import static org.mozilla.gecko.activitystream.ranking.RankingUtils.filter;
-import static org.mozilla.gecko.activitystream.ranking.RankingUtils.mapCursor;
+import static org.mozilla.gecko.activitystream.ranking.RankingUtils.looselyMapCursor;
 import static org.mozilla.gecko.activitystream.ranking.RankingUtils.mapWithLimit;
 import static org.mozilla.gecko.activitystream.ranking.RankingUtils.reduce;
 
 /**
  * HighlightsRanking.rank() takes a Cursor of highlight candidates and applies ranking to find a set
  * of good highlights. The result set is likely smaller than the cursor size.
  *
  * - First we calculate an initial score based on how frequent we visit the URL and domain.
@@ -89,17 +89,17 @@ public class HighlightsRanking {
         return createHighlightsList(highlights, limit);
     }
 
     /**
      * Extract features for every candidate. The heavy lifting is done in
      * HighlightCandidate.fromCursor().
      */
     @VisibleForTesting static List<HighlightCandidate> extractFeatures(Cursor cursor) {
-        return mapCursor(cursor, new Func1<Cursor, HighlightCandidate>() {
+        return looselyMapCursor(cursor, new Func1<Cursor, HighlightCandidate>() {
             @Override
             public HighlightCandidate call(Cursor cursor) {
                 return HighlightCandidate.fromCursor(cursor);
             }
         });
     }
 
     /**
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/RankingUtils.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/ranking/RankingUtils.java
@@ -2,18 +2,16 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.activitystream.ranking;
 
 import android.database.Cursor;
 
-import org.mozilla.gecko.R;
-
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
 
 /**
  * Some helper methods that make processing lists in a pipeline easier. This wouldn't be needed with
  * Java 8 streams or RxJava. But we haven't anything like that available.
@@ -143,30 +141,34 @@ import java.util.List;
         for (int i = 0; i < items.size() && i < limit; i++) {
             newItems.add(func.call(items.get(i)));
         }
 
         return newItems;
     }
 
     /**
-     * Transform a cursor into a list of items by calling a function for every cursor position to
-     * return one item.
+     * Transform a cursor of size N into a list of items of size M by calling a function for every
+     * cursor position. If `null` is returned, that cursor position is skipped. Otherwise, value is
+     * added to the list.
      */
-    static <T> List<T> mapCursor(Cursor cursor, Func1<Cursor, T> func) {
+    static <T> List<T> looselyMapCursor(Cursor cursor, Func1<Cursor, T> func) {
         List<T> items = new ArrayList<>(cursor.getCount());
 
         if (cursor.getCount() == 0) {
             return items;
         }
 
         cursor.moveToFirst();
 
         do {
-            items.add(func.call(cursor));
+            T item = func.call(cursor);
+            if (item != null) {
+                items.add(item);
+            }
         } while (cursor.moveToNext());
 
         return items;
     }
 
     static double normalize(double value, double min, double max) {
         if (max > min) {
             if (value < min || value > max) {
--- a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/activitystream/ranking/TestRankingUtils.java
+++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/activitystream/ranking/TestRankingUtils.java
@@ -186,40 +186,56 @@ public class TestRankingUtils {
 
         Assert.assertEquals(10, result.get(0), 1e-6);
         Assert.assertEquals(14, result.get(1), 1e-6);
         Assert.assertEquals(6, result.get(2), 1e-6);
         Assert.assertEquals(4, result.get(3), 1e-6);
     }
 
     @Test
-    public void testMapCursor() {
+    public void testLooselyMapCursor() {
         final MatrixCursor cursor = new MatrixCursor(new String[] {
                 "column1", "column2"
         });
 
         cursor.addRow(new Object[] { 1, 3 });
         cursor.addRow(new Object[] { 5, 7 });
         cursor.addRow(new Object[] { 2, 9 });
 
         final Func1<Cursor, Integer> func = new Func1<Cursor, Integer>() {
             @Override
             public Integer call(Cursor cursor) {
-                return cursor.getInt(cursor.getColumnIndexOrThrow("column1"))
-                        + cursor.getInt(cursor.getColumnIndexOrThrow("column2"));
+                // -1 is our "fail this cursor entry" sentinel.
+                final int col1 = cursor.getInt(cursor.getColumnIndexOrThrow("column1"));
+                if (col1 == -1) {
+                    return null;
+                }
+                return col1 + cursor.getInt(cursor.getColumnIndexOrThrow("column2"));
             }
         };
 
-        final List<Integer> result = RankingUtils.mapCursor(cursor, func);
+        List<Integer> result = RankingUtils.looselyMapCursor(cursor, func);
 
         Assert.assertEquals(3, result.size());
 
         Assert.assertEquals(4, result.get(0).intValue());
         Assert.assertEquals(12, result.get(1).intValue());
         Assert.assertEquals(11, result.get(2).intValue());
+
+        // Test that cursor entries for which func returns null are ignored.
+        cursor.addRow(new Object[] {-1, 6});
+        cursor.addRow(new Object[] {3, 3});
+
+        result = RankingUtils.looselyMapCursor(cursor, func);
+        Assert.assertEquals(4, result.size());
+
+        Assert.assertEquals(4, result.get(0).intValue());
+        Assert.assertEquals(12, result.get(1).intValue());
+        Assert.assertEquals(11, result.get(2).intValue());
+        Assert.assertEquals(6, result.get(3).intValue());
     }
 
     @Test
     public void testNormalize() {
         Assert.assertEquals(0, RankingUtils.normalize(0, 0, 100), 1e-6);
         Assert.assertEquals(0.5, RankingUtils.normalize(0, -100, 100), 1e-6);
         Assert.assertEquals(1, RankingUtils.normalize(1, 0, 1), 1e-6);
         Assert.assertEquals(0, RankingUtils.normalize(50, 50, 100), 1e-6);