Mercurial > hg > release > thermostat-0.13
changeset 1216:91e56d2ba874
Remove Storage.removePojo().
Reviewed-by: omajid
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-August/007909.html
PR1484
line wrap: on
line diff
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/core/Put.java Fri Aug 09 15:45:12 2013 -0400 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/Put.java Wed Aug 14 17:48:13 2013 +0200 @@ -44,10 +44,7 @@ /** * Sets the POJO that is to be put into the database. - * Notice that this is usually not necessary to call, because Storage.add() - * and Storage.replace() already take this argument. It is useful - * for pre-building and caching queries though. - * + * * @param the pojo to be put into the database */ void setPojo(Pojo pojo);
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/core/QueuedStorage.java Fri Aug 09 15:45:12 2013 -0400 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/QueuedStorage.java Wed Aug 14 17:48:13 2013 +0200 @@ -176,20 +176,6 @@ } @Override - public void removePojo(final Remove remove) { - - executor.execute(new Runnable() { - - @Override - public void run() { - delegate.removePojo(remove); - } - - }); - - } - - @Override public void purge(final String agentId) { executor.execute(new Runnable() {
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/core/Remove.java Fri Aug 09 15:45:12 2013 -0400 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/Remove.java Wed Aug 14 17:48:13 2013 +0200 @@ -44,9 +44,14 @@ */ public interface Remove { - Remove from(Category category); + void from(Category category); - Remove where(Expression where); + void where(Expression where); + + /** + * Applies this remove operation. + */ + void apply(); }
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/core/Storage.java Fri Aug 09 15:45:12 2013 -0400 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/Storage.java Wed Aug 14 17:48:13 2013 +0200 @@ -83,8 +83,6 @@ Add createAdd(Category<?> category); Replace createReplace(Category<?> category); - void removePojo(Remove remove); - /** * Drop all data related to the specified agent. */
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/internal/dao/AgentInfoDAOImpl.java Fri Aug 09 15:45:12 2013 -0400 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/internal/dao/AgentInfoDAOImpl.java Wed Aug 14 17:48:13 2013 +0200 @@ -174,8 +174,10 @@ @Override public void removeAgentInformation(AgentInformation agentInfo) { Expression expr = factory.equalTo(Key.AGENT_ID, agentInfo.getAgentId()); - Remove remove = storage.createRemove().from(CATEGORY).where(expr); - storage.removePojo(remove); + Remove remove = storage.createRemove(); + remove.from(CATEGORY); + remove.where(expr); + remove.apply(); } @Override
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/internal/dao/BackendInfoDAOImpl.java Fri Aug 09 15:45:12 2013 -0400 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/internal/dao/BackendInfoDAOImpl.java Wed Aug 14 17:48:13 2013 +0200 @@ -116,8 +116,10 @@ @Override public void removeBackendInformation(BackendInformation info) { Expression expr = factory.equalTo(BACKEND_NAME, info.getName()); - Remove remove = storage.createRemove().from(CATEGORY).where(expr); - storage.removePojo(remove); + Remove remove = storage.createRemove(); + remove.from(CATEGORY); + remove.where(expr); + remove.apply(); } }
--- a/storage/core/src/test/java/com/redhat/thermostat/storage/core/QueuedStorageTest.java Fri Aug 09 15:45:12 2013 -0400 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/core/QueuedStorageTest.java Wed Aug 14 17:48:13 2013 +0200 @@ -280,25 +280,6 @@ } @Test - public void testRemovePojo() { - - Remove remove = queuedStorage.createRemove(); - verify(delegateStorage).createRemove(); - verifyNoMoreInteractions(delegateStorage); - - queuedStorage.removePojo(remove); - - Runnable r = executor.getTask(); - assertNotNull(r); - verifyZeroInteractions(delegateStorage); - r.run(); - verify(delegateStorage, times(1)).removePojo(remove); - verifyNoMoreInteractions(delegateStorage); - - assertNull(fileExecutor.getTask()); - } - - @Test public void testPurge() { queuedStorage.purge("fluff"); @@ -473,12 +454,6 @@ } @Override - public void removePojo(Remove remove) { - // not implemented - throw new AssertionError(); - } - - @Override public void purge(String agentId) { // not implemented throw new AssertionError();
--- a/storage/core/src/test/java/com/redhat/thermostat/storage/internal/dao/AgentInfoDAOTest.java Fri Aug 09 15:45:12 2013 -0400 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/internal/dao/AgentInfoDAOTest.java Wed Aug 14 17:48:13 2013 +0200 @@ -272,15 +272,15 @@ @Test public void verifyRemoveAgentInformation() { - Remove mockRemove = QueryTestHelper.createMockRemove(); + Remove mockRemove = mock(Remove.class); Storage storage = mock(Storage.class); when(storage.createRemove()).thenReturn(mockRemove); AgentInfoDAO dao = new AgentInfoDAOImpl(storage); dao.removeAgentInformation(agentInfo1); - verify(storage).removePojo(mockRemove); verify(mockRemove).from(AgentInfoDAO.CATEGORY); + verify(mockRemove).apply(); Expression expr = factory.equalTo(Key.AGENT_ID, "1234"); verify(mockRemove).where(eq(expr)); }
--- a/storage/core/src/test/java/com/redhat/thermostat/storage/internal/dao/BackendInfoDAOTest.java Fri Aug 09 15:45:12 2013 -0400 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/internal/dao/BackendInfoDAOTest.java Wed Aug 14 17:48:13 2013 +0200 @@ -176,18 +176,18 @@ @Test public void verifyRemoveBackendInformation() { - Remove remove = QueryTestHelper.createMockRemove(); + Remove remove = mock(Remove.class); Storage storage = mock(Storage.class); when(storage.createRemove()).thenReturn(remove); BackendInfoDAO dao = new BackendInfoDAOImpl(storage); dao.removeBackendInformation(backendInfo1); - verify(storage).removePojo(remove); InOrder inOrder = inOrder(remove); inOrder.verify(remove).from(BackendInfoDAO.CATEGORY); Expression expr = factory.equalTo(BackendInfoDAO.BACKEND_NAME, "backend-name"); inOrder.verify(remove).where(eq(expr)); + inOrder.verify(remove).apply(); } }
--- a/storage/core/src/test/java/com/redhat/thermostat/storage/internal/dao/QueryTestHelper.java Fri Aug 09 15:45:12 2013 -0400 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,58 +0,0 @@ -/* - * Copyright 2012, 2013 Red Hat, Inc. - * - * This file is part of Thermostat. - * - * Thermostat is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published - * by the Free Software Foundation; either version 2, or (at your - * option) any later version. - * - * Thermostat is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with Thermostat; see the file COPYING. If not see - * <http://www.gnu.org/licenses/>. - * - * Linking this code with other modules is making a combined work - * based on this code. Thus, the terms and conditions of the GNU - * General Public License cover the whole combination. - * - * As a special exception, the copyright holders of this code give - * you permission to link this code with independent modules to - * produce an executable, regardless of the license terms of these - * independent modules, and to copy and distribute the resulting - * executable under terms of your choice, provided that you also - * meet, for each linked independent module, the terms and conditions - * of the license of that module. An independent module is a module - * which is not derived from or based on this code. If you modify - * this code, you may extend this exception to your version of the - * library, but you are not obligated to do so. If you do not wish - * to do so, delete this exception statement from your version. - */ - - -package com.redhat.thermostat.storage.internal.dao; - -import static org.mockito.Matchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -import com.redhat.thermostat.storage.core.Category; -import com.redhat.thermostat.storage.core.Remove; -import com.redhat.thermostat.storage.query.Expression; - -public class QueryTestHelper { - - public static Remove createMockRemove() { - Remove mockRemove = mock(Remove.class); - when(mockRemove.from(any(Category.class))).thenReturn(mockRemove); - when(mockRemove.where(any(Expression.class))).thenReturn(mockRemove); - return mockRemove; - } - -} -
--- a/storage/mongo/src/main/java/com/redhat/thermostat/storage/mongodb/internal/MongoRemove.java Fri Aug 09 15:45:12 2013 -0400 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,83 +0,0 @@ -/* - * Copyright 2012, 2013 Red Hat, Inc. - * - * This file is part of Thermostat. - * - * Thermostat is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published - * by the Free Software Foundation; either version 2, or (at your - * option) any later version. - * - * Thermostat is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with Thermostat; see the file COPYING. If not see - * <http://www.gnu.org/licenses/>. - * - * Linking this code with other modules is making a combined work - * based on this code. Thus, the terms and conditions of the GNU - * General Public License cover the whole combination. - * - * As a special exception, the copyright holders of this code give - * you permission to link this code with independent modules to - * produce an executable, regardless of the license terms of these - * independent modules, and to copy and distribute the resulting - * executable under terms of your choice, provided that you also - * meet, for each linked independent module, the terms and conditions - * of the license of that module. An independent module is a module - * which is not derived from or based on this code. If you modify - * this code, you may extend this exception to your version of the - * library, but you are not obligated to do so. If you do not wish - * to do so, delete this exception statement from your version. - */ - - -package com.redhat.thermostat.storage.mongodb.internal; - -import com.mongodb.DBObject; -import com.redhat.thermostat.storage.core.Category; -import com.redhat.thermostat.storage.core.Remove; -import com.redhat.thermostat.storage.query.Expression; - -class MongoRemove implements Remove { - - private Category category; - private DBObject query; - private MongoExpressionParser parser; - - public MongoRemove() { - this(new MongoExpressionParser()); - } - - MongoRemove(MongoExpressionParser parser) { - this.parser = parser; - } - - @Override - public Remove from(Category category) { - if (query != null) { - throw new IllegalStateException(); - } - this.category = category; - return this; - } - - Category getCategory() { - return category; - } - - @Override - public Remove where(Expression expr) { - query = parser.parse(expr); - return this; - } - - DBObject getQuery() { - return query; - } - -} -
--- a/storage/mongo/src/main/java/com/redhat/thermostat/storage/mongodb/internal/MongoStorage.java Fri Aug 09 15:45:12 2013 -0400 +++ b/storage/mongo/src/main/java/com/redhat/thermostat/storage/mongodb/internal/MongoStorage.java Wed Aug 14 17:48:13 2013 +0200 @@ -72,6 +72,7 @@ import com.redhat.thermostat.storage.core.StatementDescriptor; import com.redhat.thermostat.storage.core.Update; import com.redhat.thermostat.storage.model.Pojo; +import com.redhat.thermostat.storage.query.Expression; /** * Implementation of the Storage interface that uses MongoDB to store the instrumentation data. @@ -97,6 +98,41 @@ } } + + private class MongoRemove implements Remove { + + @SuppressWarnings("rawtypes") + private Category category; + private DBObject query; + private MongoExpressionParser parser; + + private MongoRemove() { + this(new MongoExpressionParser()); + } + + private MongoRemove(MongoExpressionParser parser) { + this.parser = parser; + } + + @Override + public void from(@SuppressWarnings("rawtypes") Category category) { + if (query != null) { + throw new IllegalStateException(); + } + this.category = category; + } + + @Override + public void where(Expression expr) { + query = parser.parse(expr); + } + + @Override + public void apply() { + removePojo(category, query); + } + + } private MongoConnection conn; private DB db = null; @@ -191,14 +227,8 @@ coll.update(query, values); } - @Override - public void removePojo(Remove remove) { - assert (remove instanceof MongoRemove); - MongoRemove mongoRemove = (MongoRemove) remove; - DBObject query = mongoRemove.getQuery(); - Category<?> category = mongoRemove.getCategory(); + private void removePojo(Category<?> category, DBObject query) { DBCollection coll = getCachedCollection(category); - coll.remove(query); }
--- a/web/client/src/main/java/com/redhat/thermostat/web/client/internal/WebStorage.java Fri Aug 09 15:45:12 2013 -0400 +++ b/web/client/src/main/java/com/redhat/thermostat/web/client/internal/WebStorage.java Wed Aug 14 17:48:13 2013 +0200 @@ -327,6 +327,24 @@ } } + private class WebRemoveImpl extends WebRemove { + + // required for serialization + private WebRemoveImpl() { + this(null); + } + + private WebRemoveImpl(Map<Category<?>, Integer> categoryIds) { + super(categoryIds); + } + + @Override + public void apply() { + removePojo(this); + } + + } + private class WebPreparedStatementImpl<T extends Pojo> extends WebPreparedStatement<T> { // The type of the query result objects we'd get back upon @@ -513,7 +531,7 @@ @Override public Remove createRemove() { - return new WebRemove(categoryIds); + return new WebRemoveImpl(categoryIds); } @Override @@ -660,8 +678,7 @@ } } - @Override - public void removePojo(Remove remove) throws StorageException { + private void removePojo(Remove remove) throws StorageException { NameValuePair removeParam = new BasicNameValuePair("remove", gson.toJson(remove)); List<NameValuePair> formparams = Arrays.asList(removeParam);
--- a/web/client/src/test/java/com/redhat/thermostat/web/client/internal/WebStorageTest.java Fri Aug 09 15:45:12 2013 -0400 +++ b/web/client/src/test/java/com/redhat/thermostat/web/client/internal/WebStorageTest.java Wed Aug 14 17:48:13 2013 +0200 @@ -432,22 +432,22 @@ public void testCreateRemove() { WebRemove remove = (WebRemove) storage.createRemove(); assertNotNull(remove); - remove = remove.from(category); + remove.from(category); assertEquals(42, remove.getCategoryId()); - assertNotNull(remove); Expression expr = factory.equalTo(key1, "test"); - remove = remove.where(expr); - assertNotNull(remove); + remove.where(expr); assertEquals(expr, remove.getWhereExpression()); } @Test public void testRemovePojo() throws UnsupportedEncodingException, IOException { Expression expr = factory.equalTo(key1, "test"); - Remove remove = storage.createRemove().from(category).where(expr); + Remove remove = storage.createRemove(); + remove.from(category); + remove.where(expr); prepareServer(); - storage.removePojo(remove); + remove.apply(); Gson gson = new GsonBuilder() .registerTypeHierarchyAdapter(Expression.class,
--- a/web/common/src/main/java/com/redhat/thermostat/web/common/WebRemove.java Fri Aug 09 15:45:12 2013 -0400 +++ b/web/common/src/main/java/com/redhat/thermostat/web/common/WebRemove.java Wed Aug 14 17:48:13 2013 +0200 @@ -48,25 +48,18 @@ private int categoryId; private Expression whereExpression; - // NOTE: This is needed for de-serialization! - public WebRemove() { - this(null); - } - public WebRemove(Map<Category<?>, Integer> categoryIds) { this.categoryIds = categoryIds; } @Override - public WebRemove from(Category category) { + public void from(Category category) { categoryId = categoryIds.get(category); - return this; } @Override - public WebRemove where(Expression expr) { + public void where(Expression expr) { whereExpression = expr; - return this; } public int getCategoryId() { @@ -76,6 +69,13 @@ public Expression getWhereExpression() { return whereExpression; } + + @Override + public void apply() { + // This should never be called. Overridden by the actual + // implementation. + throw new IllegalStateException(); + } }
--- a/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java Fri Aug 09 15:45:12 2013 -0400 +++ b/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java Wed Aug 14 17:48:13 2013 +0200 @@ -521,12 +521,12 @@ String removeParam = req.getParameter("remove"); WebRemove remove = gson.fromJson(removeParam, WebRemove.class); Remove targetRemove = storage.createRemove(); - targetRemove = targetRemove.from(getCategoryFromId(remove.getCategoryId())); + targetRemove.from(getCategoryFromId(remove.getCategoryId())); Expression expr = remove.getWhereExpression(); if (expr != null) { targetRemove.where(expr); } - storage.removePojo(targetRemove); + targetRemove.apply(); resp.setStatus(HttpServletResponse.SC_OK); }
--- a/web/server/src/test/java/com/redhat/thermostat/web/server/QueryTestHelper.java Fri Aug 09 15:45:12 2013 -0400 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,58 +0,0 @@ -/* - * Copyright 2012, 2013 Red Hat, Inc. - * - * This file is part of Thermostat. - * - * Thermostat is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published - * by the Free Software Foundation; either version 2, or (at your - * option) any later version. - * - * Thermostat is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with Thermostat; see the file COPYING. If not see - * <http://www.gnu.org/licenses/>. - * - * Linking this code with other modules is making a combined work - * based on this code. Thus, the terms and conditions of the GNU - * General Public License cover the whole combination. - * - * As a special exception, the copyright holders of this code give - * you permission to link this code with independent modules to - * produce an executable, regardless of the license terms of these - * independent modules, and to copy and distribute the resulting - * executable under terms of your choice, provided that you also - * meet, for each linked independent module, the terms and conditions - * of the license of that module. An independent module is a module - * which is not derived from or based on this code. If you modify - * this code, you may extend this exception to your version of the - * library, but you are not obligated to do so. If you do not wish - * to do so, delete this exception statement from your version. - */ - - -package com.redhat.thermostat.web.server; - -import static org.mockito.Matchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -import com.redhat.thermostat.storage.core.Category; -import com.redhat.thermostat.storage.core.Remove; -import com.redhat.thermostat.storage.query.Expression; - -public class QueryTestHelper { - - public static Remove createMockRemove() { - Remove mockRemove = mock(Remove.class); - when(mockRemove.from(any(Category.class))).thenReturn(mockRemove); - when(mockRemove.where(any(Expression.class))).thenReturn(mockRemove); - return mockRemove; - } - -} -
--- a/web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndpointTest.java Fri Aug 09 15:45:12 2013 -0400 +++ b/web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndpointTest.java Wed Aug 14 17:48:13 2013 +0200 @@ -855,8 +855,6 @@ Remove mockRemove = mock(Remove.class); - when(mockRemove.from(any(Category.class))).thenReturn(mockRemove); - when(mockRemove.where(any(Expression.class))).thenReturn(mockRemove); when(mockStorage.createRemove()).thenReturn(mockRemove); @@ -871,7 +869,9 @@ Map<Category<?>,Integer> categoryIds = new HashMap<>(); categoryIds.put(category, categoryId); Expression expr = factory.equalTo(key1, "test"); - WebRemove remove = new WebRemove(categoryIds).from(category).where(expr); + WebRemove remove = new WebRemove(categoryIds); + remove.from(category); + remove.where(expr); Gson gson = new GsonBuilder() .registerTypeHierarchyAdapter(Expression.class, new ExpressionSerializer()) @@ -887,7 +887,7 @@ verify(mockStorage).createRemove(); verify(mockRemove).from(category); verify(mockRemove).where(eq(expr)); - verify(mockStorage).removePojo(mockRemove); + verify(mockRemove).apply(); } @Test