From 3085cb1bf05fc617a3225edd142b31d48314bff5 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Wed, 7 Jan 2015 09:21:28 +0100 Subject: 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. --- tests/src/cgeo/CGeoTestCase.java | 12 ++++++ tests/src/cgeo/geocaching/GeocacheTest.java | 64 +++++++++++++++++++++++++++-- 2 files changed, 73 insertions(+), 3 deletions(-) (limited to 'tests/src') 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 } /** + * 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 attributes = new ArrayList(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.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"); -- cgit v1.1