From 299a63d4c6e86ed7991a23d3b958b27f3d4d1be5 Mon Sep 17 00:00:00 2001 From: Alex Baker Date: Fri, 26 Jan 2018 16:38:03 -0600 Subject: [PATCH] Remove DISTINCT and GROUP BY, double visitor --- .../com/todoroo/astrid/dao/TaskDaoTests.java | 32 ++++++---------- .../tasks/gtasks/GoogleTaskSyncAdapter.java | 2 +- .../com/todoroo/andlib/data/Property.java | 14 +------ .../todoroo/andlib/data/TodorooCursor.java | 6 --- .../java/com/todoroo/andlib/sql/Query.java | 37 +++---------------- .../com/todoroo/andlib/sql/QueryTemplate.java | 14 ------- .../com/todoroo/andlib/sql/SqlConstants.java | 2 - .../astrid/backup/TasksXmlExporter.java | 4 +- .../astrid/backup/TasksXmlImporter.java | 2 +- .../astrid/core/OldTaskPreferences.java | 2 +- .../java/com/todoroo/astrid/dao/TaskDao.java | 7 ++-- .../astrid/provider/Astrid2TaskProvider.java | 2 +- .../astrid/reminders/ReminderService.java | 2 +- .../AstridOrderedListFragmentHelper.java | 2 +- .../notifications/NotificationManager.java | 2 +- 15 files changed, 32 insertions(+), 98 deletions(-) diff --git a/app/src/androidTest/java/com/todoroo/astrid/dao/TaskDaoTests.java b/app/src/androidTest/java/com/todoroo/astrid/dao/TaskDaoTests.java index be00c29ac..16f05f603 100644 --- a/app/src/androidTest/java/com/todoroo/astrid/dao/TaskDaoTests.java +++ b/app/src/androidTest/java/com/todoroo/astrid/dao/TaskDaoTests.java @@ -7,7 +7,6 @@ package com.todoroo.astrid.dao; import android.support.test.runner.AndroidJUnit4; -import com.todoroo.andlib.data.Property; import com.todoroo.andlib.sql.Query; import com.todoroo.andlib.utility.DateUtilities; import com.todoroo.astrid.dao.TaskDao.TaskCriteria; @@ -21,19 +20,12 @@ import org.tasks.injection.TestComponent; import javax.inject.Inject; import static junit.framework.Assert.assertEquals; -import static junit.framework.Assert.assertFalse; import static junit.framework.Assert.assertNotSame; import static junit.framework.Assert.assertNull; -import static junit.framework.Assert.assertTrue; @RunWith(AndroidJUnit4.class) public class TaskDaoTests extends InjectingTestCase { - public static Property[] IDS = new Property[] { Task.ID }; - - public static Property[] TITLES = new Property[] { Task.ID, - Task.TITLE }; - @Inject TaskDao taskDao; /** @@ -41,13 +33,13 @@ public class TaskDaoTests extends InjectingTestCase { */ @Test public void testTaskCreation() { - assertEquals(0, taskDao.toList(Query.select(IDS)).size()); + assertEquals(0, taskDao.toList(Query.select()).size()); // create task "happy" Task task = new Task(); task.setTitle("happy"); taskDao.save(task); - assertEquals(1, taskDao.toList(Query.select(IDS)).size()); + assertEquals(1, taskDao.toList(Query.select()).size()); long happyId = task.getId(); assertNotSame(Task.NO_ID, happyId); task = taskDao.fetch(happyId); @@ -57,14 +49,14 @@ public class TaskDaoTests extends InjectingTestCase { task = new Task(); task.setTitle("sad"); taskDao.save(task); - assertEquals(2, taskDao.toList(Query.select(IDS)).size()); + assertEquals(2, taskDao.toList(Query.select()).size()); // rename sad to melancholy long sadId = task.getId(); assertNotSame(Task.NO_ID, sadId); task.setTitle("melancholy"); taskDao.save(task); - assertEquals(2, taskDao.toList(Query.select(IDS)).size()); + assertEquals(2, taskDao.toList(Query.select()).size()); // check state task = taskDao.fetch(happyId); @@ -112,10 +104,10 @@ public class TaskDaoTests extends InjectingTestCase { taskDao.save(task); // check is active - assertEquals(5, taskDao.toList(Query.select(TITLES).where(TaskCriteria.isActive())).size()); + assertEquals(5, taskDao.toList(Query.select().where(TaskCriteria.isActive())).size()); // check is visible - assertEquals(5, taskDao.toList(Query.select(TITLES).where(TaskCriteria.isVisible())).size()); + assertEquals(5, taskDao.toList(Query.select().where(TaskCriteria.isVisible())).size()); } /** @@ -123,18 +115,18 @@ public class TaskDaoTests extends InjectingTestCase { */ @Test public void testTDeletion() { - assertEquals(0, taskDao.toList(Query.select(IDS)).size()); + assertEquals(0, taskDao.toList(Query.select()).size()); // create task "happy" Task task = new Task(); task.setTitle("happy"); taskDao.save(task); - assertEquals(1, taskDao.toList(Query.select(IDS)).size()); + assertEquals(1, taskDao.toList(Query.select()).size()); // delete long happyId = task.getId(); assertEquals(1, taskDao.deleteById(happyId)); - assertEquals(0, taskDao.toList(Query.select(IDS)).size()); + assertEquals(0, taskDao.toList(Query.select()).size()); } /** @@ -149,7 +141,7 @@ public class TaskDaoTests extends InjectingTestCase { taskDao.save(task); - assertEquals(0, taskDao.toList(Query.select(IDS)).size()); + assertEquals(0, taskDao.toList(Query.select()).size()); } /** @@ -157,14 +149,14 @@ public class TaskDaoTests extends InjectingTestCase { */ @Test public void testInvalidIndex() { - assertEquals(0, taskDao.toList(Query.select(IDS)).size()); + assertEquals(0, taskDao.toList(Query.select()).size()); assertNull(taskDao.fetch(1)); assertEquals(0, taskDao.deleteById(1)); // make sure db still works - assertEquals(0, taskDao.toList(Query.select(IDS)).size()); + assertEquals(0, taskDao.toList(Query.select()).size()); } @Override diff --git a/app/src/googleplay/java/org/tasks/gtasks/GoogleTaskSyncAdapter.java b/app/src/googleplay/java/org/tasks/gtasks/GoogleTaskSyncAdapter.java index c6fb7f6e8..7392409c4 100644 --- a/app/src/googleplay/java/org/tasks/gtasks/GoogleTaskSyncAdapter.java +++ b/app/src/googleplay/java/org/tasks/gtasks/GoogleTaskSyncAdapter.java @@ -190,7 +190,7 @@ public class GoogleTaskSyncAdapter extends InjectingAbstractThreadedSyncAdapter } private void pushLocalChanges() throws UserRecoverableAuthIOException { - List tasks = taskDao.toList(Query.select(Task.PROPERTIES) + List tasks = taskDao.toList(Query.select() .join(Join.left(GoogleTask.TABLE.as("gt"), Task.ID.eq(Field.field("gt.task")))) .where(Criterion.or( Task.MODIFICATION_DATE.gt(Field.field("gt.last_sync")), diff --git a/app/src/main/java/com/todoroo/andlib/data/Property.java b/app/src/main/java/com/todoroo/andlib/data/Property.java index e0c493b73..a4486e664 100644 --- a/app/src/main/java/com/todoroo/andlib/data/Property.java +++ b/app/src/main/java/com/todoroo/andlib/data/Property.java @@ -108,8 +108,6 @@ public abstract class Property extends Field implements Cloneable { RETURN visitLong(Property property, PARAMETER data); - RETURN visitDouble(Property property, PARAMETER data); - RETURN visitString(Property property, PARAMETER data); } @@ -231,18 +229,10 @@ public abstract class Property extends Field implements Cloneable { // --- pseudo-properties /** Runs a SQL function and returns the result as a string */ - public static class IntegerFunctionProperty extends IntegerProperty { - public IntegerFunctionProperty(String function, String columnName) { - super(columnName, function); - alias = columnName; - } - } - - /** Counting in aggregated tables. Returns the result of COUNT(1) */ - public static final class CountProperty extends IntegerFunctionProperty { + public static class CountProperty extends IntegerProperty { public CountProperty() { super("COUNT(1)", "count"); + alias = "count"; } } - } diff --git a/app/src/main/java/com/todoroo/andlib/data/TodorooCursor.java b/app/src/main/java/com/todoroo/andlib/data/TodorooCursor.java index 49275def6..68b703e1d 100644 --- a/app/src/main/java/com/todoroo/andlib/data/TodorooCursor.java +++ b/app/src/main/java/com/todoroo/andlib/data/TodorooCursor.java @@ -114,12 +114,6 @@ public class TodorooCursor extends CursorWrapper { return cursor.getLong(column); } - @Override - public Object visitDouble(Property property, TodorooCursor cursor) { - int column = columnIndex(property, cursor); - return cursor.getDouble(column); - } - @Override public Object visitString(Property property, TodorooCursor cursor) { int column = columnIndex(property, cursor); diff --git a/app/src/main/java/com/todoroo/andlib/sql/Query.java b/app/src/main/java/com/todoroo/andlib/sql/Query.java index 3379d4f58..b0d1d24fe 100644 --- a/app/src/main/java/com/todoroo/andlib/sql/Query.java +++ b/app/src/main/java/com/todoroo/andlib/sql/Query.java @@ -6,14 +6,13 @@ package com.todoroo.andlib.sql; import com.todoroo.andlib.data.Property; +import com.todoroo.astrid.data.Task; import java.util.ArrayList; import static com.todoroo.andlib.sql.SqlConstants.ALL; import static com.todoroo.andlib.sql.SqlConstants.COMMA; -import static com.todoroo.andlib.sql.SqlConstants.DISTINCT; import static com.todoroo.andlib.sql.SqlConstants.FROM; -import static com.todoroo.andlib.sql.SqlConstants.GROUP_BY; import static com.todoroo.andlib.sql.SqlConstants.LIMIT; import static com.todoroo.andlib.sql.SqlConstants.ORDER_BY; import static com.todoroo.andlib.sql.SqlConstants.SELECT; @@ -28,23 +27,19 @@ public final class Query { private final ArrayList criterions = new ArrayList<>(); private final ArrayList fields = new ArrayList<>(); private final ArrayList joins = new ArrayList<>(); - private final ArrayList groupBies = new ArrayList<>(); private final ArrayList orders = new ArrayList<>(); private int limits = -1; - private boolean distinct = false; private Query(Field... fields) { this.fields.addAll(asList(fields)); } - public static Query select(Field... fields) { - return new Query(fields); + public static Query select() { + return new Query(Task.PROPERTIES); } - public static Query selectDistinct(Field... fields) { - Query query = new Query(fields); - query.distinct = true; - return query; + public static Query select(Field... fields) { + return new Query(fields); } public Query from(SqlTable fromTable) { @@ -62,11 +57,6 @@ public final class Query { return this; } - public Query groupBy(Field... groupBy) { - groupBies.addAll(asList(groupBy)); - return this; - } - public Query orderBy(Order... order) { orders.addAll(asList(order)); return this; @@ -96,11 +86,10 @@ public final class Query { visitJoinClause(sql); if(queryTemplate == null) { visitWhereClause(sql); - visitGroupByClause(sql); visitOrderByClause(sql); visitLimitClause(sql); } else { - if(groupBies.size() > 0 || orders.size() > 0) { + if(orders.size() > 0) { throw new IllegalStateException("Can't have extras AND query template"); //$NON-NLS-1$ } sql.append(queryTemplate); @@ -120,17 +109,6 @@ public final class Query { sql.deleteCharAt(sql.length() - 1).append(SPACE); } - private void visitGroupByClause(StringBuilder sql) { - if (groupBies.isEmpty()) { - return; - } - sql.append(GROUP_BY); - for (Field groupBy : groupBies) { - sql.append(SPACE).append(groupBy).append(COMMA); - } - sql.deleteCharAt(sql.length() - 1).append(SPACE); - } - private void visitWhereClause(StringBuilder sql) { if (criterions.isEmpty()) { return; @@ -156,9 +134,6 @@ public final class Query { private void visitSelectClause(StringBuilder sql) { sql.append(SELECT).append(SPACE); - if(distinct) { - sql.append(DISTINCT).append(SPACE); - } if (fields.isEmpty()) { sql.append(ALL).append(SPACE); return; diff --git a/app/src/main/java/com/todoroo/andlib/sql/QueryTemplate.java b/app/src/main/java/com/todoroo/andlib/sql/QueryTemplate.java index 8e6d08d86..bd31642d5 100644 --- a/app/src/main/java/com/todoroo/andlib/sql/QueryTemplate.java +++ b/app/src/main/java/com/todoroo/andlib/sql/QueryTemplate.java @@ -8,7 +8,6 @@ package com.todoroo.andlib.sql; import java.util.ArrayList; import static com.todoroo.andlib.sql.SqlConstants.COMMA; -import static com.todoroo.andlib.sql.SqlConstants.GROUP_BY; import static com.todoroo.andlib.sql.SqlConstants.LIMIT; import static com.todoroo.andlib.sql.SqlConstants.ORDER_BY; import static com.todoroo.andlib.sql.SqlConstants.SPACE; @@ -26,7 +25,6 @@ public final class QueryTemplate { private final ArrayList criterions = new ArrayList<>(); private final ArrayList joins = new ArrayList<>(); - private final ArrayList groupBies = new ArrayList<>(); private final ArrayList orders = new ArrayList<>(); private Integer limit = null; @@ -50,7 +48,6 @@ public final class QueryTemplate { StringBuilder sql = new StringBuilder(); visitJoinClause(sql); visitWhereClause(sql); - visitGroupByClause(sql); visitOrderByClause(sql); if(limit != null) { sql.append(LIMIT).append(SPACE).append(limit); @@ -69,17 +66,6 @@ public final class QueryTemplate { sql.deleteCharAt(sql.length() - 1).append(SPACE); } - private void visitGroupByClause(StringBuilder sql) { - if (groupBies.isEmpty()) { - return; - } - sql.append(GROUP_BY); - for (Field groupBy : groupBies) { - sql.append(SPACE).append(groupBy).append(COMMA); - } - sql.deleteCharAt(sql.length() - 1).append(SPACE); - } - private void visitWhereClause(StringBuilder sql) { if (criterions.isEmpty()) { return; diff --git a/app/src/main/java/com/todoroo/andlib/sql/SqlConstants.java b/app/src/main/java/com/todoroo/andlib/sql/SqlConstants.java index 5a46e772a..bd0ed7f6b 100644 --- a/app/src/main/java/com/todoroo/andlib/sql/SqlConstants.java +++ b/app/src/main/java/com/todoroo/andlib/sql/SqlConstants.java @@ -7,7 +7,6 @@ package com.todoroo.andlib.sql; public final class SqlConstants { public static final String SELECT = "SELECT"; - public static final String DISTINCT = "DISTINCT"; public static final String SPACE = " "; public static final String AS = "AS"; public static final String COMMA = ","; @@ -20,7 +19,6 @@ public final class SqlConstants { public static final String AND = "AND"; public static final String OR = "OR"; public static final String ORDER_BY = "ORDER BY"; - public static final String GROUP_BY = "GROUP BY"; public static final String WHERE = "WHERE"; public static final String NOT = "NOT"; public static final String LIMIT = "LIMIT"; diff --git a/app/src/main/java/com/todoroo/astrid/backup/TasksXmlExporter.java b/app/src/main/java/com/todoroo/astrid/backup/TasksXmlExporter.java index ae37cfa69..ceddc744c 100755 --- a/app/src/main/java/com/todoroo/astrid/backup/TasksXmlExporter.java +++ b/app/src/main/java/com/todoroo/astrid/backup/TasksXmlExporter.java @@ -124,7 +124,7 @@ public class TasksXmlExporter { try { String output = setupFile(backupDirectory, exportType); - int tasks = taskDao.count(Query.select(Task.ID)); + int tasks = taskDao.count(Query.select()); if(tasks > 0) { doTasksExport(output); @@ -185,7 +185,7 @@ public class TasksXmlExporter { } private void serializeTasks() throws IOException { - List tasks = taskDao.toList(Query.select(Task.PROPERTIES).orderBy(Order.asc(Task.ID))); + List tasks = taskDao.toList(Query.select().orderBy(Order.asc(Task.ID))); int length = tasks.size(); for(int i = 0; i < length; i++) { Task task = tasks.get(i); diff --git a/app/src/main/java/com/todoroo/astrid/backup/TasksXmlImporter.java b/app/src/main/java/com/todoroo/astrid/backup/TasksXmlImporter.java index ebb927782..5bb09445b 100755 --- a/app/src/main/java/com/todoroo/astrid/backup/TasksXmlImporter.java +++ b/app/src/main/java/com/todoroo/astrid/backup/TasksXmlImporter.java @@ -206,7 +206,7 @@ public class TasksXmlImporter { } // if the task's name and creation date match an existing task, skip - Query query = Query.select(Task.ID, Task.COMPLETION_DATE, Task.DELETION_DATE) + Query query = Query.select() .where(Criterion.and(Task.TITLE.eq(title), Task.CREATION_DATE.eq(created))); if (taskDao.count(query) > 0) { skipCount++; diff --git a/app/src/main/java/com/todoroo/astrid/core/OldTaskPreferences.java b/app/src/main/java/com/todoroo/astrid/core/OldTaskPreferences.java index 4d8c7e9ae..7cd4b667b 100644 --- a/app/src/main/java/com/todoroo/astrid/core/OldTaskPreferences.java +++ b/app/src/main/java/com/todoroo/astrid/core/OldTaskPreferences.java @@ -125,7 +125,7 @@ public class OldTaskPreferences extends InjectingPreferenceActivity { private int deleteCalendarEvents(Criterion criterion) { int deletedEventCount = 0; - List tasks = taskDao.toList(Query.select(Task.ID, Task.CALENDAR_URI).where(criterion)); + List tasks = taskDao.toList(Query.select().where(criterion)); for (Task task : tasks) { if (calendarEventProvider.deleteEvent(task)) { deletedEventCount++; diff --git a/app/src/main/java/com/todoroo/astrid/dao/TaskDao.java b/app/src/main/java/com/todoroo/astrid/dao/TaskDao.java index 242deebe6..76bb889d3 100644 --- a/app/src/main/java/com/todoroo/astrid/dao/TaskDao.java +++ b/app/src/main/java/com/todoroo/astrid/dao/TaskDao.java @@ -82,7 +82,7 @@ public abstract class TaskDao { public List query(Filter filter) { String query = PermaSql.replacePlaceholders(filter.getSqlQuery()); - return query(Query.select(Task.PROPERTIES).withQueryTemplate(query)).toList(); + return query(Query.select().withQueryTemplate(query)).toList(); } public List toList(Query query) { @@ -207,9 +207,8 @@ public abstract class TaskDao { } public TodorooCursor fetchFiltered(String queryTemplate, Property... properties) { - return query(queryTemplate == null - ? Query.selectDistinct(properties) - : Query.select(properties).withQueryTemplate(PermaSql.replacePlaceholders(queryTemplate))); + return query(Query.select(properties) + .withQueryTemplate(PermaSql.replacePlaceholders(queryTemplate))); } /** diff --git a/app/src/main/java/com/todoroo/astrid/provider/Astrid2TaskProvider.java b/app/src/main/java/com/todoroo/astrid/provider/Astrid2TaskProvider.java index 5a63f56b2..a9f4456d4 100644 --- a/app/src/main/java/com/todoroo/astrid/provider/Astrid2TaskProvider.java +++ b/app/src/main/java/com/todoroo/astrid/provider/Astrid2TaskProvider.java @@ -167,7 +167,7 @@ public class Astrid2TaskProvider extends InjectingContentProvider { private Cursor getTasks() { MatrixCursor ret = new MatrixCursor(TASK_FIELD_LIST); List importanceColors = checkBoxes.get().getPriorityColors(); - Query query = Query.select(Task.ID, Task.TITLE, Task.IMPORTANCE, Task.DUE_DATE) + Query query = Query.select() .where(Criterion.and(TaskCriteria.isActive(), TaskCriteria.isVisible())) .orderBy(SortHelper.defaultTaskOrder()).limit(MAX_NUMBER_OF_TASKS); for (Task task : taskDao.get().toList(query)) { diff --git a/app/src/main/java/com/todoroo/astrid/reminders/ReminderService.java b/app/src/main/java/com/todoroo/astrid/reminders/ReminderService.java index e2a71caf5..994724646 100644 --- a/app/src/main/java/com/todoroo/astrid/reminders/ReminderService.java +++ b/app/src/main/java/com/todoroo/astrid/reminders/ReminderService.java @@ -53,7 +53,7 @@ public final class ReminderService { } public void scheduleAllAlarms(TaskDao taskDao) { - Query query = Query.select(Task.PROPERTIES).where(Criterion.and( + Query query = Query.select().where(Criterion.and( TaskCriteria.isActive(), Criterion.or(Task.REMINDER_FLAGS.gt(0), Task.REMINDER_PERIOD.gt(0)))); for (Task task : taskDao.toList(query)) { diff --git a/app/src/main/java/com/todoroo/astrid/subtasks/AstridOrderedListFragmentHelper.java b/app/src/main/java/com/todoroo/astrid/subtasks/AstridOrderedListFragmentHelper.java index 7fcd2df3e..ec000931e 100644 --- a/app/src/main/java/com/todoroo/astrid/subtasks/AstridOrderedListFragmentHelper.java +++ b/app/src/main/java/com/todoroo/astrid/subtasks/AstridOrderedListFragmentHelper.java @@ -147,7 +147,7 @@ class AstridOrderedListFragmentHelper { if(chained.size() > 0) { // move recurring items to item parent - List tasks = taskDao.toList(Query.select(Task.UUID, Task.RECURRENCE).where( + List tasks = taskDao.toList(Query.select().where( Criterion.and(Task.UUID.in(chained.toArray(new String[chained.size()])), Task.RECURRENCE.isNotNull(), Functions.length(Task.RECURRENCE).gt(0)))); diff --git a/app/src/main/java/org/tasks/notifications/NotificationManager.java b/app/src/main/java/org/tasks/notifications/NotificationManager.java index a2efd6d75..698661953 100644 --- a/app/src/main/java/org/tasks/notifications/NotificationManager.java +++ b/app/src/main/java/org/tasks/notifications/NotificationManager.java @@ -231,7 +231,7 @@ public class NotificationManager { ArrayList taskIds = newArrayList(transform(notifications, n -> n.taskId)); QueryTemplate query = new QueryTemplate().where(Task.ID.in(taskIds)); Filter filter = new Filter(context.getString(R.string.notifications), query); - List tasks = taskDao.toList(Query.select(Task.PROPERTIES) + List tasks = taskDao.toList(Query.select() .withQueryTemplate(query.toString())); long when = notificationDao.latestTimestamp(); int maxPriority = 3;