reduce memory usage for cache lists

* lazily load some of the strings in caches
* remove latlon, it was never used except in parsing itself
* change some collection handling to avoid huge memory hogs

8 files changed, 110 insertions, 75 deletions

diff --git a/main/src/cgeo/geocaching/Geocache.java b/main/src/cgeo/geocaching/Geocache.java
index cfa66bb..18a315c 100644
--- a/main/src/cgeo/geocaching/Geocache.java
+++ b/main/src/cgeo/geocaching/Geocache.java
@@ -70,19 +70,30 @@ public class Geocache implements ICache, IWaypoint {
     private String ownerDisplayName = "";
     private String ownerUserId = "";
     private Date hidden = null;
-    private String hint = "";
+    /**
+     * lazy initialized
+     */
+    private String hint = null;
     private CacheSize size = CacheSize.UNKNOWN;
     private float difficulty = 0;
     private float terrain = 0;
     private Float direction = null;
     private Float distance = null;
-    private String latlon = "";
-    private String location = "";
+    /**
+     * lazy initialized
+     */
+    private String location = null;
     private Geopoint coords = null;
     private boolean reliableLatLon = false;
     private Double elevation = null;
     private String personalNote = null;
-    private String shortdesc = "";
+    /**
+     * lazy initialized
+     */
+    private String shortdesc = null;
+    /**
+     * lazy initialized
+     */
     private String description = null;
     private boolean disabled = false;
     private boolean archived = false;
@@ -234,8 +245,8 @@ public class Geocache implements ICache, IWaypoint {
         if (hidden == null) {
             hidden = other.hidden;
         }
-        if (StringUtils.isBlank(hint)) {
-            hint = other.hint;
+        if (StringUtils.isBlank(getHint())) {
+            hint = other.getHint();
         }
         if (size == null || CacheSize.UNKNOWN == size) {
             size = other.size;
@@ -252,11 +263,8 @@ public class Geocache implements ICache, IWaypoint {
         if (distance == null) {
             distance = other.distance;
         }
-        if (StringUtils.isBlank(latlon)) {
-            latlon = other.latlon;
-        }
-        if (StringUtils.isBlank(location)) {
-            location = other.location;
+        if (StringUtils.isBlank(getLocation())) {
+            location = other.getLocation();
         }
         if (coords == null) {
             coords = other.coords;
@@ -267,11 +275,11 @@ public class Geocache implements ICache, IWaypoint {
         if (personalNote == null) { // don't use StringUtils.isBlank here. Otherwise we cannot recognize a note which was deleted on GC
             personalNote = other.personalNote;
         }
-        if (StringUtils.isBlank(shortdesc)) {
-            shortdesc = other.shortdesc;
+        if (StringUtils.isBlank(getShortDescription())) {
+            shortdesc = other.getShortDescription();
         }
-        if (StringUtils.isBlank(description)) {
-            description = other.description;
+        if (StringUtils.isBlank(getDescription())) {
+            description = other.getDescription();
         }
         // FIXME: this makes no sense to favor this over the other. 0 should not be a special case here as it is
         // in the range of acceptable values. This is probably the case at other places (rating, votes, etc.) too.
@@ -367,17 +375,16 @@ public class Geocache implements ICache, IWaypoint {
                 listId == other.listId &&
                 StringUtils.equalsIgnoreCase(ownerDisplayName, other.ownerDisplayName) &&
                 StringUtils.equalsIgnoreCase(ownerUserId, other.ownerUserId) &&
-                StringUtils.equalsIgnoreCase(description, other.description) &&
+                StringUtils.equalsIgnoreCase(getDescription(), other.getDescription()) &&
                 StringUtils.equalsIgnoreCase(personalNote, other.personalNote) &&
-                StringUtils.equalsIgnoreCase(shortdesc, other.shortdesc) &&
-                StringUtils.equalsIgnoreCase(latlon, other.latlon) &&
-                StringUtils.equalsIgnoreCase(location, other.location) &&
+                StringUtils.equalsIgnoreCase(getShortDescription(), other.getShortDescription()) &&
+                StringUtils.equalsIgnoreCase(getLocation(), other.getLocation()) &&
                 favorite == other.favorite &&
                 favoritePoints == other.favoritePoints &&
                 onWatchlist == other.onWatchlist &&
                 (hidden != null ? hidden.equals(other.hidden) : null == other.hidden) &&
                 StringUtils.equalsIgnoreCase(guid, other.guid) &&
-                StringUtils.equalsIgnoreCase(hint, other.hint) &&
+                StringUtils.equalsIgnoreCase(getHint(), other.getHint()) &&
                 StringUtils.equalsIgnoreCase(cacheId, other.cacheId) &&
                 (direction != null ? direction.equals(other.direction) : null == other.direction) &&
                 (distance != null ? distance.equals(other.distance) : null == other.distance) &&
@@ -605,21 +612,39 @@ public class Geocache implements ICache, IWaypoint {
         return ownerUserId;
     }
 
+    /**
+     * Attention, calling this method may trigger a database access for the cache!
+     */
     @Override
     public String getHint() {
+        initializeCacheTexts();
         return hint;
     }
 
+    /**
+     * Attention, calling this method may trigger a database access for the cache!
+     */
     @Override
     public String getDescription() {
-        if (description == null) {
-            description = StringUtils.defaultString(cgData.getCacheDescription(geocode));
-        }
+        initializeCacheTexts();
         return description;
     }
 
+    /**
+     * loads long text parts of a cache on demand (but all fields together)
+     */
+    private void initializeCacheTexts() {
+        if (description == null || shortdesc == null || hint == null || location == null) {
+            cgData.loadCacheTexts(this);
+        }
+    }
+
+    /**
+     * Attention, calling this method may trigger a database access for the cache!
+     */
     @Override
     public String getShortDescription() {
+        initializeCacheTexts();
         return shortdesc;
     }
 
@@ -642,8 +667,12 @@ public class Geocache implements ICache, IWaypoint {
         return guid;
     }
 
+    /**
+     * Attention, calling this method may trigger a database access for the cache!
+     */
     @Override
     public String getLocation() {
+        initializeCacheTexts();
         return location;
     }
 
@@ -843,14 +872,6 @@ public class Geocache implements ICache, IWaypoint {
         this.distance = distance;
     }
 
-    public String getLatlon() {
-        return latlon;
-    }
-
-    public void setLatlon(String latlon) {
-        this.latlon = latlon;
-    }
-
     @Override
     public Geopoint getCoords() {
         return coords;
@@ -879,11 +900,7 @@ public class Geocache implements ICache, IWaypoint {
         this.elevation = elevation;
     }
 
-    public String getShortdesc() {
-        return shortdesc;
-    }
-
-    public void setShortdesc(String shortdesc) {
+    public void setShortDescription(String shortdesc) {
         this.shortdesc = shortdesc;
     }
 
diff --git a/main/src/cgeo/geocaching/ICache.java b/main/src/cgeo/geocaching/ICache.java
index c852d05..6d0d89a 100644
--- a/main/src/cgeo/geocaching/ICache.java
+++ b/main/src/cgeo/geocaching/ICache.java
@@ -4,7 +4,6 @@
 package cgeo.geocaching;
 
 import cgeo.geocaching.enumerations.LogType;
-import cgeo.geocaching.utils.LazyInitializedList;
 
 import java.util.Date;
 import java.util.List;
diff --git a/main/src/cgeo/geocaching/cgData.java b/main/src/cgeo/geocaching/cgData.java
index 13546f3..cbbd852 100644
--- a/main/src/cgeo/geocaching/cgData.java
+++ b/main/src/cgeo/geocaching/cgData.java
@@ -69,6 +69,9 @@ public class cgData {
             "inventoryunknown", "onWatchlist", "reliable_latlon", "coordsChanged", "latitude", "longitude",  "finalDefined", "_id", "inventorycoins", "inventorytags"
             // reason is replaced by listId in Geocache
     };
+
+    //TODO: remove "latlon" field from cache table
+
     /** The list of fields needed for mapping. */
     private static final String[] WAYPOINT_COLUMNS = new String[] { "_id", "geocode", "updated", "type", "prefix", "lookup", "name", "latlon", "latitude", "longitude", "note", "own" };
 
@@ -998,14 +1001,13 @@ public class cgData {
         values.put("size", cache.getSize() == null ? "" : cache.getSize().id);
         values.put("difficulty", cache.getDifficulty());
         values.put("terrain", cache.getTerrain());
-        values.put("latlon", cache.getLatlon());
         values.put("location", cache.getLocation());
         values.put("distance", cache.getDistance());
         values.put("direction", cache.getDirection());
         putCoords(values, cache.getCoords());
         values.put("reliable_latlon", cache.isReliableLatLon() ? 1 : 0);
         values.put("elevation", cache.getElevation());
-        values.put("shortdesc", cache.getShortdesc());
+        values.put("shortdesc", cache.getShortDescription());
         values.put("personal_note", cache.getPersonalNote());
         values.put("description", cache.getDescription());
         values.put("favourite_cnt", cache.getFavoritePoints());
@@ -1444,9 +1446,7 @@ public class cgData {
             return Collections.emptySet();
         }
 
-
-        Log.d("cgData.loadCachesFromGeocodes(" + geocodes.toString() + ") from DB");
-
+        // do not log the entire collection of geo codes to the debug log. This can be more than 100 KB of text for large lists!
         init();
 
         final StringBuilder query = new StringBuilder("SELECT ");
@@ -1573,7 +1573,7 @@ public class cgData {
         if (dateValue != 0) {
             cache.setHidden(new Date(dateValue));
         }
-        cache.setHint(cursor.getString(cacheColumnIndex[13]));
+        // do not set cache.hint
         cache.setSize(CacheSize.getById(cursor.getString(cacheColumnIndex[14])));
         cache.setDifficulty(cursor.getFloat(cacheColumnIndex[15]));
         int index = cacheColumnIndex[16];
@@ -1589,8 +1589,7 @@ public class cgData {
             cache.setDistance(cursor.getFloat(index));
         }
         cache.setTerrain(cursor.getFloat(cacheColumnIndex[18]));
-        cache.setLatlon(cursor.getString(cacheColumnIndex[19]));
-        cache.setLocation(cursor.getString(cacheColumnIndex[20]));
+        // do not set cache.location
         cache.setCoords(getCoords(cursor, cacheColumnIndex[37], cacheColumnIndex[38]));
         index = cacheColumnIndex[21];
         if (cursor.isNull(index)) {
@@ -1599,8 +1598,8 @@ public class cgData {
             cache.setElevation(cursor.getDouble(index));
         }
         cache.setPersonalNote(cursor.getString(cacheColumnIndex[22]));
-        cache.setShortdesc(cursor.getString(cacheColumnIndex[23]));
-        // do not set cache.description !
+        // do not set cache.shortdesc
+        // do not set cache.description
         cache.setFavoritePoints(cursor.getInt(cacheColumnIndex[24]));
         cache.setRating(cursor.getFloat(cacheColumnIndex[25]));
         cache.setVotes(cursor.getInt(cacheColumnIndex[26]));
@@ -2635,18 +2634,32 @@ public class cgData {
         return result;
     }
 
-    public static String getCacheDescription(String geocode) {
+    public static String loadCacheTexts(final Geocache cache) {
+        final String geocode = cache.getGeocode();
         if (StringUtils.isBlank(geocode)) {
             return null;
         }
         init();
 
         try {
-            final SQLiteStatement description = PreparedStatements.getDescriptionOfGeocode();
-            synchronized (description) {
-                description.bindString(1, geocode);
-                return description.simpleQueryForString();
+            final Cursor cursor = database.query(
+                    dbTableCaches,
+                    new String[] { "description", "shortdesc", "hint", "location" },
+                    "geocode = ?",
+                    new String[] { geocode },
+                    null,
+                    null,
+                    null,
+                    "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)));
             }
+
+            cursor.close();
         } catch (SQLiteDoneException e) {
             // Do nothing, it only means we have no information on the cache
         } catch (Exception e) {
@@ -2672,14 +2685,14 @@ public class cgData {
         newlyCreatedDatabase = false;
     }
 
-    private static String whereGeocodeIn(Set<String> geocodes) {
+    private static StringBuilder whereGeocodeIn(Set<String> geocodes) {
         final StringBuilder where = new StringBuilder();
 
         if (geocodes != null && !geocodes.isEmpty()) {
             StringBuilder all = new StringBuilder();
             for (String geocode : geocodes) {
                 if (all.length() > 0) {
-                    all.append(", ");
+                    all.append(',');
                 }
                 all.append(DatabaseUtils.sqlEscapeString(geocode));
             }
@@ -2687,7 +2700,7 @@ public class cgData {
             where.append("geocode in (").append(all).append(')');
         }
 
-        return where.toString();
+        return where;
     }
 
     /**
@@ -2839,10 +2852,6 @@ public class cgData {
             return getStatement("InsertAttribute", "INSERT INTO " + dbTableAttributes + " (geocode, updated, attribute) VALUES (?, ?, ?)");
         }
 
-        private static SQLiteStatement getDescriptionOfGeocode() {
-            return getStatement("descriptionFromGeocode", "SELECT description FROM " + dbTableCaches + " WHERE geocode = ?");
-        }
-
         private static SQLiteStatement getListIdOfGeocode() {
             return getStatement("listFromGeocode", "SELECT reason FROM " + dbTableCaches + " WHERE geocode = ?");
         }
diff --git a/main/src/cgeo/geocaching/cgeocaches.java b/main/src/cgeo/geocaching/cgeocaches.java
index cf10fb5..7fbb112 100644
--- a/main/src/cgeo/geocaching/cgeocaches.java
+++ b/main/src/cgeo/geocaching/cgeocaches.java
@@ -281,11 +281,15 @@ public class cgeocaches extends AbstractListActivity implements FilteredActivity
 
     private void replaceCacheListFromSearch() {
         if (search != null) {
-            final Set<Geocache> cachesFromSearchResult = search.getCachesFromSearchResult(LoadFlags.LOAD_CACHE_OR_DB);
             runOnUiThread(new Runnable() {
                 @Override
                 public void run() {
                     cacheList.clear();
+
+                    // The database search was moved into the UI call intentionally. If this is done before the runOnUIThread,
+                    // then we have 2 sets of caches in memory. This can lead to OOM for huge cache lists.
+                    final Set<Geocache> cachesFromSearchResult = search.getCachesFromSearchResult(LoadFlags.LOAD_CACHE_OR_DB);
+
                     cacheList.addAll(cachesFromSearchResult);
                     adapter.reFilter();
                     updateTitle();
diff --git a/main/src/cgeo/geocaching/connector/gc/GCParser.java b/main/src/cgeo/geocaching/connector/gc/GCParser.java
index 2117053..5481b0c 100644
--- a/main/src/cgeo/geocaching/connector/gc/GCParser.java
+++ b/main/src/cgeo/geocaching/connector/gc/GCParser.java
@@ -1,6 +1,7 @@
 package cgeo.geocaching.connector.gc;
 
 import cgeo.geocaching.Geocache;
+import cgeo.geocaching.Image;
 import cgeo.geocaching.LogEntry;
 import cgeo.geocaching.R;
 import cgeo.geocaching.SearchResult;
@@ -9,7 +10,6 @@ import cgeo.geocaching.Trackable;
 import cgeo.geocaching.TrackableLog;
 import cgeo.geocaching.Waypoint;
 import cgeo.geocaching.cgData;
-import cgeo.geocaching.Image;
 import cgeo.geocaching.cgeoapplication;
 import cgeo.geocaching.enumerations.CacheSize;
 import cgeo.geocaching.enumerations.CacheType;
@@ -29,7 +29,6 @@ import cgeo.geocaching.network.Parameters;
 import cgeo.geocaching.ui.DirectionImage;
 import cgeo.geocaching.utils.BaseUtils;
 import cgeo.geocaching.utils.CancellableHandler;
-import cgeo.geocaching.utils.LazyInitializedList;
 import cgeo.geocaching.utils.Log;
 import cgeo.geocaching.utils.MatcherWrapper;
 
@@ -455,10 +454,10 @@ public abstract class GCParser {
         cache.setOnWatchlist(BaseUtils.matches(page, GCConstants.PATTERN_WATCHLIST));
 
         // latitude and longitude. Can only be retrieved if user is logged in
-        cache.setLatlon(BaseUtils.getMatch(page, GCConstants.PATTERN_LATLON, true, cache.getLatlon()));
-        if (StringUtils.isNotEmpty(cache.getLatlon())) {
+        String latlon = BaseUtils.getMatch(page, GCConstants.PATTERN_LATLON, true, "");
+        if (StringUtils.isNotEmpty(latlon)) {
             try {
-                cache.setCoords(new Geopoint(cache.getLatlon()));
+                cache.setCoords(new Geopoint(latlon));
                 cache.setReliableLatLon(true);
             } catch (Geopoint.GeopointException e) {
                 Log.w("GCParser.parseCache: Failed to parse cache coordinates", e);
@@ -466,7 +465,7 @@ public abstract class GCParser {
         }
 
         // cache location
-        cache.setLocation(BaseUtils.getMatch(page, GCConstants.PATTERN_LOCATION, true, cache.getLocation()));
+        cache.setLocation(BaseUtils.getMatch(page, GCConstants.PATTERN_LOCATION, true, ""));
 
         // cache hint
         String result = BaseUtils.getMatch(page, GCConstants.PATTERN_HINT, false, null);
@@ -484,7 +483,7 @@ public abstract class GCParser {
         cache.setPersonalNote(BaseUtils.getMatch(page, GCConstants.PATTERN_PERSONALNOTE, true, cache.getPersonalNote()));
 
         // cache short description
-        cache.setShortdesc(BaseUtils.getMatch(page, GCConstants.PATTERN_SHORTDESC, true, cache.getShortdesc()));
+        cache.setShortDescription(BaseUtils.getMatch(page, GCConstants.PATTERN_SHORTDESC, true, ""));
 
         // cache description
         cache.setDescription(BaseUtils.getMatch(page, GCConstants.PATTERN_DESC, true, ""));
@@ -666,7 +665,7 @@ public abstract class GCParser {
                     waypoint.setLookup(BaseUtils.getMatch(wp[5], GCConstants.PATTERN_WPPREFIXORLOOKUPORLATLON, true, 2, waypoint.getLookup(), false));
 
                     // waypoint latitude and logitude
-                    String latlon = Html.fromHtml(BaseUtils.getMatch(wp[7], GCConstants.PATTERN_WPPREFIXORLOOKUPORLATLON, false, 2, "", false)).toString().trim();
+                    latlon = Html.fromHtml(BaseUtils.getMatch(wp[7], GCConstants.PATTERN_WPPREFIXORLOOKUPORLATLON, false, 2, "", false)).toString().trim();
                     if (!StringUtils.startsWith(latlon, "???")) {
                         waypoint.setLatlon(latlon);
                         waypoint.setCoords(new Geopoint(latlon));
diff --git a/main/src/cgeo/geocaching/connector/oc/OC11XMLParser.java b/main/src/cgeo/geocaching/connector/oc/OC11XMLParser.java
index a9c041c..e44b0a5 100644
--- a/main/src/cgeo/geocaching/connector/oc/OC11XMLParser.java
+++ b/main/src/cgeo/geocaching/connector/oc/OC11XMLParser.java
@@ -464,7 +464,7 @@ public class OC11XMLParser {
             public void end() {
                 final Geocache cache = caches.get(descHolder.cacheId);
                 if (cache != null) {
-                    cache.setShortdesc(descHolder.shortDesc);
+                    cache.setShortDescription(descHolder.shortDesc);
                     cache.setDescription(cache.getDescription() + descHolder.desc);
                     cache.setHint(descHolder.hint);
                 }
diff --git a/main/src/cgeo/geocaching/files/GPXParser.java b/main/src/cgeo/geocaching/files/GPXParser.java
index a691677..5647d14 100644
--- a/main/src/cgeo/geocaching/files/GPXParser.java
+++ b/main/src/cgeo/geocaching/files/GPXParser.java
@@ -331,7 +331,7 @@ public abstract class GPXParser extends FileParser {
                     // lookup cache for waypoint in already parsed caches
                     final Geocache cacheForWaypoint = cgData.loadCache(cacheGeocodeForWaypoint, LoadFlags.LOAD_CACHE_OR_DB);
                     if (cacheForWaypoint != null) {
-                        final Waypoint waypoint = new Waypoint(cache.getShortdesc(), convertWaypointSym2Type(sym), false);
+                        final Waypoint waypoint = new Waypoint(cache.getShortDescription(), convertWaypointSym2Type(sym), false);
                         waypoint.setId(-1);
                         waypoint.setGeocode(cacheGeocodeForWaypoint);
                         waypoint.setPrefix(cache.getName().substring(0, 2));
@@ -388,7 +388,7 @@ public abstract class GPXParser extends FileParser {
             public void end(String body) {
                 desc = body;
 
-                cache.setShortdesc(validate(body));
+                cache.setShortDescription(validate(body));
             }
         });
 
@@ -638,7 +638,7 @@ public abstract class GPXParser extends FileParser {
 
                 @Override
                 public void end(String shortDesc) {
-                    cache.setShortdesc(validate(shortDesc));
+                    cache.setShortDescription(validate(shortDesc));
                 }
             });
 
@@ -859,6 +859,13 @@ public abstract class GPXParser extends FileParser {
         cmt = null;
 
         cache = new Geocache(this);
+
+        // explicitly set all properties which could lead to database access, if left as null value
+        cache.setLocation("");
+        cache.setDescription("");
+        cache.setShortDescription("");
+        cache.setHint("");
+
         for (int i = 0; i < userData.length; i++) {
             userData[i] = null;
         }
diff --git a/tests/src/cgeo/geocaching/files/GPXParserTest.java b/tests/src/cgeo/geocaching/files/GPXParserTest.java
index 1849124..3579d5a 100644
--- a/tests/src/cgeo/geocaching/files/GPXParserTest.java
+++ b/tests/src/cgeo/geocaching/files/GPXParserTest.java
@@ -4,8 +4,8 @@ import cgeo.geocaching.Geocache;
 import cgeo.geocaching.LogEntry;
 import cgeo.geocaching.SearchResult;
 import cgeo.geocaching.StoredList;
-import cgeo.geocaching.cgData;
 import cgeo.geocaching.Waypoint;
+import cgeo.geocaching.cgData;
 import cgeo.geocaching.enumerations.CacheSize;
 import cgeo.geocaching.enumerations.CacheType;
 import cgeo.geocaching.enumerations.LoadFlags;
@@ -52,7 +52,7 @@ public class GPXParserTest extends AbstractResourceInstrumentationTestCase {
         assertEquals(1.0f, cache.getDifficulty());
         assertEquals(5.0f, cache.getTerrain());
         assertEquals("Baden-Württemberg, Germany", cache.getLocation());
-        assertEquals("Ein alter Kindheitstraum, ein Schatz auf einer unbewohnten Insel.\nA old dream of my childhood, a treasure on a lonely island.", cache.getShortdesc());
+        assertEquals("Ein alter Kindheitstraum, ein Schatz auf einer unbewohnten Insel.\nA old dream of my childhood, a treasure on a lonely island.", cache.getShortDescription());
         assertEquals(new Geopoint(48.859683, 9.1874), cache.getCoords());
         return cache;
     }
@@ -77,7 +77,7 @@ public class GPXParserTest extends AbstractResourceInstrumentationTestCase {
         assertEquals(1.0f, cache.getDifficulty());
         assertEquals(4.0f, cache.getTerrain());
         assertEquals("Baden-Württemberg, Germany", cache.getLocation());
-        assertEquals("Ein alter Kindheitstraum, ein Schatz auf einer unbewohnten Insel. A old dream of my childhood, a treasure on a lonely is", cache.getShortdesc());
+        assertEquals("Ein alter Kindheitstraum, ein Schatz auf einer unbewohnten Insel. A old dream of my childhood, a treasure on a lonely is", cache.getShortDescription());
         assertEquals(new Geopoint(48.85968, 9.18740), cache.getCoords());
         assertTrue(cache.isReliableLatLon());
     }