diff --git a/server/eclipse-project/src/main/java/us/freeandfair/corla/controller/ComparisonAuditController.java b/server/eclipse-project/src/main/java/us/freeandfair/corla/controller/ComparisonAuditController.java index 06fb4929..ef5b2f89 100644 --- a/server/eclipse-project/src/main/java/us/freeandfair/corla/controller/ComparisonAuditController.java +++ b/server/eclipse-project/src/main/java/us/freeandfair/corla/controller/ComparisonAuditController.java @@ -232,13 +232,14 @@ public static boolean reaudit(final CountyDashboard cdb, final String comment) { LOGGER.info("[reaudit] cvr: " + cvr.toString()); - final CVRAuditInfo cai = - Persistence.getByID(cvr.id(), CVRAuditInfo.class); - final CastVoteRecord oldAcvr = cai.acvr(); - if (null == oldAcvr) { + + // DemocracyDevelopers: If this cvr has not been audited before, cai will be null. + final CVRAuditInfo cai = Persistence.getByID(cvr.id(), CVRAuditInfo.class); + if (cai == null || cai.acvr() == null) { LOGGER.error("can't reaudit a cvr that hasn't been audited"); return false; } + final CastVoteRecord oldAcvr = cai.acvr(); final Integer former_count = unaudit(cdb, cai); LOGGER.debug("[reaudit] former_count: " + former_count.toString()); diff --git a/server/eclipse-project/src/main/java/us/freeandfair/corla/query/CastVoteRecordQueries.java b/server/eclipse-project/src/main/java/us/freeandfair/corla/query/CastVoteRecordQueries.java index de877266..07c7f01b 100644 --- a/server/eclipse-project/src/main/java/us/freeandfair/corla/query/CastVoteRecordQueries.java +++ b/server/eclipse-project/src/main/java/us/freeandfair/corla/query/CastVoteRecordQueries.java @@ -577,11 +577,17 @@ public static Long maxRevision(final CastVoteRecord cvr) { q.setString("imprintedId", cvr.imprintedID()); try { - return (Long) q.getSingleResult(); + // Make sure not to return null, which causes errors later. 0L is the correct return value if + // there are no prior revisions in the database. + // Some documentation says that getSingleResult() never returns null, but it definitely does. + final Long result = (Long) q.getSingleResult(); + return result == null ? 0L : result; } 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? + // VT: Agree. Since 0L is used for "valid; no prior revisions", suggest using a different value + // here (-1?) or throwing an exception. return 0L; } diff --git a/server/eclipse-project/src/test/java/au/org/democracydevelopers/corla/endpoint/ACVRUploadTests.java b/server/eclipse-project/src/test/java/au/org/democracydevelopers/corla/endpoint/ACVRUploadTests.java index 7c0c95e4..8abc6c55 100644 --- a/server/eclipse-project/src/test/java/au/org/democracydevelopers/corla/endpoint/ACVRUploadTests.java +++ b/server/eclipse-project/src/test/java/au/org/democracydevelopers/corla/endpoint/ACVRUploadTests.java @@ -21,21 +21,23 @@ import static au.org.democracydevelopers.corla.util.testUtils.tinyIRV; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertTrue; +import static org.testng.Assert.assertFalse; import spark.Request; import javax.transaction.Transactional; import java.util.HashSet; import java.util.List; +import java.util.OptionalLong; import static us.freeandfair.corla.endpoint.Endpoint.AuthorizationType.COUNTY; import static us.freeandfair.corla.model.CastVoteRecord.RecordType.AUDITOR_ENTERED; +import static us.freeandfair.corla.model.CastVoteRecord.RecordType.REAUDITED; /** * Test upload of IRV audit cvrs. Includes tests of both valid and invalid IRV CVRs, and tests that * the interpreted ballots are properly stored in the database. * This makes use of TestClassWithAuth to mock authentication as Adams County. - * TODO test reaudits, both IRV and plurality (can they be mixed in one CVR?) * See ... */ public class ACVRUploadTests extends TestClassWithAuth { @@ -305,6 +307,71 @@ public class ACVRUploadTests extends TestClassWithAuth { " \"auditBoardIndex\": -1" + "}"; + /** + * 8 & 9. Reaudit of valid IRV vote, for CVR ID 240509; imprinted ID 1-1-1. + */ + private static final String validIRVReauditAsJson = "{" + + " \"cvr_id\": 240509," + + " \"audit_cvr\": {" + + " \"record_type\": \"AUDITOR_ENTERED\"," + + " \"county_id\": 1," + + " \"cvr_number\": 1," + + " \"sequence_number\": 1," + + " \"scanner_id\": 1," + + " \"batch_id\": \"1\"," + + " \"record_id\": 1," + + " \"imprinted_id\": \"1-1-1\"," + + " \"uri\": \"acvr:1:1-1-1\"," + + " \"ballot_type\": \"Ballot 1 - Type 1\"," + + " \"contest_info\": [" + + " {" + + " \"contest\": 240503," + + " \"comment\": \"A comment\"," + + " \"consensus\": \"YES\"," + + " \"choices\": [" + + " \"Alice(1)\"," + + " \"Chuan(2)\"" + + " ]" + + " }" + + " ]" + + " }," + + " \"reaudit\": true," + + " \"comment\": \"\"," + + " \"auditBoardIndex\": -1" + + "}"; + + /** + * 10. Attempt to reaudit a vote that has not yet been audited - #7, the IRV vote with typos. + * Should cause an error. + */ + private static final String IRVReauditNoPriorUpload = "{" + + " \"cvr_id\": 240515," + + " \"audit_cvr\": {" + + " \"record_type\": \"AUDITOR_ENTERED\"," + + " \"county_id\": 1," + + " \"cvr_number\": 7," + + " \"sequence_number\": 1," + + " \"scanner_id\": 1," + + " \"batch_id\": \"1\"," + + " \"record_id\": 7," + + " \"imprinted_id\": \"1-1-7\"," + + " \"uri\": \"acvr:1:1-1-7\"," + + " \"ballot_type\": \"Ballot 1 - Type 1\"," + + " \"contest_info\": [" + + " {" + + " \"contest\": 240503," + + " \"comment\": \"A comment\"," + + " \"consensus\": \"YES\"," + + " \"choices\": [" + + " ]" + + " }" + + " ]" + + " }," + + " \"reaudit\": true," + + " \"comment\": \"\"," + + " \"auditBoardIndex\": -1" + + "}"; + /** * Database init. */ @@ -338,10 +405,13 @@ public void initMocks() { * 3. a blank IRV vote, * 4. a vote with non-IRV choices ("Alice" instead of "Alice(1)"), * 5. a vote with invalid choices (names not in the list of choices for the contest), - * 6. a vote that doesn't properly correspond to the IDs it should have, and - * 7. an unparseable vote (typos in json data). - * We check that it is accepted and that the right records for CVR and CVRContestInfo are - * stored in the database. + * 6. a vote that doesn't properly correspond to the IDs it should have, + * 7. an unparseable vote (typos in json data), + * 8. a reaudit (which checks that the superseded upload is marked REAUDITED), and + * 9. a second reaudit with the same data (which checks that the previous reaudit is also now + * marked REAUDITED and that there are now two revisions). + * We check that it is accepted (or rejected if it should be) and that the right records for CVR + * and CVRContestInfo are stored in the database. */ @Test @Transactional @@ -386,19 +456,37 @@ void testACVRUploadAndStorage() { // Expected error messages for malformed upload cvrs. String malformedACVRMsg = "malformed audit CVR upload"; - testErrorResponse(240512L, pluralityIRVAsJson, malformedACVRMsg); + testErrorResponseAndNoMatchingCvr(240512L, pluralityIRVAsJson, malformedACVRMsg); // Fifth test: upload a vote with IRV choices that are not among the valid candidates. This // should cause an error. - testErrorResponse(240513L, wrongCandidateNamesIRVAsJson, malformedACVRMsg); + testErrorResponseAndNoMatchingCvr(240513L, wrongCandidateNamesIRVAsJson, malformedACVRMsg); // Sixth test: upload a vote with IDs that do not correspond properly to the expected CVR. // This should cause an error. - testErrorResponse(240514L, IRVWithInconsistentIDsAsJson, malformedACVRMsg); + testErrorResponseAndNoMatchingCvr(240514L, IRVWithInconsistentIDsAsJson, malformedACVRMsg); // Seventh test: upload a vote that has typos preventing json deserialization. This should // cause an error. - testErrorResponse(240515L, IRVJsonDeserializationFail, malformedACVRMsg); + testErrorResponseAndNoMatchingCvr(240515L, IRVJsonDeserializationFail, malformedACVRMsg); + + // Eighth test: upload a reaudited ballot. + // Check that the new data successfully replaces the prior upload. + testSuccessResponse(240509L, "1-1-1", validIRVReauditAsJson, List.of("Alice","Chuan"), 3); + // Test that the previous upload, with a vote for Bob and Chuan (validIRVAsJson) is now tagged + // as REAUDITED. + testPreviousAreReaudited(240509L, "1-1-1", List.of("Bob","Chuan"), 1, 1); + + // Ninth test: upload the same ballot being reaudited, again. + // Check that the new data is identical (it replaces it, but it's the same). + testSuccessResponse(240509L, "1-1-1", validIRVReauditAsJson, List.of("Alice","Chuan"), 3); + // Test that the previous upload (from test 8), with a vote for Alice and Chuan + // (validIRVReauditAsJson) is now tagged as REAUDITED. + // There should be two reaudited ballots now, and the max revision should be 2. + testPreviousAreReaudited(240509L, "1-1-1", List.of("Alice","Chuan"), 2, 2); + + // Tenth test: try to "re-"audit something that has not previously been successfully audited. Should throw an error. + testErrorResponseAndNoMatchingCvr(240515L, IRVReauditNoPriorUpload, "CVR has not previously been audited"); } } @@ -427,14 +515,14 @@ private void testIRVBallotInterpretations(final long CvrNum, final String imprin /** * Test expected consequences of successful ACVR upload. - * @param CvrId The CVR number (last number of the imprinted ID). + * @param CvrId The CVR ID (of the original UPLOADED CVR). * @param expectedImprintedId The imprinted id, scanner-batch-record. * @param CvrAsJson The upload cvr, as a json string. * @param expectedInterpretedChoices The expected valid interpretation, which should be stored. * @param expectedACVRs The number of audit CVRs expected in total. */ private void testSuccessResponse(final long CvrId, final String expectedImprintedId, final String CvrAsJson, - final List expectedInterpretedChoices, final int expectedACVRs) { + final List expectedInterpretedChoices, final int expectedACVRs) { final Request request = new SparkRequestStub(CvrAsJson, new HashSet<>()); uploadEndpoint.endpointBody(request, response); @@ -443,7 +531,7 @@ private void testSuccessResponse(final long CvrId, final String expectedImprinte AUDITOR_ENTERED).toList(); assertEquals(acvrs.size(), expectedACVRs); - // There should now be an ACVR with matching cvrId. + // There should now be an AUDITOR_ENTERED ACVR with matching cvrId. final CastVoteRecord acvr = acvrs.stream().filter(a -> a.getCvrId() == CvrId).findFirst().orElseThrow(); assertEquals(acvr.recordType(), AUDITOR_ENTERED); // Check that we have the right record: CvrId and Imprinted ID should match. @@ -456,6 +544,43 @@ private void testSuccessResponse(final long CvrId, final String expectedImprinte assertTrue(testUtils.equalStringLists(choices, expectedInterpretedChoices)); } + /** + * Check that some previously-uploaded ballot has been tagged as "REAUDITED". This happens when a new ballot with the + * same CvrId is uploaded with "reaudit = true". The idea is that the most recent REAUDIT is always simply + * of type AUDITOR_ENTERED, but all _previously_ submitted versions of the same ballot are tagged as REAUDITED - these + * are then omitted from discrepancy and risk calculations, because only the most recent upload counts. + * @param CvrId The CVR ID (of the original UPLOADED CVR). + * @param expectedImprintedId The imprinted id, scanner-batch-record. + * @param expectedInterpretedChoices The expected valid interpretation, which should be stored. + * @param expectedReauditedBallots The number of expected REAUDITED ballots with this CvrId. + * @param expectedMaxRevision The revision of the old audit cvr (null for the first audit ballot; 1L for the + * first reaudit, then incrementing by 1 for subsequent reaudits. + */ + private void testPreviousAreReaudited(final long CvrId, final String expectedImprintedId, + final List expectedInterpretedChoices, final long expectedReauditedBallots, final long expectedMaxRevision) { + + // Check for the expected number of Reaudited ballots of the required CvrId. + final List acvrs = CastVoteRecordQueries.getMatching(1L, REAUDITED).filter(a -> a.getCvrId() == CvrId).toList(); + assertEquals(acvrs.size(), expectedReauditedBallots); + + // Check that we have the right records: Imprinted ID should match. + assertTrue(acvrs.stream().allMatch(cvr -> cvr.imprintedID().equals(expectedImprintedId))); + + // Find the maximum revision; check that it matches the expected maximum revision (this should match the number of + // reaudits). + assertFalse(acvrs.isEmpty()); + final OptionalLong maxRevision = acvrs.stream().mapToLong(CastVoteRecord::getRevision).max(); + assertEquals(maxRevision, OptionalLong.of(expectedMaxRevision)); + + // Check that there is one record with the expected revision and it has the expected vote choices. + final List maxRevisionAcvrs + = acvrs.stream().filter(a -> a.getRevision() == expectedMaxRevision).toList(); + assertEquals(maxRevisionAcvrs.size(), 1); + assertTrue(maxRevisionAcvrs.get(0).contestInfoForContestResult(tinyIRV).isPresent()); + final List choices = maxRevisionAcvrs.get(0).contestInfoForContestResult(tinyIRV).get().choices(); + assertTrue(testUtils.equalStringLists(choices, expectedInterpretedChoices)); + } + /** * Test that submitting the given ACVR to the endpoint produces the given error, and that there are no corresponding * ACVRs stored in the database afterward. @@ -463,7 +588,7 @@ private void testSuccessResponse(final long CvrId, final String expectedImprinte * @param CvrAsJson The upload cvr, as a json string. * @param expectedError The expected error message. */ - private void testErrorResponse(final long CvrId, final String CvrAsJson, final String expectedError) { + private void testErrorResponseAndNoMatchingCvr(final long CvrId, final String CvrAsJson, final String expectedError) { final Request request = new SparkRequestStub(CvrAsJson, new HashSet<>()); String errorBody = "";