Merge pull request #127 from DemocracyDevelopers/queries-tests

Test for DB queries

server/eclipse-project/pom.xml

@@ -25,7 +25,7 @@
 	<properties>
 		<compileSource>11</compileSource>
 		<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
-		<corla.test.excludedGroups>integration</corla.test.excludedGroups>
+		<!--<corla.test.excludedGroups>integration</corla.test.excludedGroups>-->
 		<checkstyle.skip>true</checkstyle.skip>
 		<maven.javadoc.skip>true</maven.javadoc.skip>
         <maven.javadoc.failOnError>false</maven.javadoc.failOnError>

server/eclipse-project/src/main/java/us/freeandfair/corla/query/AdministratorQueries.java

@@ -63,7 +63,8 @@ public static Administrator byUsername(final String the_username) {
       // if there's exactly one result, return that
       if (query_results.size() == 1) {
         result = query_results.get(0);
-      } 
+      }
+      // TODO: if two admins have the same username, null gets returned. This seems like a bug?
     } catch (final PersistenceException e) {
       Main.LOGGER.error("could not query database for administrator");
     }

server/eclipse-project/src/main/java/us/freeandfair/corla/query/BallotManifestInfoQueries.java

@@ -206,6 +206,7 @@ Find the batch(bmi) that would hold the sequence number given.
       result = new HashSet<BallotManifestInfo>(query.getResultList());
     } catch (final PersistenceException e) {
       Main.LOGGER.error("Exception when reading ballot manifests from database: ", e);
+      return Optional.empty();
     }
     return result.stream().findFirst();
   }

server/eclipse-project/src/main/java/us/freeandfair/corla/query/CastVoteRecordQueries.java

@@ -149,7 +149,7 @@ public static OptionalLong countMatching(final RecordType the_type) {
     } catch (final PersistenceException e) {
       Main.LOGGER.error(COULD_NOT_QUERY_DATABASE);
     }
