fix #2534: Updating a OC.de cache doubles the cache description

5 files changed, 75 insertions, 17 deletions

diff --git a/main/src/cgeo/geocaching/Geocache.java b/main/src/cgeo/geocaching/Geocache.java
index 18a315c..e672ca8 100644
--- a/main/src/cgeo/geocaching/Geocache.java
+++ b/main/src/cgeo/geocaching/Geocache.java
@@ -618,15 +618,27 @@ public class Geocache implements ICache, IWaypoint {
     @Override
     public String getHint() {
         initializeCacheTexts();
+        assertTextNotNull(hint, "Hint");
         return hint;
     }
 
     /**
+     * After lazy loading the lazily loaded field must be non {@code null}.
+     *
+     */
+    private static void assertTextNotNull(final String field, final String name) throws InternalError {
+        if (field == null) {
+            throw new InternalError(name + " field is not allowed to be null here");
+        }
+    }
+
+    /**
      * Attention, calling this method may trigger a database access for the cache!
      */
     @Override
     public String getDescription() {
         initializeCacheTexts();
+        assertTextNotNull(description, "Description");
         return description;
     }
 
@@ -635,7 +647,19 @@ public class Geocache implements ICache, IWaypoint {
      */
     private void initializeCacheTexts() {
         if (description == null || shortdesc == null || hint == null || location == null) {
-            cgData.loadCacheTexts(this);
+            Geocache partial = cgData.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());
+            }
         }
     }
 
