Bug 1391413: Add SuggestedSites.cachedDistributionSites and accessors. r=sebastian draft
authorMichael Comella <michael.l.comella@gmail.com>
Thu, 07 Sep 2017 15:18:41 -0700
changeset 661132 432c1cdab9bc37473cb9da72b066153a83c45139
parent 661131 ac78d69222924cd043e055f6764a98b09db4206e
child 661133 90c3a58b05c1a6fcaff56e7524b3d0f6c851e9cd
push id78639
push usermichael.l.comella@gmail.com
push dateFri, 08 Sep 2017 00:35:03 +0000
reviewerssebastian
bugs1391413
milestone57.0a1
Bug 1391413: Add SuggestedSites.cachedDistributionSites and accessors. r=sebastian MozReview-Commit-ID: HBqKc8a0tNJ
mobile/android/base/java/org/mozilla/gecko/db/SuggestedSites.java
--- a/mobile/android/base/java/org/mozilla/gecko/db/SuggestedSites.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/SuggestedSites.java
@@ -18,16 +18,17 @@ import android.util.Log;
 
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.OutputStreamWriter;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Scanner;
 import java.util.Set;
 
@@ -146,16 +147,17 @@ public class SuggestedSites {
             return json;
         }
     }
 
     final Context context;
     final Distribution distribution;
     private File cachedFile;
     private Map<String, Site> cachedSites;
+    private Map<String, Site> cachedDistributionSites; // to be kept in sync with cachedSites.
     private Set<String> cachedBlacklist;
 
     public SuggestedSites(Context appContext) {
         this(appContext, null);
     }
 
     public SuggestedSites(Context appContext, Distribution distribution) {
         this(appContext, distribution, null);
@@ -374,16 +376,40 @@ public class SuggestedSites {
         } catch (IOException e) {
             return null;
         }
     }
 
     private synchronized void setCachedSites(Map<String, Site> sites) {
         cachedSites = Collections.unmodifiableMap(sites);
         updateSuggestedSitesLocale(context);
+        cachedDistributionSites = getDistributionSites(cachedSites, loadFromResource());
+    }
+
+    /**
+     * Gets the list of distribution sites from the list of all suggested sites and those drawn from the resources.
+     *
+     * This isn't the most efficient way to get the distribution sites, especially since we currently call
+     * {@link #loadFromResource()} an additional time to get our list of sites in resources. However, I
+     * found this to be the simplest approach. One alternative was to store the is-from-distribution state in
+     * the saved-site JSON files but the same code that reads these files also reads the distribution files and
+     * the resource file so each of these would also need explicit `isFromDistribution` flags, which is annoying
+     * to write and would create a lot of churn. I found the addition of this method and a cached value to be much
+     * simpler to add (and perhaps later remove).
+     */
+    private static Map<String, Site> getDistributionSites(final Map<String, Site> allSuggestedSites,
+            final Map<String, Site> suggestedSitesFromResources) {
+        final Set<String> allSitesURLsCopy = new HashSet<>(allSuggestedSites.keySet());
+        allSitesURLsCopy.removeAll(suggestedSitesFromResources.keySet());
+
+        final Map<String, Site> distributionSites = new HashMap<>(allSitesURLsCopy.size());
+        for (final String distributionSiteURL : allSitesURLsCopy) {
+            distributionSites.put(distributionSiteURL, allSuggestedSites.get(distributionSiteURL));
+        }
+        return distributionSites;
     }
 
     /**
      * Refreshes the cached list of sites either from the default raw
      * source or standard file location. This will be called on every
      * cache miss during a {@code get()} call.
      */
     private void refresh() {
@@ -510,16 +536,29 @@ public class SuggestedSites {
 
         return cursor;
     }
 
     public boolean contains(String url) {
         return (getSiteForUrl(url) != null);
     }
 
+    /**
+     * Returns whether or not the url both represents a suggested site
+     * and was added to suggested sites by a distribution.
+     *
+     * We synchronize over our access to {@link #cachedDistributionSites},
+     * like we synchronize over all uses of {@link #cachedSites}.
+     */
+    public synchronized boolean containsSiteAndSiteIsFromDistribution(final String url) {
+        return cachedDistributionSites != null &&
+                contains(url) &&
+                cachedDistributionSites.containsKey(url);
+    }
+
     public String getImageUrlForUrl(String url) {
         final Site site = getSiteForUrl(url);
         return (site != null ? site.imageUrl : null);
     }
 
     public String getBackgroundColorForUrl(String url) {
         final Site site = getSiteForUrl(url);
         return (site != null ? site.bgColor : null);