Do not lazy load fields if the cache doesn't come from the database

When a cache is stored in the database, some fields are lazily loaded
from the database. This should only happen when the cache itself has
been loaded from the database, otherwise we might fill fields of a cache
acquired from the network with older version of the fields as stored in
the database.

Noticed during the work on #4576.

4 files changed, 104 insertions, 20 deletions

diff --git a/main/src/cgeo/geocaching/DataStore.java b/main/src/cgeo/geocaching/DataStore.java
index d36a9c9..f614ab1 100644
--- a/main/src/cgeo/geocaching/DataStore.java
+++ b/main/src/cgeo/geocaching/DataStore.java
@@ -2946,7 +2946,7 @@ public class DataStore {
     }
 
     public static void saveChangedCache(final Geocache cache) {
-        DataStore.saveCache(cache, cache.getStorageLocation().contains(StorageLocation.DATABASE) ? LoadFlags.SAVE_ALL : EnumSet.of(SaveFlag.CACHE));
+        DataStore.saveCache(cache, cache.inDatabase() ? LoadFlags.SAVE_ALL : EnumSet.of(SaveFlag.CACHE));
     }
 
     private static enum PreparedStatement {
diff --git a/main/src/cgeo/geocaching/Geocache.java b/main/src/cgeo/geocaching/Geocache.java
index c842a7f..dda19c8 100644
--- a/main/src/cgeo/geocaching/Geocache.java
+++ b/main/src/cgeo/geocaching/Geocache.java
@@ -133,13 +133,13 @@ public class Geocache implements IWaypoint {
     private final LazyInitializedList<String> attributes = new LazyInitializedList<String>() {
         @Override
         public List<String> call() {
-            return DataStore.loadAttributes(geocode);
+            return inDatabase() ? DataStore.loadAttributes(geocode) : new LinkedList<String>();
         }
     };
     private final LazyInitializedList<Waypoint> waypoints = new LazyInitializedList<Waypoint>() {
         @Override
         public List<Waypoint> call() {
-            return DataStore.loadWaypoints(geocode);
+            return inDatabase() ? DataStore.loadWaypoints(geocode) : new LinkedList<Waypoint>();
         }
     };
     private List<Image> spoilers = null;
@@ -629,18 +629,25 @@ public class Geocache implements IWaypoint {
      */
     private void initializeCacheTexts() {
         if (description == null || shortdesc == null || hint == null || location == null) {
-            final Geocache partial = DataStore.loadCacheTexts(this.getGeocode());
-            if (description == null) {
-                setDescription(partial.getDescription());
-            }
-            if (shortdesc == null) {
-                setShortDescription(partial.getShortDescription());
-            }
-            if (hint == null) {
-                setHint(partial.getHint());
-            }
-            if (location == null) {
-                setLocation(partial.getLocation());
+            if (inDatabase()) {
+                final Geocache partial = DataStore.loadCacheTexts(this.getGeocode());
+                if (description == null) {
+                    setDescription(partial.getDescription());
+                }
+                if (shortdesc == null) {
+                    setShortDescription(partial.getShortDescription());
+                }
+                if (hint == null) {
+                    setHint(partial.getHint());
+                }
+                if (location == null) {
+                    setLocation(partial.getLocation());
+                }
+            } else {
+                description = StringUtils.defaultString(description);
+                shortdesc = StringUtils.defaultString(shortdesc);
+                hint = StringUtils.defaultString(hint);
+                location = StringUtils.defaultString(location);
             }
         }
     }
@@ -1013,7 +1020,7 @@ public class Geocache implements IWaypoint {
      */
     @NonNull
     public List<LogEntry> getLogs() {
-        return DataStore.loadLogs(geocode);
+        return inDatabase() ? DataStore.loadLogs(geocode) : Collections.<LogEntry>emptyList();
     }
 
     /**
@@ -1182,6 +1189,13 @@ public class Geocache implements IWaypoint {
     }
 
     /**
+     * Check if this cache instance comes from or has been stored into the database.
+     */
+    public boolean inDatabase() {
+        return storageLocation.contains(StorageLocation.DATABASE);
+    }
+
+    /**
      * @param waypoint
      *            Waypoint to add to the cache
      * @param saveToDatabase
@@ -1748,7 +1762,7 @@ public class Geocache implements IWaypoint {
      */
     public int getFindsCount() {
         if (getLogCounts().isEmpty()) {
-            setLogCounts(DataStore.loadLogCounts(getGeocode()));
+            setLogCounts(inDatabase() ? DataStore.loadLogCounts(getGeocode()) : Collections.<LogType, Integer>emptyMap());
         }
         final Integer logged = getLogCounts().get(LogType.FOUND_IT);
         if (logged != null) {
diff --git a/tests/src/cgeo/CGeoTestCase.java b/tests/src/cgeo/CGeoTestCase.java
index 5759bc7..7fc1e8d 100644
--- a/tests/src/cgeo/CGeoTestCase.java
+++ b/tests/src/cgeo/CGeoTestCase.java
@@ -2,6 +2,7 @@ package cgeo;
 
 import cgeo.geocaching.CgeoApplication;
 import cgeo.geocaching.DataStore;
+import cgeo.geocaching.Geocache;
 import cgeo.geocaching.enumerations.LoadFlags;
 import cgeo.geocaching.enumerations.LoadFlags.RemoveFlag;
 import cgeo.geocaching.settings.Settings;
@@ -44,6 +45,17 @@ public abstract class CGeoTestCase extends ApplicationTestCase<CgeoApplication>
     }
 
     /**
+     * Remove completely the previous instance of a cache, then save this object into the database
+     * and the cache cache.
+     *
+     * @param cache the fresh cache to save
+     */
+    protected static void saveFreshCacheToDB(final Geocache cache) {
+        removeCacheCompletely(cache.getGeocode());
+        DataStore.saveCache(cache, LoadFlags.SAVE_ALL);
+    }
+
+    /**
      * must be called once before setting the flags
      * can be called again after restoring the flags
      */
diff --git a/tests/src/cgeo/geocaching/GeocacheTest.java b/tests/src/cgeo/geocaching/GeocacheTest.java
index 28f6620..37b6883 100644
--- a/tests/src/cgeo/geocaching/GeocacheTest.java
+++ b/tests/src/cgeo/geocaching/GeocacheTest.java
@@ -12,6 +12,7 @@ import org.apache.commons.lang3.StringUtils;
 
 import java.util.ArrayList;
 import java.util.Calendar;
+import java.util.Collections;
 import java.util.Date;
 import java.util.List;
 
@@ -93,6 +94,39 @@ public class GeocacheTest extends CGeoTestCase {
         }
     }
 
+    public static void testMergeDownloaded() {
+        final Geocache previous = new Geocache();
+        previous.setGeocode("GC12345");
+        previous.setDetailed(true);
+        previous.setDisabled(true);
+        previous.setType(CacheType.TRADITIONAL);
+        previous.setCoords(new Geopoint(40.0, 8.0));
+        previous.setDescription("Test1");
+        previous.setAttributes(Collections.singletonList("TestAttribute"));
+        previous.setShortDescription("Short");
+        previous.setHint("Hint");
+        removeCacheCompletely(previous.getGeocode());
+
+        final Geocache download = new Geocache();
+        download.setGeocode("GC12345");
+        download.setDetailed(true);
+        download.setDisabled(false);
+        download.setType(CacheType.MULTI);
+        download.setCoords(new Geopoint(41.0, 9.0));
+        download.setDescription("Test2");
+
+        download.gatherMissingFrom(previous);
+
+        assertThat(download.isDetailed()).as("merged detailed").isTrue();
+        assertThat(download.isDisabled()).as("merged disabled").isFalse();
+        assertThat(download.getType()).as("merged download").isEqualTo(CacheType.MULTI);
+        assertThat(download.getCoords()).as("merged coordinates").isEqualTo(new Geopoint(41.0, 9.0));
+        assertThat(download.getDescription()).as("merged description").isEqualTo("Test2");
+        assertThat(download.getShortDescription()).as("merged short description").isEmpty();
+        assertThat(download.getAttributes()).as("merged attributes").isEmpty();
+        assertThat(download.getHint()).as("merged hint").isEmpty();
+    }
+
     public static void testMergeDownloadedStored() {
         final Geocache stored = new Geocache();
         stored.setGeocode("GC12345");
@@ -101,11 +135,10 @@ public class GeocacheTest extends CGeoTestCase {
         stored.setType(CacheType.TRADITIONAL);
         stored.setCoords(new Geopoint(40.0, 8.0));
         stored.setDescription("Test1");
-        final ArrayList<String> attributes = new ArrayList<String>(1);
-        attributes.add("TestAttribute");
-        stored.setAttributes(attributes);
+        stored.setAttributes(Collections.singletonList("TestAttribute"));
         stored.setShortDescription("Short");
         stored.setHint("Hint");
+        saveFreshCacheToDB(stored);
 
         final Geocache download = new Geocache();
         download.setGeocode("GC12345");
@@ -114,6 +147,7 @@ public class GeocacheTest extends CGeoTestCase {
         download.setType(CacheType.MULTI);
         download.setCoords(new Geopoint(41.0, 9.0));
         download.setDescription("Test2");
+        download.setAttributes(Collections.<String>emptyList());
 
         download.gatherMissingFrom(stored);
 
@@ -127,6 +161,29 @@ public class GeocacheTest extends CGeoTestCase {
         assertThat(download.getHint()).as("merged hint").isEmpty();
     }
 
+    public static void testMergeLivemap() {
+        final Geocache previous = new Geocache();
+        previous.setGeocode("GC12345");
+        previous.setDetailed(true);
+        previous.setDisabled(true);
+        previous.setType(CacheType.TRADITIONAL);
+        previous.setCoords(new Geopoint(40.0, 8.0));
+        removeCacheCompletely(previous.getGeocode());
+
+        final Geocache livemap = new Geocache();
+        livemap.setGeocode("GC12345");
+        livemap.setType(CacheType.MULTI, 12);
+        livemap.setCoords(new Geopoint(41.0, 9.0), 12);
+
+        livemap.gatherMissingFrom(previous);
+
+        assertThat(livemap.isDetailed()).as("merged detailed").isTrue();
+        assertThat(livemap.isDisabled()).as("merged disabled").isTrue();
+        assertThat(livemap.getType()).as("merged type").isEqualTo(CacheType.TRADITIONAL);
+        assertThat(livemap.getCoords()).as("merged coordinates").isEqualToComparingFieldByField(new Geopoint(40.0, 8.0));
+        assertThat(livemap.getCoordZoomLevel()).as("merged zoomlevel").isEqualTo(previous.getCoordZoomLevel());
+    }
+
     public static void testMergeLivemapStored() {
         final Geocache stored = new Geocache();
         stored.setGeocode("GC12345");
@@ -134,6 +191,7 @@ public class GeocacheTest extends CGeoTestCase {
         stored.setDisabled(true);
         stored.setType(CacheType.TRADITIONAL);
         stored.setCoords(new Geopoint(40.0, 8.0));
+        saveFreshCacheToDB(stored);
 
         final Geocache livemap = new Geocache();
         livemap.setGeocode("GC12345");