Mercurial > hg > release > thermostat-1.4

changeset 1544:d318a5be8867

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Return null for all Cursor.next() impls if no more elements. Reviewed-by: omajid Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2014-November/011545.html
author Severin Gehwolf <sgehwolf@redhat.com>
date Wed, 05 Nov 2014 16:55:16 +0100
parents 87c0133216f6
children 722862a0c335
files integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/WebAppTest.java storage/core/src/main/java/com/redhat/thermostat/storage/core/Cursor.java storage/core/src/main/java/com/redhat/thermostat/storage/model/AggregateCount.java storage/core/src/test/java/com/redhat/thermostat/storage/model/AggregateCountTest.java storage/mongo/src/main/java/com/redhat/thermostat/storage/mongodb/internal/MongoCursor.java web/client/src/main/java/com/redhat/thermostat/web/client/internal/WebCursor.java web/client/src/test/java/com/redhat/thermostat/web/client/internal/WebCursorTest.java web/client/src/test/java/com/redhat/thermostat/web/client/internal/WebStorageTest.java
diffstat 8 files changed, 55 insertions(+), 40 deletions(-) [+]
line wrap: on
line diff
--- a/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/WebAppTest.java	Thu Nov 13 11:38:01 2014 -0500
+++ b/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/WebAppTest.java	Wed Nov 05 16:55:16 2014 +0100
@@ -39,6 +39,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -57,7 +58,6 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.NoSuchElementException;
 import java.util.Properties;
 import java.util.Set;
 import java.util.UUID;
@@ -95,11 +95,11 @@
 import com.redhat.thermostat.storage.core.Storage;
 import com.redhat.thermostat.storage.core.StorageCredentials;
 import com.redhat.thermostat.storage.core.auth.DescriptorMetadata;
+import com.redhat.thermostat.storage.dao.AgentInfoDAO;
 import com.redhat.thermostat.storage.dao.HostInfoDAO;
+import com.redhat.thermostat.storage.model.AgentInformation;
 import com.redhat.thermostat.storage.model.AggregateCount;
 import com.redhat.thermostat.storage.model.HostInfo;
-import com.redhat.thermostat.storage.dao.AgentInfoDAO;
-import com.redhat.thermostat.storage.model.AgentInformation;
 import com.redhat.thermostat.storage.mongodb.internal.MongoStorage;
 import com.redhat.thermostat.storage.query.Expression;
 import com.redhat.thermostat.storage.query.ExpressionFactory;
@@ -791,12 +791,7 @@
         
         Cursor<CpuStat> cursor = query.executeQuery();
         assertFalse(cursor.hasNext());
-        try {
-            cursor.next();
-            fail("cursor should have thrown exception!");
-        } catch (NoSuchElementException e) {
-            // pass
-        }
+        assertNull(cursor.next());
 
         webStorage.getConnection().disconnect();
     }
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/core/Cursor.java	Thu Nov 13 11:38:01 2014 -0500
+++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/Cursor.java	Wed Nov 05 16:55:16 2014 +0100
@@ -36,20 +36,43 @@
 
 package com.redhat.thermostat.storage.core;
 
+import java.util.NoSuchElementException;
+
 import com.redhat.thermostat.storage.model.Pojo;
 
 /**
- * Allows traversing over objects obtained from Storage.
+ * Allows traversing over objects obtained from {@link Storage}.
+ * 
+ * @see PreparedStatement#executeQuery()
  */
 public interface Cursor<T extends Pojo> {
 
     /**
+     * @return {@code true} if there are more elements, {@code false} otherwise.
+     * 
      * @throws StorageException
+     *             If there was a problem with underlying {@link Storage}.
      */
     boolean hasNext();
 
     /**
+     * Retrieves the next element from the result set. Users are advised to call
+     * {@link #hasNext()} prior to calling this method.
+     * 
+     * @return <p>
+     *         The next element of the result set. {@code null} if there is no
+     *         next element.
+     *         </p>
+     *         <p>
+     *         <strong>Please note: </strong> This will change with the next
+     *         release. In the next major release a
+     *         {@link NoSuchElementException} will be thrown if there is no next
+     *         element. I.e. {@link #hasNext()} returns {@code false}, but
+     *         {@code next()} is still being called.
+     *         </p>
+     * 
      * @throws StorageException
+     *             If there was a problem with underlying {@link Storage}.
      */
     T next();
 
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/model/AggregateCount.java	Thu Nov 13 11:38:01 2014 -0500
+++ b/storage/core/src/main/java/com/redhat/thermostat/storage/model/AggregateCount.java	Wed Nov 05 16:55:16 2014 +0100
@@ -36,9 +36,11 @@
 
 package com.redhat.thermostat.storage.model;
 
-import java.util.NoSuchElementException;
 import java.util.Objects;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
+import com.redhat.thermostat.common.utils.LoggingUtils;
 import com.redhat.thermostat.storage.core.Cursor;
 import com.redhat.thermostat.storage.core.Entity;
 import com.redhat.thermostat.storage.core.Persist;
@@ -85,6 +87,7 @@
     
     private static class AggregateCursor<T extends Pojo> extends BasicBatchCursor<T> {
 
+        private static final Logger logger = LoggingUtils.getLogger(AggregateCursor.class);
         private boolean available = true;
         private final T count;
         
@@ -103,7 +106,11 @@
                 available = false;
                 return count;
             } else {
-                throw new NoSuchElementException();
+                // FIXME: Thermostat 2.0: Change to throwing NoSuchElementException
+                String warning = "No next element but next() is being called. " +
+                                 "This will throw NoSuchElementException in the next release!";
+                logger.log(Level.WARNING, warning);
+                return null;
             }
         }
         
