Provide missing IDs to withOptionCaching (#987)

The `withOptionCaching` calls are designed to be given an ID, and if that ID isn't in the cache it will call the database. There are cases where the ID was not provided and it would always ping the database and update the cache entry.

app/org/maproulette/cache/Cache.scala

@@ -7,6 +7,8 @@ package org.maproulette.cache
 
 import org.joda.time.{LocalDateTime, Seconds}
 
+import scala.concurrent.Future
+
 /**
   * @author mcuthbert
   */
@@ -19,6 +21,7 @@ case class BasicInnerValue[Key, Value](
 
 trait Cache[Key, Value <: CacheObject[Key]] {
 
+  import scala.concurrent.ExecutionContext.Implicits.global
   implicit val cacheLimit: Int
   implicit val cacheExpiry: Int
 
@@ -34,6 +37,17 @@ trait Cache[Key, Value <: CacheObject[Key]] {
     this.add(obj.id, obj, localExpiry)
   }
 
+  /** Adds a list of objects to the cache within a future.
+    *
+    * @param objs the objects to add to the cache
+    * @return the list of items added to the cache, although these may or may not still be in the cache
+    */
+  def addObjectsAsync(objs: List[Value]): Future[List[Value]] = {
+    Future {
+      objs.map(obj => this.addObject(obj).get)
+    }
+  }
+
   /**
     * Checks if an item is cached or not
     *

app/org/maproulette/models/dal/ChallengeDAL.scala

@@ -433,7 +433,7 @@ class ChallengeDAL @Inject() (
              WHERE c.id = $id
            """.as(projectParser.*).headOption
           }
-        }
+        }(Some(id))
       case Right(challenge) =>
         this.permission.hasObjectReadAccess(challenge, user)
         this.serviceManager.project.cacheManager.withOptionCaching { () =>
@@ -442,7 +442,7 @@ class ChallengeDAL @Inject() (
               .as(projectParser.*)
               .headOption
           }
-        }
+        }(Some(challenge.general.parent))
     }
   }
 
@@ -958,7 +958,10 @@ class ChallengeDAL @Inject() (
                       AND (c.status = ${Challenge.STATUS_READY})
                       AND c.requires_local = false
                       LIMIT ${this.sqlLimit(limit)} OFFSET $offset"""
-      SQL(query).as(this.parser.*)
+      val challenges = SQL(query).as(this.parser.*)
+      // Insert the challenges into the cache
+      this.cacheManager.cache.addObjectsAsync(challenges)
+      challenges
     }
   }
 
@@ -983,7 +986,10 @@ class ChallengeDAL @Inject() (
                       AND (c.status = ${Challenge.STATUS_READY})
                       AND c.requires_local = false
                       ORDER BY popularity DESC LIMIT ${this.sqlLimit(limit)} OFFSET $offset"""
-      SQL(query).as(this.parser.*)
+      val challenges = SQL(query).as(this.parser.*)
+      // Insert the challenges into the cache
+      this.cacheManager.cache.addObjectsAsync(challenges)
+      challenges
     }
   }
 
@@ -1009,7 +1015,10 @@ class ChallengeDAL @Inject() (
                       AND c.requires_local = false
                       ${this.order(Some("created"), "DESC", "c", true)}
                       LIMIT ${this.sqlLimit(limit)} OFFSET $offset"""
-      SQL(query).as(this.parser.*)
+      val challenges = SQL(query).as(this.parser.*)
+      // Insert the challenges into the cache
+      this.cacheManager.cache.addObjectsAsync(challenges)
+      challenges
     }
   }
 

app/org/maproulette/models/dal/TaskDAL.scala

@@ -139,6 +139,11 @@ class TaskDAL @Inject() (
         }
       case Right(task) =>
         this.permission.hasObjectReadAccess(task, user)
+
+        // TODO(ljdelight): This block was identified by a profile, it's too slow.
+        //  When a ton of tasks are scanned and the projects are fetched, it's not using the cache.
+        //  The goal here is to be able to use a task id and quickly get the project... however the
+        //  serviceManager.challenge does not have a cache manager. It looks like this needs added.
         this.serviceManager.project.cacheManager.withOptionCaching { () =>
           this.withMRConnection { implicit c =>
             SQL"""SELECT p.* FROM projects p
@@ -301,7 +306,7 @@ class TaskDAL @Inject() (
       }
 
       task
-    }
+    } // NOTE: do not look for a cached item since this is expected to update the database
   }
 
   /**