-    if (result == null) {
+    if (result == OptionalLong.empty()) {
       Main.LOGGER.debug("found no CVRs for type " + the_type);
     } else {
       Main.LOGGER.debug("query succeeded, returning CVR stream");
@@ -222,7 +222,7 @@ public static OptionalLong countMatching(final Long the_county, final RecordType
     } catch (final PersistenceException e) {
       Main.LOGGER.error(COULD_NOT_QUERY_DATABASE);
     }
-    if (result == null) {
+    if (result == OptionalLong.empty()) {
       Main.LOGGER.debug("found no CVRs for county " + the_county + ", type " + the_type);
     } else {
       Main.LOGGER.debug("query succeeded, returning CVR stream");
@@ -323,6 +323,7 @@ public static CastVoteRecord get(final Long the_county_id, final RecordType the_
       final TypedQuery<CastVoteRecord> query = s.createQuery(cq);
       final List<CastVoteRecord> query_results = query.getResultList();
       // if there's exactly one result, return that
+      // TODO the else branch here shouldn't return null?
       if (query_results.size() == 1) {
         result = query_results.get(0);
       }
@@ -352,6 +353,9 @@ public static CastVoteRecord get(final Long the_county_id, final RecordType the_
   public static Map<Integer, CastVoteRecord> get(final Long the_county_id,
                                                  final RecordType the_type,
                                                  final List<Integer> the_sequence_numbers) {
+
+    // TODO: this doesn't handle the case where two CVRs in the same county have the same sequence number.
+    // TODO: There should probably be a DB error or something if that happens.
     Map<Integer, CastVoteRecord> result = null;
     final Set<Integer> unique_numbers = new HashSet<>(the_sequence_numbers);
 
@@ -411,7 +415,7 @@ public static List<CastVoteRecord> get(final List<Long> the_ids) {
     } catch (final PersistenceException e) {
       Main.LOGGER.error(COULD_NOT_QUERY_DATABASE);
     }
-    if (result == null) {
+    if (result.isEmpty()) {
       Main.LOGGER.debug("found no CVRs with ids " + the_ids);
       return new ArrayList<>();
     } else {
@@ -474,7 +478,7 @@ public static List<CastVoteRecord> atPosition(final List<Tribute> tributes) {
           return t.getUri();
         }))
             // is it faster to let the db do this with an except query?
-            .filter(t -> !foundUris.contains(t.getUri())).map(t -> phantomRecord(t))
+            .filter(t -> !foundUris.contains(t.getUri())).map(CastVoteRecordQueries::phantomRecord)
             .map(Persistence::persist).collect(Collectors.toSet());
 
     results.addAll(phantomRecords);
@@ -496,8 +500,10 @@ public static List<CastVoteRecord> atPosition(final List<Tribute> tributes) {
     }
 
     final List<CastVoteRecord> returnList =
-        randomOrder.stream().filter(cvr -> null != cvr).collect(Collectors.toList());
+        randomOrder.stream().filter(Objects::nonNull).collect(Collectors.toList());
     if (returnList.size() != uris.size()) {
+      // TODO: I'm pretty sure this code is unreachable, since any time |URIs| < |return|, we
+      // TODO: make phantoms until they equal. Maybe take this out?
       // we got a problem here
       Main.LOGGER
           .error("something went wrong with atPosition - returnList.size() != uris.size()");
@@ -570,13 +576,15 @@ public static Long maxRevision(final CastVoteRecord cvr) {
     q.setLong("countyId", cvr.countyID());
     q.setString("imprintedId", cvr.imprintedID());
 
-    final Long result = (Long) q.getSingleResult();
-
-    if (null == result) {
+    try {
+      return (Long) q.getSingleResult();
+    } catch (final PersistenceException e) {
+      // the DB had a problem!
+      // TODO: Technically this should probably be an error code?
+      // TODO: Otherwise there's no way to discern this from a CVR with no revisions?
       return 0L;
-    } else {
-      return result;
     }
+
   }
 
   /**

server/eclipse-project/src/main/java/us/freeandfair/corla/query/ContestQueries.java

@@ -104,16 +104,21 @@ public static Set<Contest> forCounty(final County the_county) {
   /**
    * Deletes all the contests for the county with the specified ID.
    * 
-   * @param the_id The county ID.
+   * @param the_county_id The county ID.
+   * @return the number of contests that were deleted, or -1 if an error occured.
    */
-  public static void deleteForCounty(final Long the_county_id) {
+  public static int deleteForCounty(final Long the_county_id) {
     final Set<Contest> contests = 
         forCounty(Persistence.getByID(the_county_id, County.class));
+
+    int retval = -1;
     if (contests != null) {
+      retval = contests.size();
       for (final Contest c : contests) {
         Persistence.delete(c);
       }
     }
     Persistence.flush();
+    return retval;
   }
 }

server/eclipse-project/src/test/java/us/freeandfair/corla/controllers/BallotSelectionTest.java

@@ -31,56 +31,15 @@
 import org.hibernate.Session;
 import us.freeandfair.corla.query.CastVoteRecordQueries;
 import us.freeandfair.corla.query.Setup;
+import us.freeandfair.corla.util.TestClassWithDatabase;
 
 @Test(groups = {"integration"})
-public class BallotSelectionTest {
+public class BallotSelectionTest extends TestClassWithDatabase {
 
   private BallotSelectionTest() {};
 
   private Boolean return_cvr = true;
 
-  /**
-   * Container for the mock-up database.
-   */
-  static PostgreSQLContainer<?> postgres
-          = new PostgreSQLContainer<>("postgres:15-alpine")
-          // None of these actually have to be the same as the real database (except its name), but this
-          // makes it easy to match the setup scripts.
-          .withDatabaseName("corla")
-          .withUsername("corlaadmin")
-          .withPassword("corlasecret")
-          // .withInitScripts("corlaInit.sql","contest.sql");
-          .withInitScript("SQL/corlaInitEmpty.sql");
-
-  @BeforeClass
-  public static void beforeAll() {
-    postgres.start();
-    Properties hibernateProperties = new Properties();
-    hibernateProperties.setProperty("hibernate.driver", "org.postgresql.Driver");
-    hibernateProperties.setProperty("hibernate.url", postgres.getJdbcUrl());
-    hibernateProperties.setProperty("hibernate.user", postgres.getUsername());
-    hibernateProperties.setProperty("hibernate.pass", postgres.getPassword());
-    hibernateProperties.setProperty("hibernate.dialect", "org.hibernate.dialect.PostgreSQL9Dialect");
-    Persistence.setProperties(hibernateProperties);
-    Persistence.beginTransaction();
-
-  }
-
-  @AfterClass
-  public static void afterAll() {
-    postgres.stop();
-  }
-
-  @BeforeMethod
-  public static void beforeEach() {
-    Persistence.beginTransaction();
-  }
-
-  @AfterMethod
-  public static void afterEach() {
-    Persistence.rollbackTransaction();
-  }
-
   @Test()
   public void testAuditedPrefixLengthWithNone() {
     List<Long> cvrIds = new ArrayList<>();

server/eclipse-project/src/test/java/us/freeandfair/corla/endpoint/StartAuditRoundTest.java

@@ -2,36 +2,23 @@
 
 import static org.testng.Assert.assertEquals;
 
-import org.testng.annotations.AfterTest;
-import org.testng.annotations.BeforeTest;
-import org.testng.annotations.Test;
+import org.testcontainers.containers.PostgreSQLContainer;
+import org.testng.annotations.*;
 
 import us.freeandfair.corla.asm.CountyDashboardASM;
 import us.freeandfair.corla.model.County;
 import us.freeandfair.corla.model.CountyDashboard;
 import us.freeandfair.corla.persistence.Persistence;
 import us.freeandfair.corla.query.Setup;
+import us.freeandfair.corla.util.TestClassWithDatabase;
+
+import java.util.Properties;
 
 @Test(groups = {"integration"})
-public class StartAuditRoundTest {
+public class StartAuditRoundTest extends TestClassWithDatabase {
 
   private StartAuditRoundTest() {};
 
-
-  @BeforeTest()
-  public void setUp() {
-    Setup.setProperties();
-    Persistence.beginTransaction();
-  }
-
-  @AfterTest()
-  public void tearDown() {
-    try {
-      Persistence.rollbackTransaction();
-    } catch (Exception e) {
-    }
-  }
-
   // this test doesn't do much yet
   @Test()
   public void testReadyToStartFalse() {

server/eclipse-project/src/test/java/us/freeandfair/corla/query/AdministratorQueriesTest.java

@@ -0,0 +1,93 @@
+package us.freeandfair.corla.query;
+
+import org.testcontainers.containers.PostgreSQLContainer;
+import org.testng.annotations.*;
+import us.freeandfair.corla.model.Administrator;
+import us.freeandfair.corla.model.County;
+import us.freeandfair.corla.persistence.Persistence;
+import us.freeandfair.corla.util.TestClassWithDatabase;
+
+import java.util.Properties;
+
+import static org.testng.AssertJUnit.assertEquals;
+import static org.testng.AssertJUnit.assertNull;
+
+@Test
+public class AdministratorQueriesTest extends TestClassWithDatabase {
+
+    @Test
+    public void testByUsernameState() {
+
+        Administrator admin = new Administrator("testname",
+                                                Administrator.AdministratorType.STATE,
+                                                "fulltestname",
+                                                null);
+
+        Persistence.saveOrUpdate(admin);
+        assertEquals(AdministratorQueries.byUsername("testname"), admin);
+    }
+
+
+    @Test
+    public void testByUsernameCounty() {
+
+        County county = new County("test", 1L);
+        Persistence.saveOrUpdate(county);
+        Administrator admin = new Administrator("testname",
+                Administrator.AdministratorType.COUNTY,
+                "fulltestname",
+                county
+                );
+
+        Persistence.saveOrUpdate(admin);
+        assertEquals(AdministratorQueries.byUsername("testname"), admin);
+    }
+
+    @Test
+    public void testByUsernameStateAndCounty() {
+        County county = new County("test", 1L);
+        Persistence.saveOrUpdate(county);
+        Administrator countyadmin = new Administrator("county",
+                Administrator.AdministratorType.COUNTY,
+                "countyfull",
+                county);
+
+        Persistence.saveOrUpdate(countyadmin);
+
+        Administrator stateadmin = new Administrator("state",
+                Administrator.AdministratorType.STATE,
+                "statefull",
+                null);
+
+        Persistence.saveOrUpdate(stateadmin);
+
+        assertEquals(AdministratorQueries.byUsername("county"),countyadmin);
+        assertEquals(AdministratorQueries.byUsername("state"), stateadmin);
+    }
+
+    @Test
+    public void testSameName() {
+
+        Administrator first = new Administrator("state",
+                Administrator.AdministratorType.STATE,
+                "first",
+                null);
+
+        Administrator second = new Administrator("state",
+                Administrator.AdministratorType.STATE,
+                "second",
+                null);
+        Persistence.saveOrUpdate(first);
+        Persistence.saveOrUpdate(second);
+
+        assertNull(AdministratorQueries.byUsername("state"));
+    }
+
+    @Test
+    public void testDBError() {
+        // Close the DB
+        Persistence.commitTransaction();
+        assertNull(AdministratorQueries.byUsername("username"));
+        Persistence.beginTransaction();
+    }
+}