--- a/storage/core/src/test/java/com/redhat/thermostat/storage/model/AggregateCountTest.java	Thu Nov 13 11:38:01 2014 -0500
+++ b/storage/core/src/test/java/com/redhat/thermostat/storage/model/AggregateCountTest.java	Wed Nov 05 16:55:16 2014 +0100
@@ -38,10 +38,8 @@
 
 import static org.junit.Assert.assertEquals;
 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 java.util.NoSuchElementException;
 
 import org.junit.Test;
 
@@ -59,12 +57,7 @@
         AggregateCount actual = cursor.next();
         assertEquals(10, actual.getCount());
         assertFalse(cursor.hasNext());
-        try {
-            cursor.next();
-            fail("Should have thrown NoSuchElementException!");
-        } catch (NoSuchElementException e) {
-            // pass
-        }
+        assertNull(cursor.next());
     }
     
     /**
--- a/storage/mongo/src/main/java/com/redhat/thermostat/storage/mongodb/internal/MongoCursor.java	Thu Nov 13 11:38:01 2014 -0500
+++ b/storage/mongo/src/main/java/com/redhat/thermostat/storage/mongodb/internal/MongoCursor.java	Wed Nov 05 16:55:16 2014 +0100
@@ -36,16 +36,20 @@
 
 package com.redhat.thermostat.storage.mongodb.internal;
 
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
 import com.mongodb.DBCursor;
 import com.mongodb.DBObject;
 import com.mongodb.MongoException;
+import com.redhat.thermostat.common.utils.LoggingUtils;
 import com.redhat.thermostat.storage.core.StorageException;
 import com.redhat.thermostat.storage.core.experimental.BasicBatchCursor;
-import com.redhat.thermostat.storage.core.experimental.BatchCursor;
 import com.redhat.thermostat.storage.model.Pojo;
 
 class MongoCursor<T extends Pojo> extends BasicBatchCursor<T> {
 
+    private static final Logger logger = LoggingUtils.getLogger(MongoCursor.class);
     private DBCursor cursor;
     private Class<T> resultClass;
 
@@ -68,8 +72,10 @@
         try {
             DBObject next = cursor.next();
             if (next == null) {
-                // FIXME: This is inconsistent with other cursors throwing
-                //        NoSuchElementException
+                // FIXME: Thermostat 2.0: Change to throwing NoSuchElementException
+                String warning = "No next element but next() is being called. " +
+                                 "This will throw NoSuchElementException in the next release!";
+                logger.log(Level.WARNING, warning);
                 return null;
             }
             MongoPojoConverter converter = new MongoPojoConverter();
--- a/web/client/src/main/java/com/redhat/thermostat/web/client/internal/WebCursor.java	Thu Nov 13 11:38:01 2014 -0500
+++ b/web/client/src/main/java/com/redhat/thermostat/web/client/internal/WebCursor.java	Wed Nov 05 16:55:16 2014 +0100
@@ -38,7 +38,6 @@
 package com.redhat.thermostat.web.client.internal;
 
 import java.lang.reflect.Type;
-import java.util.NoSuchElementException;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -81,7 +80,11 @@
     @Override
     public T next() {
         if (batchIndex >= dataBatch.length && !hasMoreBatches) {
-            throw new NoSuchElementException();
+            // FIXME: Thermostat 2.0: Change to throwing NoSuchElementException
+            String warning = "No next element but next() is being called. " +
+                             "This will throw NoSuchElementException in the next release!";
+            logger.log(Level.WARNING, warning);
+            return null;
         }
         T result = null;
         // Check if we have still results left in batch,
--- a/web/client/src/test/java/com/redhat/thermostat/web/client/internal/WebCursorTest.java	Thu Nov 13 11:38:01 2014 -0500
+++ b/web/client/src/test/java/com/redhat/thermostat/web/client/internal/WebCursorTest.java	Wed Nov 05 16:55:16 2014 +0100
@@ -46,7 +46,6 @@
 import static org.mockito.Mockito.when;
 
 import java.lang.reflect.Type;
-import java.util.NoSuchElementException;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -96,12 +95,7 @@
         boolean hasMoreBatches = false;
         TestObj[] dataBatch = new TestObj[] { };
         WebCursor<TestObj> cursor = new WebCursor<>(storage, dataBatch, hasMoreBatches, cursorId, fakeType, stmt);
-        try {
-            cursor.next();
-            fail("no results and no more batches, expected NSEE");
-        } catch (NoSuchElementException e) {
-            // pass
-        }
+        assertNull(cursor.next());
         
         // test empty results but more batches
         hasMoreBatches = true;
--- a/web/client/src/test/java/com/redhat/thermostat/web/client/internal/WebStorageTest.java	Thu Nov 13 11:38:01 2014 -0500
+++ b/web/client/src/test/java/com/redhat/thermostat/web/client/internal/WebStorageTest.java	Wed Nov 05 16:55:16 2014 +0100
@@ -62,7 +62,6 @@
 import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.NoSuchElementException;
 import java.util.concurrent.CountDownLatch;
 
 import javax.servlet.ServletException;
@@ -415,12 +414,7 @@
         fakeQueryResponse.setHasMoreBatches(false);
         Cursor<TestObj> results = doBasicPrepareAndExecuteQueryTest(fakeQueryResponse);
         assertFalse(results.hasNext());
-        try {
-            results.next();
-            fail();
-        } catch (NoSuchElementException ex) {
-            // Pass.
-        }
+        assertNull(results.next());
     }
     
     /**