Mercurial > hg > thermostat-ng > agent
changeset 1728:5cea227b1375
Merge BatchCursor into Cursor.
Reviewed-by: neugens
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2015-April/013500.html
PR2100
line wrap: on
line diff
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/BasicBatchCursor.java Wed Apr 01 19:23:47 2015 +0200 @@ -0,0 +1,57 @@ +/* + * Copyright 2012-2015 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.core; + +import com.redhat.thermostat.storage.model.Pojo; + +public abstract class BasicBatchCursor<T extends Pojo> implements Cursor<T> { + + private Integer batchSize; + + @Override + public void setBatchSize(int n) throws IllegalArgumentException { + if (n <= 0) { + throw new IllegalArgumentException("Batch size must be > 0"); + } + this.batchSize = n; + } + + @Override + public int getBatchSize() { + return this.batchSize == null ? Cursor.DEFAULT_BATCH_SIZE : this.batchSize; + } +}
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/core/Cursor.java Wed Apr 01 20:52:28 2015 +0200 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/Cursor.java Wed Apr 01 19:23:47 2015 +0200 @@ -47,6 +47,35 @@ */ public interface Cursor<T extends Pojo> { + public static final int DEFAULT_BATCH_SIZE = 100; + + /** + * <p> + * Sets the configured batch size when retrieving more elements from the + * database. That is, no more elements will be loaded into memory than the + * configured batch size. Note that the new batch size will only take effect + * once the current batch is exhausted. + * </p> + * <p> + * The default batch size is 100. + * </p> + * + * @param n + * The number of results to fetch from storage in a single batch. + * @return A cursor with the configured batch size. + * @throws IllegalArgumentException + * If {@code n} is < 1 + */ + void setBatchSize(int n) throws IllegalArgumentException; + + /** + * + * @return The configured batch size set via {@link setBatchSize} or + * {@link BatchCursor#DEFAULT_BATCH_SIZE} if it was never set + * explicitly. + */ + int getBatchSize(); + /** * @return {@code true} if there are more elements, {@code false} otherwise. *
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/core/experimental/BasicBatchCursor.java Wed Apr 01 20:52:28 2015 +0200 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,57 +0,0 @@ -/* - * Copyright 2012-2015 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.core.experimental; - -import com.redhat.thermostat.storage.model.Pojo; - -public abstract class BasicBatchCursor<T extends Pojo> implements BatchCursor<T> { - - private Integer batchSize; - - @Override - public void setBatchSize(int n) throws IllegalArgumentException { - if (n <= 0) { - throw new IllegalArgumentException("Batch size must be > 0"); - } - this.batchSize = n; - } - - @Override - public int getBatchSize() { - return this.batchSize == null ? BatchCursor.DEFAULT_BATCH_SIZE : this.batchSize; - } -}
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/core/experimental/BatchCursor.java Wed Apr 01 20:52:28 2015 +0200 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,73 +0,0 @@ -/* - * Copyright 2012-2015 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.core.experimental; - -import com.redhat.thermostat.storage.core.Cursor; -import com.redhat.thermostat.storage.model.Pojo; - -public interface BatchCursor<T extends Pojo> extends Cursor<T> { - - public static final int DEFAULT_BATCH_SIZE = 100; - - /** - * <p> - * Sets the configured batch size when retrieving more elements from the - * database. That is, no more elements will be loaded into memory than the - * configured batch size. Note that the new batch size will only take effect - * once the current batch is exhausted. - * </p> - * <p> - * The default batch size is 100. - * </p> - * - * @param n - * The number of results to fetch from storage in a single batch. - * @return A cursor with the configured batch size. - * @throws IllegalArgumentException - * If {@code n} is < 1 - */ - void setBatchSize(int n) throws IllegalArgumentException; - - /** - * - * @return The configured batch size set via {@link setBatchSize} or - * {@link BatchCursor#DEFAULT_BATCH_SIZE} if it was never set - * explicitly. - */ - int getBatchSize(); - -}
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/model/AggregateCount.java Wed Apr 01 20:52:28 2015 +0200 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/model/AggregateCount.java Wed Apr 01 19:23:47 2015 +0200 @@ -41,7 +41,6 @@ import com.redhat.thermostat.storage.core.Cursor; import com.redhat.thermostat.storage.core.Entity; import com.redhat.thermostat.storage.core.Persist; -import com.redhat.thermostat.storage.core.experimental.BatchCursor; /** * Model class for aggregate counts. @@ -78,7 +77,7 @@ @SuppressWarnings("unchecked") public <T extends Pojo> Cursor<T> getCursor() { - return (BatchCursor<T>) new AggregateCursor<>(this); + return (Cursor<T>)new AggregateCursor<>(this); } }
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/model/AggregateCursor.java Wed Apr 01 20:52:28 2015 +0200 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/model/AggregateCursor.java Wed Apr 01 19:23:47 2015 +0200 @@ -39,7 +39,8 @@ import java.util.logging.Logger; import com.redhat.thermostat.common.utils.LoggingUtils; -import com.redhat.thermostat.storage.core.experimental.BasicBatchCursor; +import com.redhat.thermostat.storage.core.BasicBatchCursor; + import java.util.NoSuchElementException; /**
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/core/BasicBatchCursorTest.java Wed Apr 01 19:23:47 2015 +0200 @@ -0,0 +1,99 @@ +/* + * Copyright 2012-2015 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.core; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.fail; + +import org.junit.Test; + +import com.redhat.thermostat.storage.core.BasicBatchCursor; +import com.redhat.thermostat.storage.model.Pojo; + +public class BasicBatchCursorTest { + + @Test + public void testSetBatchSize() { + BasicBatchCursor<TestPojo> cursor = new BasicBatchCursorImpl<>(); + try { + cursor.setBatchSize(-1); + fail("expected IAE for batch size of -1"); + } catch (IllegalArgumentException e) { + // pass + assertEquals("Batch size must be > 0", e.getMessage()); + } + try { + cursor.setBatchSize(0); + fail("expected IAE for batch size of 0"); + } catch (IllegalArgumentException e) { + // pass + assertEquals("Batch size must be > 0", e.getMessage()); + } + cursor.setBatchSize(Cursor.DEFAULT_BATCH_SIZE); + assertEquals(Cursor.DEFAULT_BATCH_SIZE, cursor.getBatchSize()); + } + + @Test + public void testGetBatchSize() { + BasicBatchCursor<TestPojo> cursor = new BasicBatchCursorImpl<>(); + assertNotNull("should always return default if never set", cursor.getBatchSize()); + assertEquals(Cursor.DEFAULT_BATCH_SIZE, cursor.getBatchSize()); + cursor.setBatchSize(3); + assertEquals(3, cursor.getBatchSize()); + } + + private static class BasicBatchCursorImpl<T extends Pojo> extends BasicBatchCursor<T> { + + @Override + public boolean hasNext() { + // not implemented + return false; + } + + @Override + public T next() { + // not implemented + return null; + } + + } + + private static class TestPojo implements Pojo { + // nothing + } +}
--- a/storage/core/src/test/java/com/redhat/thermostat/storage/core/experimental/BasicBatchCursorTest.java Wed Apr 01 20:52:28 2015 +0200 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,98 +0,0 @@ -/* - * Copyright 2012-2015 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.core.experimental; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.fail; - -import org.junit.Test; - -import com.redhat.thermostat.storage.model.Pojo; - -public class BasicBatchCursorTest { - - @Test - public void testSetBatchSize() { - BasicBatchCursor<TestPojo> cursor = new BasicBatchCursorImpl<>(); - try { - cursor.setBatchSize(-1); - fail("expected IAE for batch size of -1"); - } catch (IllegalArgumentException e) { - // pass - assertEquals("Batch size must be > 0", e.getMessage()); - } - try { - cursor.setBatchSize(0); - fail("expected IAE for batch size of 0"); - } catch (IllegalArgumentException e) { - // pass - assertEquals("Batch size must be > 0", e.getMessage()); - } - cursor.setBatchSize(BatchCursor.DEFAULT_BATCH_SIZE); - assertEquals(BatchCursor.DEFAULT_BATCH_SIZE, cursor.getBatchSize()); - } - - @Test - public void testGetBatchSize() { - BasicBatchCursor<TestPojo> cursor = new BasicBatchCursorImpl<>(); - assertNotNull("should always return default if never set", cursor.getBatchSize()); - assertEquals(BatchCursor.DEFAULT_BATCH_SIZE, cursor.getBatchSize()); - cursor.setBatchSize(3); - assertEquals(3, cursor.getBatchSize()); - } - - private static class BasicBatchCursorImpl<T extends Pojo> extends BasicBatchCursor<T> { - - @Override - public boolean hasNext() { - // not implemented - return false; - } - - @Override - public T next() { - // not implemented - return null; - } - - } - - private static class TestPojo implements Pojo { - // nothing - } -}
--- a/storage/core/src/test/java/com/redhat/thermostat/storage/model/AggregateCursorTest.java Wed Apr 01 20:52:28 2015 +0200 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/model/AggregateCursorTest.java Wed Apr 01 19:23:47 2015 +0200 @@ -40,13 +40,13 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import java.util.NoSuchElementException; import org.junit.Test; import com.redhat.thermostat.storage.core.Cursor; -import com.redhat.thermostat.storage.core.experimental.BatchCursor; -import java.util.NoSuchElementException; -import static org.junit.Assert.fail; public class AggregateCursorTest { @@ -74,9 +74,8 @@ public void testCursorBatchSize() { AggregateTest t = new AggregateTest(); Cursor<AggregateTest> cursor = t.getCursor(); - BatchCursor<AggregateTest> advCursor = (BatchCursor<AggregateTest>)cursor; - advCursor.setBatchSize(500); - assertEquals(500, advCursor.getBatchSize()); + cursor.setBatchSize(500); + assertEquals(500, cursor.getBatchSize()); } private static class AggregateTest implements AggregateResult {
--- a/storage/mongo/src/main/java/com/redhat/thermostat/storage/mongodb/internal/MongoCursor.java Wed Apr 01 20:52:28 2015 +0200 +++ b/storage/mongo/src/main/java/com/redhat/thermostat/storage/mongodb/internal/MongoCursor.java Wed Apr 01 19:23:47 2015 +0200 @@ -42,9 +42,10 @@ import com.mongodb.DBObject; import com.mongodb.MongoException; import com.redhat.thermostat.common.utils.LoggingUtils; +import com.redhat.thermostat.storage.core.BasicBatchCursor; import com.redhat.thermostat.storage.core.StorageException; -import com.redhat.thermostat.storage.core.experimental.BasicBatchCursor; import com.redhat.thermostat.storage.model.Pojo; + import java.util.NoSuchElementException; class MongoCursor<T extends Pojo> extends BasicBatchCursor<T> {
--- a/storage/mongo/src/main/java/com/redhat/thermostat/storage/mongodb/internal/MongoStorage.java Wed Apr 01 20:52:28 2015 +0200 +++ b/storage/mongo/src/main/java/com/redhat/thermostat/storage/mongodb/internal/MongoStorage.java Wed Apr 01 19:23:47 2015 +0200 @@ -81,7 +81,6 @@ import com.redhat.thermostat.storage.core.StorageCredentials; import com.redhat.thermostat.storage.core.StorageException; import com.redhat.thermostat.storage.core.Update; -import com.redhat.thermostat.storage.core.experimental.BatchCursor; import com.redhat.thermostat.storage.model.AggregateCount; import com.redhat.thermostat.storage.model.AggregateResult; import com.redhat.thermostat.storage.model.DistinctResult; @@ -595,8 +594,8 @@ dbCursor = coll.find(); } dbCursor = applySortAndLimit(mongoQuery, dbCursor); - BatchCursor<T> mongoCursor = new MongoCursor<T>(dbCursor, resultClass); - mongoCursor.setBatchSize(BatchCursor.DEFAULT_BATCH_SIZE); + Cursor<T> mongoCursor = new MongoCursor<T>(dbCursor, resultClass); + mongoCursor.setBatchSize(Cursor.DEFAULT_BATCH_SIZE); return mongoCursor; } catch (MongoException me) { throw new StorageException(me);
--- a/storage/mongo/src/test/java/com/redhat/thermostat/storage/mongodb/internal/MongoCursorTest.java Wed Apr 01 20:52:28 2015 +0200 +++ b/storage/mongo/src/test/java/com/redhat/thermostat/storage/mongodb/internal/MongoCursorTest.java Wed Apr 01 19:23:47 2015 +0200 @@ -39,12 +39,15 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyInt; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.util.NoSuchElementException; + import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -53,12 +56,10 @@ import com.mongodb.BasicDBObject; import com.mongodb.DBCursor; import com.mongodb.DBObject; +import com.redhat.thermostat.storage.core.Cursor; import com.redhat.thermostat.storage.core.Entity; import com.redhat.thermostat.storage.core.Persist; -import com.redhat.thermostat.storage.core.experimental.BatchCursor; import com.redhat.thermostat.storage.model.BasePojo; -import java.util.NoSuchElementException; -import static org.junit.Assert.fail; public class MongoCursorTest { @@ -108,7 +109,7 @@ } private DBCursor dbCursor; - private BatchCursor<TestClass> cursor; + private Cursor<TestClass> cursor; @Before public void setUp() { @@ -121,7 +122,7 @@ value2.put("key4", "test4"); dbCursor = mock(DBCursor.class); - when(dbCursor.batchSize(BatchCursor.DEFAULT_BATCH_SIZE)).thenReturn(dbCursor); + when(dbCursor.batchSize(Cursor.DEFAULT_BATCH_SIZE)).thenReturn(dbCursor); when(dbCursor.hasNext()).thenReturn(true).thenReturn(true).thenReturn(false); when(dbCursor.next()).thenReturn(value1).thenReturn(value2).thenReturn(null); when(dbCursor.sort(any(DBObject.class))).thenReturn(dbCursor); @@ -159,7 +160,7 @@ @Test public void testBatchSize() { DBCursor mongoCursor = mock(DBCursor.class); - BatchCursor<TestClass> mC = new MongoCursor<>(mongoCursor, TestClass.class); + Cursor<TestClass> mC = new MongoCursor<>(mongoCursor, TestClass.class); try { mC.setBatchSize(-1); fail("expected IAE for batch size of -1");
--- a/web/client/src/main/java/com/redhat/thermostat/web/client/internal/WebCursor.java Wed Apr 01 20:52:28 2015 +0200 +++ b/web/client/src/main/java/com/redhat/thermostat/web/client/internal/WebCursor.java Wed Apr 01 19:23:47 2015 +0200 @@ -42,12 +42,13 @@ import java.util.logging.Logger; import com.redhat.thermostat.common.utils.LoggingUtils; +import com.redhat.thermostat.storage.core.BasicBatchCursor; import com.redhat.thermostat.storage.core.StorageException; -import com.redhat.thermostat.storage.core.experimental.BasicBatchCursor; import com.redhat.thermostat.storage.model.Pojo; import com.redhat.thermostat.web.common.PreparedStatementResponseCode; import com.redhat.thermostat.web.common.WebPreparedStatement; import com.redhat.thermostat.web.common.WebQueryResponse; + import java.util.NoSuchElementException; class WebCursor<T extends Pojo> extends BasicBatchCursor<T> {
--- a/web/client/src/test/java/com/redhat/thermostat/web/client/internal/WebCursorTest.java Wed Apr 01 20:52:28 2015 +0200 +++ b/web/client/src/test/java/com/redhat/thermostat/web/client/internal/WebCursorTest.java Wed Apr 01 19:23:47 2015 +0200 @@ -40,23 +40,23 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.lang.reflect.Type; +import java.util.NoSuchElementException; import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; +import com.redhat.thermostat.storage.core.Cursor; import com.redhat.thermostat.storage.core.StorageException; -import com.redhat.thermostat.storage.core.experimental.BatchCursor; import com.redhat.thermostat.web.common.PreparedStatementResponseCode; import com.redhat.thermostat.web.common.WebPreparedStatement; import com.redhat.thermostat.web.common.WebQueryResponse; -import java.util.NoSuchElementException; -import static org.junit.Assert.fail; public class WebCursorTest { @@ -117,7 +117,7 @@ assertNull(second.getResultList()); // be sure to return a bad result should storage.getMore() be called // more than once - when(storage.getMore(cursorId, fakeType, BatchCursor.DEFAULT_BATCH_SIZE, stmt)).thenReturn(response).thenReturn(second); + when(storage.getMore(cursorId, fakeType, Cursor.DEFAULT_BATCH_SIZE, stmt)).thenReturn(response).thenReturn(second); TestObj actual = cursor.next(); assertEquals("next-test", actual.getProperty1()); @@ -143,7 +143,7 @@ response.setResponseCode(PreparedStatementResponseCode.GET_MORE_NULL_CURSOR); response.setCursorId(cursorId); response.setHasMoreBatches(false); - when(storage.getMore(cursorId, fakeType, BatchCursor.DEFAULT_BATCH_SIZE, stmt)).thenReturn(response); + when(storage.getMore(cursorId, fakeType, Cursor.DEFAULT_BATCH_SIZE, stmt)).thenReturn(response); try { cursor.next(); fail("Expected StorageException to be thrown"); @@ -168,7 +168,7 @@ response.setResponseCode(PreparedStatementResponseCode.QUERY_FAILURE); response.setCursorId(cursorId); response.setHasMoreBatches(false); - when(storage.getMore(cursorId, fakeType, BatchCursor.DEFAULT_BATCH_SIZE, stmt)).thenReturn(response); + when(storage.getMore(cursorId, fakeType, Cursor.DEFAULT_BATCH_SIZE, stmt)).thenReturn(response); try { cursor.next(); fail("Expected StorageException to be thrown");
--- a/web/client/src/test/java/com/redhat/thermostat/web/client/internal/WebStorageTest.java Wed Apr 01 20:52:28 2015 +0200 +++ b/web/client/src/test/java/com/redhat/thermostat/web/client/internal/WebStorageTest.java Wed Apr 01 19:23:47 2015 +0200 @@ -43,11 +43,12 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.times; import static org.mockito.Mockito.when; import java.io.BufferedReader; @@ -66,6 +67,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.NoSuchElementException; import java.util.UUID; import java.util.concurrent.CountDownLatch; @@ -110,7 +112,6 @@ import com.redhat.thermostat.storage.core.StatementExecutionException; import com.redhat.thermostat.storage.core.StorageCredentials; import com.redhat.thermostat.storage.core.StorageException; -import com.redhat.thermostat.storage.core.experimental.BatchCursor; import com.redhat.thermostat.storage.model.AggregateResult; import com.redhat.thermostat.storage.model.Pojo; import com.redhat.thermostat.test.FreePortFinder; @@ -126,8 +127,6 @@ import com.redhat.thermostat.web.common.typeadapters.WebPreparedStatementResponseTypeAdapterFactory; import com.redhat.thermostat.web.common.typeadapters.WebPreparedStatementTypeAdapterFactory; import com.redhat.thermostat.web.common.typeadapters.WebQueryResponseTypeAdapterFactory; -import java.util.NoSuchElementException; -import static org.junit.Assert.fail; public class WebStorageTest { @@ -554,16 +553,14 @@ assertEquals("cursor-id", cursorIdArray[0]); assertEquals("444", cursorIdArray[1]); assertEquals("batch-size", batchSizeArray[0]); - assertEquals(Integer.toString(BatchCursor.DEFAULT_BATCH_SIZE), batchSizeArray[1]); + assertEquals(Integer.toString(Cursor.DEFAULT_BATCH_SIZE), batchSizeArray[1]); assertEquals("get-more-result", returnedGetMore.getProperty1()); // Do it again, this time with a non-default batch size: 5 - assertTrue(results instanceof BatchCursor); - BatchCursor<TestObj> advCursor = (BatchCursor<TestObj>)results; - advCursor.setBatchSize(5); + results.setBatchSize(5); WebQueryResponse<TestObj> getMoreResults2 = new WebQueryResponse<>(); getMoreResults2.setResponseCode(PreparedStatementResponseCode.QUERY_SUCCESS); @@ -571,7 +568,7 @@ getMoreResults2.setHasMoreBatches(false); // no more batches this time getMoreResults2.setResultList(new TestObj[] { more }); prepareServer(gson.toJson(getMoreResults2)); - advCursor.next(); + results.next(); path = requestURI.substring(requestURI.lastIndexOf('/')); assertEquals("/get-more", path); @@ -668,7 +665,6 @@ } assertNotNull(results); assertTrue(results instanceof WebCursor); - assertTrue("Expected WebCursor to be an AdvancedCursor", results instanceof BatchCursor); assertTrue(results.hasNext()); assertEquals("fluffor1", results.next().getProperty1()); assertTrue(results.hasNext());
--- a/web/server/src/main/java/com/redhat/thermostat/web/server/CursorManager.java Wed Apr 01 20:52:28 2015 +0200 +++ b/web/server/src/main/java/com/redhat/thermostat/web/server/CursorManager.java Wed Apr 01 19:23:47 2015 +0200 @@ -44,7 +44,7 @@ import java.util.Timer; import java.util.TimerTask; -import com.redhat.thermostat.storage.core.experimental.BatchCursor; +import com.redhat.thermostat.storage.core.Cursor; /** * Manages (query) cursors for a single user. @@ -82,7 +82,7 @@ * @return The cursor ID or {@link CursorManager#CURSOR_NOT_STORED} if the * passed in cursor has no more elements. */ - synchronized int put(final BatchCursor<?> cursor) { + synchronized int put(final Cursor<?> cursor) { int cursorId = CURSOR_NOT_STORED; if (cursor.hasNext()) { // Be sure we don't overflow. For a long running web storage we @@ -100,7 +100,7 @@ return cursorId; } - synchronized BatchCursor<?> get(int cursorId) { + synchronized Cursor<?> get(int cursorId) { CursorHolder holder = cursors.get(cursorId); if (holder == null) { return null; @@ -152,10 +152,10 @@ // The time out in minutes static final int TIMEOUT = 3 * MINUTES; - private final BatchCursor<?> cursor; + private final Cursor<?> cursor; private long lastUpdated; - CursorHolder(BatchCursor<?> cursor, long lastUpdated) { + CursorHolder(Cursor<?> cursor, long lastUpdated) { this.cursor = cursor; this.lastUpdated = lastUpdated; } @@ -168,7 +168,7 @@ return checkIsCursorExpired(currentTime, TIMEOUT); } - BatchCursor<?> getCursor() { + Cursor<?> getCursor() { return cursor; }
--- a/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java Wed Apr 01 20:52:28 2015 +0200 +++ b/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java Wed Apr 01 19:23:47 2015 +0200 @@ -74,6 +74,7 @@ import com.redhat.thermostat.shared.config.CommonPaths; import com.redhat.thermostat.shared.config.InvalidConfigurationException; import com.redhat.thermostat.shared.config.internal.CommonPathsImpl; +import com.redhat.thermostat.storage.core.BasicBatchCursor; import com.redhat.thermostat.storage.core.Categories; import com.redhat.thermostat.storage.core.Category; import com.redhat.thermostat.storage.core.CategoryAdapter; @@ -93,7 +94,6 @@ import com.redhat.thermostat.storage.core.StatementDescriptor; import com.redhat.thermostat.storage.core.Storage; import com.redhat.thermostat.storage.core.StorageCredentials; -import com.redhat.thermostat.storage.core.experimental.BatchCursor; import com.redhat.thermostat.storage.model.AggregateResult; import com.redhat.thermostat.storage.model.Pojo; import com.redhat.thermostat.storage.query.BinaryLogicalExpression; @@ -125,7 +125,7 @@ public class WebStorageEndPoint extends HttpServlet { // This is an ugly hack in order to allow for testing of batched querying. - static int DEFAULT_QUERY_BATCH_SIZE = BatchCursor.DEFAULT_BATCH_SIZE; + static int DEFAULT_QUERY_BATCH_SIZE = Cursor.DEFAULT_BATCH_SIZE; static final String CMDC_AUTHORIZATION_GRANT_ROLE_PREFIX = "thermostat-cmdc-grant-"; static final String FILES_READ_GRANT_ROLE_PREFIX = "thermostat-files-grant-read-filename-"; @@ -136,7 +136,6 @@ static final String CATEGORY_MANAGER_KEY = "category-manager"; static final String PREPARED_STMT_MANAGER_KEY = "prepared-stmt-manager"; static final String SERVER_TOKEN_KEY = "server-token"; - private static final int UNKNOWN_CURSOR_ID = -0xdeadbeef; // our strings can contain non-ASCII characters. Use UTF-8 // see also PR 1344 @@ -649,41 +648,28 @@ UserPrincipal userPrincipal = getUserPrincipal(req); targetQuery = getQueryForPrincipal(userPrincipal, targetQuery, desc); - // While the signature still says the retval of query execute is - // cursor, we return an instance of AdvancedCursor instead for new code. - // This is the case for MongoStorage. However, in order to work - // around potential third-party implementations perform the check - // and fall back to legacy behaviour. Cursor<T> cursor = targetQuery.execute(); List<T> resultsList = null; - if (cursor instanceof BatchCursor) { - BatchCursor<T> batchCursor = (BatchCursor<T>)cursor; - resultsList = getBatchFromCursor(batchCursor, DEFAULT_QUERY_BATCH_SIZE); - assert(resultsList.size() <= DEFAULT_QUERY_BATCH_SIZE); - CursorManager cursorManager = null; - HttpSession userSession = req.getSession(); - synchronized(userSession) { - cursorManager = (CursorManager)userSession.getAttribute(CURSOR_MANAGER_KEY); - if (cursorManager == null) { - // Not yet set for this user, create a new cursor manager - // and start the sweeper timer so as to prevent memory - // leaks due to cursors kept as a reference in cursor manager - cursorManager = new CursorManager(timerRegistry); - cursorManager.startSweeperTimer(); - userSession.setAttribute(CURSOR_MANAGER_KEY, cursorManager); - } + resultsList = getBatchFromCursor(cursor, DEFAULT_QUERY_BATCH_SIZE); + assert(resultsList.size() <= DEFAULT_QUERY_BATCH_SIZE); + CursorManager cursorManager = null; + HttpSession userSession = req.getSession(); + synchronized(userSession) { + cursorManager = (CursorManager)userSession.getAttribute(CURSOR_MANAGER_KEY); + if (cursorManager == null) { + // Not yet set for this user, create a new cursor manager + // and start the sweeper timer so as to prevent memory + // leaks due to cursors kept as a reference in cursor manager + cursorManager = new CursorManager(timerRegistry); + cursorManager.startSweeperTimer(); + userSession.setAttribute(CURSOR_MANAGER_KEY, cursorManager); } - // Only record cursor if there are more results to return than the - // first batch size. - int cursorId = cursorManager.put(batchCursor); - response.setCursorId(cursorId); - response.setHasMoreBatches(batchCursor.hasNext()); - } else { - // fallback to old behaviour - resultsList = getLegacyResultList(cursor); - response.setHasMoreBatches(false); // only one batch - response.setCursorId(UNKNOWN_CURSOR_ID); } + // Only record cursor if there are more results to return than the + // first batch size. + int cursorId = cursorManager.put(cursor); + response.setCursorId(cursorId); + response.setHasMoreBatches(cursor.hasNext()); writeQueryResponse(resp, response, resultsList, targetStmtHolder); } @@ -756,7 +742,7 @@ throw new IllegalStateException("[get-more] No cursor manager available in session for " + req.getRemoteUser()); } @SuppressWarnings("unchecked") - BatchCursor<T> batchCursor = (BatchCursor<T>)cursorManager.get(cursorId); + Cursor<T> batchCursor = (Cursor<T>)cursorManager.get(cursorId); PreparedStatementManager prepStmtManager = getPreparedStmtManager(); PreparedStatementHolder<T> targetStmtHolder = prepStmtManager.getStatementHolder(id); @@ -810,7 +796,7 @@ // Fetches the first batch of results. Number of results are determined // by the default batch size in AdvancedCursor - private <T extends Pojo> List<T> getBatchFromCursor(final BatchCursor<T> cursor, final int batchSize) { + private <T extends Pojo> List<T> getBatchFromCursor(final Cursor<T> cursor, final int batchSize) { ArrayList<T> resultList = new ArrayList<>(batchSize); for (int i = 0; i < batchSize && cursor.hasNext(); i++) { resultList.add(cursor.next()); @@ -818,16 +804,6 @@ return resultList; } - // Fetches all results imposing no bound on the result set if the underlying - // query was unbounded. - private <T extends Pojo> ArrayList<T> getLegacyResultList(Cursor<T> cursor) { - ArrayList<T> resultList = new ArrayList<>(); - while (cursor.hasNext()) { - resultList.add(cursor.next()); - } - return resultList; - } - @SuppressWarnings("unchecked") @WebStoragePathHandler( path = "write-execute" ) private <T extends Pojo> void writeExecute(HttpServletRequest req, HttpServletResponse resp) throws IOException { @@ -1059,7 +1035,7 @@ } private <T extends Pojo> Cursor<T> getEmptyCursor() { - final Cursor<T> empty = new Cursor<T>() { + final Cursor<T> empty = new BasicBatchCursor<T>() { @Override public boolean hasNext() {
--- a/web/server/src/test/java/com/redhat/thermostat/web/server/CursorManagerTest.java Wed Apr 01 20:52:28 2015 +0200 +++ b/web/server/src/test/java/com/redhat/thermostat/web/server/CursorManagerTest.java Wed Apr 01 19:23:47 2015 +0200 @@ -54,7 +54,7 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; -import com.redhat.thermostat.storage.core.experimental.BatchCursor; +import com.redhat.thermostat.storage.core.Cursor; import com.redhat.thermostat.web.server.CursorManager.CursorHolder; import com.redhat.thermostat.web.server.CursorManager.CursorSweeper; import com.redhat.thermostat.web.server.CursorManager.CursorTimer; @@ -106,12 +106,12 @@ @Test public void testPutNoHasMoreCursor() { CursorManager manager = new CursorManager(mock(TimerRegistry.class)); - int id = manager.put(mock(BatchCursor.class)); + int id = manager.put(mock(Cursor.class)); assertEquals(CursorManager.CURSOR_NOT_STORED, id); } - private BatchCursor<?> getHasMoreBatchCursor() { - BatchCursor<?> c = mock(BatchCursor.class); + private Cursor<?> getHasMoreBatchCursor() { + Cursor<?> c = mock(Cursor.class); when(c.hasNext()).thenReturn(true); return c; } @@ -119,7 +119,7 @@ @Test public void testGetInvalidId() { CursorManager manager = new CursorManager(mock(TimerRegistry.class)); - BatchCursor<?> c = manager.get(CursorManager.CURSOR_NOT_STORED); + Cursor<?> c = manager.get(CursorManager.CURSOR_NOT_STORED); assertNull(c); } @@ -133,11 +133,11 @@ CursorManager manager = new CursorManager(mock(TimerRegistry.class)); int num = (int)(Math.random() * 300); addCursors(num, manager); - BatchCursor<?> cursor = getHasMoreBatchCursor(); + Cursor<?> cursor = getHasMoreBatchCursor(); int interestingId = manager.put(cursor); num = (int)(Math.random() * 40); addCursors(num, manager); - BatchCursor<?> actual = manager.get(interestingId); + Cursor<?> actual = manager.get(interestingId); assertSame(actual, cursor); } @@ -146,10 +146,10 @@ Map<Integer, CursorHolder> cursors = new HashMap<>(); long expiredTime = System.currentTimeMillis() - ( 5 * 60 * 1000); long notExpiredTime = System.currentTimeMillis(); - cursors.put(3, new CursorHolder(mock(BatchCursor.class), expiredTime)); - cursors.put(4, new CursorHolder(mock(BatchCursor.class), expiredTime)); - cursors.put(5, new CursorHolder(mock(BatchCursor.class), notExpiredTime)); - cursors.put(7, new CursorHolder(mock(BatchCursor.class), expiredTime)); + cursors.put(3, new CursorHolder(mock(Cursor.class), expiredTime)); + cursors.put(4, new CursorHolder(mock(Cursor.class), expiredTime)); + cursors.put(5, new CursorHolder(mock(Cursor.class), notExpiredTime)); + cursors.put(7, new CursorHolder(mock(Cursor.class), expiredTime)); CursorManager manager = new CursorManager(mock(TimerRegistry.class), cursors); manager.expireCursors(); // should remove old cursors assertEquals(1, cursors.keySet().size()); @@ -159,9 +159,9 @@ @Test public void testRemoveCursor() { Map<Integer, CursorHolder> cursors = new HashMap<>(); - cursors.put(3, new CursorHolder(mock(BatchCursor.class), 3)); - cursors.put(4, new CursorHolder(mock(BatchCursor.class), 4)); - cursors.put(5, new CursorHolder(mock(BatchCursor.class), 5)); + cursors.put(3, new CursorHolder(mock(Cursor.class), 3)); + cursors.put(4, new CursorHolder(mock(Cursor.class), 4)); + cursors.put(5, new CursorHolder(mock(Cursor.class), 5)); CursorManager manager = new CursorManager(mock(TimerRegistry.class), cursors); manager.removeCursor(3); assertEquals(2, cursors.keySet().size()); @@ -175,9 +175,9 @@ Map<Integer, CursorHolder> cursors = new HashMap<>(); long expiredTime = System.currentTimeMillis() - ( 5 * 60 * 1000); long notExpiredTime = System.currentTimeMillis(); - cursors.put(4, new CursorHolder(mock(BatchCursor.class), expiredTime)); - cursors.put(5, new CursorHolder(mock(BatchCursor.class), notExpiredTime)); - cursors.put(7, new CursorHolder(mock(BatchCursor.class), expiredTime)); + cursors.put(4, new CursorHolder(mock(Cursor.class), expiredTime)); + cursors.put(5, new CursorHolder(mock(Cursor.class), notExpiredTime)); + cursors.put(7, new CursorHolder(mock(Cursor.class), expiredTime)); CursorManager manager = new CursorManager(mock(TimerRegistry.class), cursors); // refresh 4's timestamp so that it's now no longer expired manager.updateCursorTimeStamp(4); @@ -231,7 +231,7 @@ @Test public void testUpdateTimeStamp() { long now = System.currentTimeMillis(); - CursorHolder holder = new CursorHolder(mock(BatchCursor.class), now); + CursorHolder holder = new CursorHolder(mock(Cursor.class), now); sleep(10); holder.updateTimestamp(); assertTrue(now != holder.getLastUpdated()); @@ -240,7 +240,7 @@ @Test public void testCheckIsCursorExpired() { long now = System.currentTimeMillis(); - CursorHolder holder = new CursorHolder(mock(BatchCursor.class), now); + CursorHolder holder = new CursorHolder(mock(Cursor.class), now); sleep(10); long laterTime = System.currentTimeMillis(); assertFalse("cursor still valid. timeout == 20ms, but only 10ms old.", @@ -251,7 +251,7 @@ @Test public void testGetCursor() { long now = System.currentTimeMillis(); - BatchCursor<?> cursor = mock(BatchCursor.class); + Cursor<?> cursor = mock(Cursor.class); CursorHolder holder = new CursorHolder(cursor, now); assertSame(cursor, holder.getCursor()); }
--- a/web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndpointTest.java Wed Apr 01 20:52:28 2015 +0200 +++ b/web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndpointTest.java Wed Apr 01 19:23:47 2015 +0200 @@ -123,7 +123,6 @@ import com.redhat.thermostat.storage.core.StatementDescriptor; import com.redhat.thermostat.storage.core.auth.CategoryRegistration; import com.redhat.thermostat.storage.core.auth.StatementDescriptorRegistration; -import com.redhat.thermostat.storage.core.experimental.BatchCursor; import com.redhat.thermostat.storage.dao.HostInfoDAO; import com.redhat.thermostat.storage.model.AggregateCount; import com.redhat.thermostat.storage.model.BasePojo; @@ -538,10 +537,10 @@ private final Gson gson; private final Query<TestClass> mockMongoQuery; - private final BatchCursor<TestClass> cursor; + private final Cursor<TestClass> cursor; private final SharedStateId stmtId; - private TrustedPreparedQueryTestResult(Gson gson, Query<TestClass> mockMongoQuery, BatchCursor<TestClass> cursor, SharedStateId stmtId) { + private TrustedPreparedQueryTestResult(Gson gson, Query<TestClass> mockMongoQuery, Cursor<TestClass> cursor, SharedStateId stmtId) { this.cursor = cursor; this.gson = gson; this.mockMongoQuery = mockMongoQuery; @@ -561,7 +560,7 @@ Query<TestClass> mockMongoQuery = mock(Query.class); when(mockStorage.createQuery(eq(category))).thenReturn(mockMongoQuery); - BatchCursor<TestClass> cursor = mock(BatchCursor.class); + Cursor<TestClass> cursor = mock(Cursor.class); WebStorageEndPoint.DEFAULT_QUERY_BATCH_SIZE = 2; // Assuming: moreBatches == true then we have // WebStorageEndpoint.getBatchFromCursor() method calls hasNext() twice,