diff options
| -rw-r--r-- | main/src/cgeo/geocaching/DataStore.java | 114 | ||||
| -rw-r--r-- | main/src/cgeo/geocaching/utils/MiscUtils.java | 40 | ||||
| -rw-r--r-- | tests/src/cgeo/geocaching/utils/MiscUtilsTest.java | 57 |
3 files changed, 56 insertions, 155 deletions
diff --git a/main/src/cgeo/geocaching/DataStore.java b/main/src/cgeo/geocaching/DataStore.java index 623301b..302bfc3 100644 --- a/main/src/cgeo/geocaching/DataStore.java +++ b/main/src/cgeo/geocaching/DataStore.java @@ -20,7 +20,6 @@ import cgeo.geocaching.settings.Settings; import cgeo.geocaching.ui.dialog.Dialogs; import cgeo.geocaching.utils.FileUtils; import cgeo.geocaching.utils.Log; -import cgeo.geocaching.utils.MiscUtils; import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.collections4.MapUtils; @@ -1314,7 +1313,7 @@ public class DataStore { */ private static void removeOutdatedWaypointsOfCache(final @NonNull Geocache cache, final @NonNull Collection<String> remainingWaypointIds) { final String idList = StringUtils.join(remainingWaypointIds, ','); - database.delete(dbTableWaypoints, "geocode = ? AND _id NOT in (" + idList + ")", new String[]{cache.getGeocode()}); + database.delete(dbTableWaypoints, "geocode = ? AND _id NOT in (" + idList + ")", new String[] { cache.getGeocode() }); } /** @@ -1559,8 +1558,8 @@ public class DataStore { return new HashSet<Geocache>(); } - final Set<Geocache> result = new HashSet<Geocache>(); - final List<String> remaining = new LinkedList<String>(geocodes); + Set<Geocache> result = new HashSet<Geocache>(); + Set<String> remaining = new HashSet<String>(geocodes); if (loadFlags.contains(LoadFlag.LOAD_CACHE_BEFORE)) { for (String geocode : new HashSet<String>(remaining)) { @@ -1580,7 +1579,7 @@ public class DataStore { loadFlags.contains(LoadFlag.LOAD_INVENTORY) || loadFlags.contains(LoadFlag.LOAD_OFFLINE_LOG)) { - final Collection<Geocache> cachesFromDB = loadCachesFromGeocodes(remaining, loadFlags); + final Set<Geocache> cachesFromDB = loadCachesFromGeocodes(remaining, loadFlags); result.addAll(cachesFromDB); for (final Geocache cache : cachesFromDB) { remaining.remove(cache.getGeocode()); @@ -1606,11 +1605,15 @@ public class DataStore { /** * Load caches. * - * @param allGeocodes + * @param geocodes * @param loadFlags * @return Set of loaded caches. Never null. */ - private static Collection<Geocache> loadCachesFromGeocodes(final List<String> allGeocodes, final EnumSet<LoadFlag> loadFlags) { + private static Set<Geocache> loadCachesFromGeocodes(final Set<String> geocodes, final EnumSet<LoadFlag> loadFlags) { + if (CollectionUtils.isEmpty(geocodes)) { + return Collections.emptySet(); + } + // 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(); @@ -1625,73 +1628,68 @@ public class DataStore { } query.append(" WHERE ").append(dbTableCaches).append('.'); + query.append(DataStore.whereGeocodeIn(geocodes)); - final String queryBegin = query.toString(); - final List<Geocache> result = new ArrayList<Geocache>(allGeocodes.size()); - - for (List<String> geocodes: MiscUtils.buffer(allGeocodes, 50)) { - final Cursor cursor = database.rawQuery(queryBegin + DataStore.whereGeocodeIn(geocodes), null); - try { - final Set<Geocache> caches = new HashSet<Geocache>(); - int logIndex = -1; + Cursor cursor = database.rawQuery(query.toString(), null); + try { + final Set<Geocache> caches = new HashSet<Geocache>(); + int logIndex = -1; - while (cursor.moveToNext()) { - final Geocache cache = DataStore.createCacheFromDatabaseContent(cursor); + while (cursor.moveToNext()) { + Geocache cache = DataStore.createCacheFromDatabaseContent(cursor); - if (loadFlags.contains(LoadFlag.LOAD_ATTRIBUTES)) { - cache.setAttributes(loadAttributes(cache.getGeocode())); - } + if (loadFlags.contains(LoadFlag.LOAD_ATTRIBUTES)) { + cache.setAttributes(loadAttributes(cache.getGeocode())); + } - if (loadFlags.contains(LoadFlag.LOAD_WAYPOINTS)) { - final List<Waypoint> waypoints = loadWaypoints(cache.getGeocode()); - if (CollectionUtils.isNotEmpty(waypoints)) { - cache.setWaypoints(waypoints, false); - } + if (loadFlags.contains(LoadFlag.LOAD_WAYPOINTS)) { + final List<Waypoint> waypoints = loadWaypoints(cache.getGeocode()); + if (CollectionUtils.isNotEmpty(waypoints)) { + cache.setWaypoints(waypoints, false); } + } - if (loadFlags.contains(LoadFlag.LOAD_SPOILERS)) { - final List<Image> spoilers = loadSpoilers(cache.getGeocode()); - cache.setSpoilers(spoilers); - } + if (loadFlags.contains(LoadFlag.LOAD_SPOILERS)) { + final List<Image> spoilers = loadSpoilers(cache.getGeocode()); + cache.setSpoilers(spoilers); + } - if (loadFlags.contains(LoadFlag.LOAD_LOGS)) { - cache.setLogs(loadLogs(cache.getGeocode())); - final Map<LogType, Integer> logCounts = loadLogCounts(cache.getGeocode()); - if (MapUtils.isNotEmpty(logCounts)) { - cache.getLogCounts().clear(); - cache.getLogCounts().putAll(logCounts); - } + if (loadFlags.contains(LoadFlag.LOAD_LOGS)) { + cache.setLogs(loadLogs(cache.getGeocode())); + final Map<LogType, Integer> logCounts = loadLogCounts(cache.getGeocode()); + if (MapUtils.isNotEmpty(logCounts)) { + cache.getLogCounts().clear(); + cache.getLogCounts().putAll(logCounts); } + } - if (loadFlags.contains(LoadFlag.LOAD_INVENTORY)) { - final List<Trackable> inventory = loadInventory(cache.getGeocode()); - if (CollectionUtils.isNotEmpty(inventory)) { - if (cache.getInventory() == null) { - cache.setInventory(new ArrayList<Trackable>()); - } else { - cache.getInventory().clear(); - } - cache.getInventory().addAll(inventory); + if (loadFlags.contains(LoadFlag.LOAD_INVENTORY)) { + final List<Trackable> inventory = loadInventory(cache.getGeocode()); + if (CollectionUtils.isNotEmpty(inventory)) { + if (cache.getInventory() == null) { + cache.setInventory(new ArrayList<Trackable>()); + } else { + cache.getInventory().clear(); } + cache.getInventory().addAll(inventory); } + } - if (loadFlags.contains(LoadFlag.LOAD_OFFLINE_LOG)) { - if (logIndex < 0) { - logIndex = cursor.getColumnIndex("log"); - } - cache.setLogOffline(!cursor.isNull(logIndex)); + if (loadFlags.contains(LoadFlag.LOAD_OFFLINE_LOG)) { + if (logIndex < 0) { + logIndex = cursor.getColumnIndex("log"); } - cache.addStorageLocation(StorageLocation.DATABASE); - cacheCache.putCacheInCache(cache); - - caches.add(cache); + cache.setLogOffline(!cursor.isNull(logIndex)); } - result.addAll(caches); - } finally { - cursor.close(); + cache.addStorageLocation(StorageLocation.DATABASE); + cacheCache.putCacheInCache(cache); + + caches.add(cache); } + return caches; + } finally { + cursor.close(); } - return result; } diff --git a/main/src/cgeo/geocaching/utils/MiscUtils.java b/main/src/cgeo/geocaching/utils/MiscUtils.java deleted file mode 100644 index 122c4eb..0000000 --- a/main/src/cgeo/geocaching/utils/MiscUtils.java +++ /dev/null @@ -1,40 +0,0 @@ -package cgeo.geocaching.utils; - -import org.apache.commons.collections4.iterators.IteratorIterable; -import org.apache.commons.lang3.NotImplementedException; - -import java.util.Iterator; -import java.util.List; - -final public class MiscUtils { - - private MiscUtils() {} // Do not instantiate - - public static <T> Iterable<List<T>> buffer(final List<T> original, final int n) { - if (n <= 0) { - throw new IllegalArgumentException("buffer size must be positive"); - } - return new IteratorIterable<List<T>>(new Iterator<List<T>>() { - final int size = original.size(); - int next = 0; - - @Override - public boolean hasNext() { - return next < size; - } - - @Override - public List<T> next() { - final List<T> result = original.subList(next, Math.min(next + n, size)); - next += n; - return result; - } - - @Override - public void remove() { - throw new NotImplementedException("remove"); - } - }); - } - -} diff --git a/tests/src/cgeo/geocaching/utils/MiscUtilsTest.java b/tests/src/cgeo/geocaching/utils/MiscUtilsTest.java deleted file mode 100644 index 2e170d2..0000000 --- a/tests/src/cgeo/geocaching/utils/MiscUtilsTest.java +++ /dev/null @@ -1,57 +0,0 @@ -package cgeo.geocaching.utils; - -import static org.assertj.core.api.Assertions.assertThat; - -import java.util.LinkedList; -import java.util.List; - -import junit.framework.TestCase; - -public class MiscUtilsTest extends TestCase { - - public static void testBufferEmpty() { - for (final List<String> s : MiscUtils.buffer(new LinkedList<String>(), 10)) { - assertThat(s).isNotNull(); // only to silence findbugs and the compiler - fail("empty collection should not iterate"); - } - } - - public static void testMultiple() { - final List<Integer> list = new LinkedList<Integer>(); - for (int i = 0; i < 50; i++) { - list.add(i); - } - int count = 0; - for (final List<Integer> subList: MiscUtils.buffer(list, 10)) { - assertThat(subList).hasSize(10); - assertThat(subList.get(0)).as("sublist content").isEqualTo(count * 10); - count++; - } - assertThat(count).isEqualTo(5); - } - - public static void testNonMultiple() { - final List<Integer> list = new LinkedList<Integer>(); - for (int i = 0; i < 48; i++) { - list.add(i); - } - int count = 0; - for (final List<Integer> subList: MiscUtils.buffer(list, 10)) { - assertThat(subList.size()).overridingErrorMessage("each sublist has no more than the allowed number of arguments").isLessThanOrEqualTo(10); - count += subList.size(); - } - assertEquals("all the elements were seen", 48, count); - } - - public static void testArguments() { - try { - MiscUtils.buffer(new LinkedList<Integer>(), 0); - fail("an exception should be raised"); - } catch (final IllegalArgumentException e) { - // Ok - } catch (final Exception e) { - fail("bad exception raised: " + e); - } - } - -} |