@@ -645,6 +669,7 @@ public class Geocache implements ICache, IWaypoint {
     @Override
     public String getShortDescription() {
         initializeCacheTexts();
+        assertTextNotNull(shortdesc, "Short description");
         return shortdesc;
     }
 
@@ -673,6 +698,7 @@ public class Geocache implements ICache, IWaypoint {
     @Override
     public String getLocation() {
         initializeCacheTexts();
+        assertTextNotNull(location, "Location");
         return location;
     }
 
diff --git a/main/src/cgeo/geocaching/cgData.java b/main/src/cgeo/geocaching/cgData.java
index f83ba87..770a28f 100644
--- a/main/src/cgeo/geocaching/cgData.java
+++ b/main/src/cgeo/geocaching/cgData.java
@@ -2660,11 +2660,21 @@ public class cgData {
         return result;
     }
 
-    public static String loadCacheTexts(final Geocache cache) {
-        final String geocode = cache.getGeocode();
-        if (StringUtils.isBlank(geocode)) {
-            return null;
-        }
+    /**
+     * Load the lazily initialized fields of a cache and return them as partial cache (all other fields unset).
+     *
+     * @param geocode
+     * @return
+     */
+    public static Geocache loadCacheTexts(final String geocode) {
+        final Geocache partial = new Geocache();
+
+        // in case of database issues, we still need to return a result to avoid endless loops
+        partial.setDescription(StringUtils.EMPTY);
+        partial.setShortDescription(StringUtils.EMPTY);
+        partial.setHint(StringUtils.EMPTY);
+        partial.setLocation(StringUtils.EMPTY);
+
         init();
 
         try {
@@ -2679,10 +2689,10 @@ public class cgData {
                     "1");
 
             if (cursor.moveToFirst()) {
-                cache.setDescription(StringUtils.defaultString(cursor.getString(0)));
-                cache.setShortDescription(StringUtils.defaultString(cursor.getString(1)));
-                cache.setHint(StringUtils.defaultString(cursor.getString(2)));
-                cache.setLocation(StringUtils.defaultString(cursor.getString(3)));
+                partial.setDescription(StringUtils.defaultString(cursor.getString(0)));
+                partial.setShortDescription(StringUtils.defaultString(cursor.getString(1)));
+                partial.setHint(StringUtils.defaultString(cursor.getString(2)));
+                partial.setLocation(StringUtils.defaultString(cursor.getString(3)));
             }
 
             cursor.close();
@@ -2692,7 +2702,7 @@ public class cgData {
             Log.e("cgData.getCacheDescription", e);
         }
 
-        return null;
+        return partial;
     }
 
     /**
diff --git a/tests/src/cgeo/CGeoTestCase.java b/tests/src/cgeo/CGeoTestCase.java
index 31fde34..6a63cbc 100644
--- a/tests/src/cgeo/CGeoTestCase.java
+++ b/tests/src/cgeo/CGeoTestCase.java
@@ -1,6 +1,8 @@
 package cgeo;
 
+import cgeo.geocaching.cgData;
 import cgeo.geocaching.cgeoapplication;
+import cgeo.geocaching.enumerations.LoadFlags;
 
 import android.test.ApplicationTestCase;
 
@@ -16,4 +18,9 @@ public abstract class CGeoTestCase extends ApplicationTestCase<cgeoapplication>
         createApplication();
     }
 
+    /** Remove cache from DB and cache to ensure that the cache is not loaded from the database */
+    protected static void deleteCacheFromDB(String geocode) {
+        cgData.removeCache(geocode, LoadFlags.REMOVE_ALL);
+    }
+
 }
diff --git a/tests/src/cgeo/geocaching/cgeoApplicationTest.java b/tests/src/cgeo/geocaching/cgeoApplicationTest.java
index c1b01e7..9c882a5 100644
--- a/tests/src/cgeo/geocaching/cgeoApplicationTest.java
+++ b/tests/src/cgeo/geocaching/cgeoApplicationTest.java
@@ -406,11 +406,6 @@ public class cgeoApplicationTest extends CGeoTestCase {
     }
 
     /** Remove cache from DB and cache to ensure that the cache is not loaded from the database */
-    private static void deleteCacheFromDB(String geocode) {
-        cgData.removeCache(geocode, LoadFlags.REMOVE_ALL);
-    }
-
-    /** Remove cache from DB and cache to ensure that the cache is not loaded from the database */
     private static void deleteCacheFromDBAndLogout(String geocode) {
         deleteCacheFromDB(geocode);
 
diff --git a/tests/src/cgeo/geocaching/connector/oc/OCXMLTest.java b/tests/src/cgeo/geocaching/connector/oc/OCXMLTest.java
index a1992bd..46c3fd1 100644
--- a/tests/src/cgeo/geocaching/connector/oc/OCXMLTest.java
+++ b/tests/src/cgeo/geocaching/connector/oc/OCXMLTest.java
@@ -3,7 +3,6 @@ package cgeo.geocaching.connector.oc;
 import cgeo.CGeoTestCase;
 import cgeo.geocaching.Geocache;
 import cgeo.geocaching.Settings;
-import cgeo.geocaching.connector.oc.OCXMLClient;
 import cgeo.geocaching.enumerations.CacheType;
 
 import java.util.Collection;
@@ -80,4 +79,25 @@ public class OCXMLTest extends CGeoTestCase {
             Settings.setExcludeMine(oldExcludeMine);
         }
     }
+
+    public static void testFetchTwiceDuplicatesDescription() {
+        final String geoCode = "OCEFBA";
+        final String description = "<p><span>Bei dem Cache kannst du einen kleinen Schatz bergen. Bitte lege aber einen ander Schatz in das Döschen. Achtung vor Automuggels.</span></p>";
+
+        deleteCacheFromDB(geoCode);
+        Geocache cache = OCXMLClient.getCache(geoCode);
+        assertNotNull(cache);
+        try {
+            assertEquals(geoCode, cache.getGeocode());
+            assertEquals(description, cache.getDescription());
+            cache.store(null);
+
+            // reload, make sure description is not duplicated
+            cache = OCXMLClient.getCache(geoCode);
+            assertNotNull(cache);
+            assertEquals(description, cache.getDescription());
+        } finally {
+            deleteCacheFromDB(geoCode);
+        }
+    }
 }