Merge pull request #3459 from AtlasOfLivingAustralia/feature/issue3458

Smarter stats loading to prevent blocking homepage load #3458

Author: temi

grails-app/controllers/au/org/ala/merit/ReportController.groovy

@@ -486,7 +486,7 @@ class ReportController {
 
     def statisticsReport() {
         int exclude = session.lastGroup?:-1
-        def statistics = statisticsFactory.randomGroup(exclude)
+        def statistics = statisticsFactory.randomGroup(exclude, true)
         session.lastGroup = statistics.group
         render view:'_statistics', layout:'ajax', model:[statistics:statistics.statistics]
     }

grails-app/views/home/public.gsp

@@ -59,20 +59,29 @@
     <asset:deferredScripts/>
     <script>
                 $(function() {
-
+                    var working = false;
                     var url = '${g.createLink(controller:'report', action:'statisticsReport')}';
 
-                    var working = false;
-                    $('#stats-holder').on('click', '.show-more-stats', function() {
+                    function showMoreStats() {
                         if (!working) {
                             working = true;
                             replaceContentSection('.statistics', url).always(function() { working = false; });
                         }
+                    }
+
+                    $('#stats-holder').on('click', '.show-more-stats', function() {
+                       showMoreStats();
                     });
 
                     if ($('#latest-news').height() > 400) {
                         $('#latest-news').height(400).css('overflow-y', 'scroll');
                     }
+
+                    var hasStatistics = ${statistics ? 'true' : 'false'};
+                    if (!hasStatistics) {
+                        showMoreStats();
+                    }
+
                 });
 
     </script>

grails-app/views/report/_statistics.gsp

@@ -1,4 +1,5 @@
 <div class="statistics">
+    <g:if test="${statistics}">
     %{--This section expects exactly 6 statistics to display--}%
     <div class="row">
         <div class="col-sm-4 box1">
@@ -22,7 +23,9 @@
             <g:render template="/report/statistic" model="${statistics[5]}"/>
         </div>
     </div>
+
     <div class="row">
         <div class="col-sm-12 align-content-center text-center"><a href="#" class="show-more-stats text-dark">Show more stats <i class="fa fa-refresh"></i></a></div>
     </div>
+    </g:if>
 </div>

src/integration-test/groovy/au/org/ala/fieldcapture/HomeIndexPageSpec.groovy

@@ -30,8 +30,9 @@ class HomeIndexPageSpec extends StubbedCasSpec {
         to HomePage
 
         then:
-        waitFor 30,{ at HomePage}
-
+        waitFor 60, { // The homepage statistics are loaded via AJAX when the cache is cleared
+            box1.size() == 1
+        }
         and:
         box1[0].statTitle.text() == "THREATENED SPECIES STRATEGY"
         box1[0].statValue.text() =="3"

src/main/groovy/au/org/ala/merit/StatisticsFactory.groovy

@@ -29,6 +29,9 @@ class StatisticsFactory {
     @Autowired
     GrailsApplication grailsApplication
 
+    /** Lazily initialized in the getConfig method based on the HOME_PAGE_STATISTICS setting */
+    private Map config
+
     public StatisticsFactory() {}
 
     private String initialize() {
@@ -46,29 +49,52 @@ class StatisticsFactory {
     public synchronized void clearConfig() {
         Cache cache = grailsCacheManager.getCache(STATISTICS_CACHE_REGION)
         cache.clear()
+        config = null
     }
 
     private synchronized Map getConfig() {
-        String config = grailsCacheManager.getCache(STATISTICS_CACHE_REGION).get(CONFIG_KEY)?.get()
-        if (!config) {
-            config = initialize()
-            grailsCacheManager.getCache(STATISTICS_CACHE_REGION).put(CONFIG_KEY, config)
+        String configString = grailsCacheManager.getCache(STATISTICS_CACHE_REGION).get(CONFIG_KEY)?.get()
+        // We check for the config string cache instead of whether this.config is null because
+        // this allows the generic admin cache page to clear the cache and allow a config update
+        // without having to know to call StatisticsFactory.clearConfig()
+        // We can't cache the parsed config directly as it is not serializable.
+        if (!configString || !this.config) { // Check both as the cache can survive a restart sometimes, but not the field.
+            configString = initialize()
+            grailsCacheManager.getCache(STATISTICS_CACHE_REGION).put(CONFIG_KEY, configString)
+            JsonSlurper jsonSlurper = new JsonSlurper()
+            this.config = Collections.synchronizedMap(jsonSlurper.parseText(configString))
         }
-        JsonSlurper jsonSlurper = new JsonSlurper()
-        jsonSlurper.parseText(config)
+
+        this.config
     }
 
-    public synchronized List<Map> getStatisticsGroup(int groupNumber) {
+    public List<Map> getStatisticsGroupFromCache(int groupNumber) {
+        fromCache(groupNumber)
+    }
+
+    public List<Map> getStatisticsGroup(int groupNumber) {
 
         Map config = getConfig()
-        List<Map> statistics = fromCache(groupNumber)
-        if (!statistics) {
-            log.info("Cache miss for homepage stats, key: ${groupNumber}")
-            statistics = this.config.groups[groupNumber].collect { statisticName ->
-                Map statistic = config.statistics[statisticName]
-                evaluateStatistic(statistic)
+        List groupConfig = config.groups[groupNumber]
+        if (!groupConfig) {
+            log.error("No configuration found for group number: ${groupNumber}")
+            return null
+        }
+
+        List<Map> statistics
+        // Only one thread should recalculate the statistics.  The config is a shared
+        // synchronized instance so is safe to synchronize on.
+        synchronized (groupConfig) {
+            statistics = fromCache(groupNumber)
+            if (!statistics) {
+                log.info("Cache miss for homepage stats, key: ${groupNumber} - recalculating...")
+                statistics = groupConfig.collect { statisticName ->
+                    Map statistic = config.statistics[statisticName]
+                    evaluateStatistic(statistic)
+                }
+                log.info("Caching homepage stats, key: ${groupNumber}")
+                cache(groupNumber, statistics)
             }
-            cache(groupNumber, statistics)
         }
         statistics
     }
@@ -87,13 +113,19 @@ class StatisticsFactory {
         stats
     }
 
-    public Map randomGroup(int exclude = -1) {
+    public Map randomGroup(int exclude = -1, boolean recacluateIfMissing = false) {
         int groupCount = getGroupCount()
         int group = Math.floor(Math.random()*groupCount)
         while (group == exclude) {
             group = Math.floor(Math.random()*groupCount)
         }
-        List stats = getStatisticsGroup(group)
+        List stats
+        if (recacluateIfMissing) {
+            stats = getStatisticsGroup(group)
+        }
+        else {
+            stats = getStatisticsGroupFromCache(group)
+        }
 
         [group:group, statistics:stats]
     }