From fba74d764e396c01ffc260aac7c47e11209d8b13 Mon Sep 17 00:00:00 2001 From: Armin Samii Date: Fri, 14 Jun 2024 14:55:02 -0400 Subject: [PATCH 1/5] output folder DNE fix and audit log addition --- .../brightspots/rcv/TabulatorSession.java | 50 +++++++++++-------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/src/main/java/network/brightspots/rcv/TabulatorSession.java b/src/main/java/network/brightspots/rcv/TabulatorSession.java index 6eb46c7ad..2bdac894d 100644 --- a/src/main/java/network/brightspots/rcv/TabulatorSession.java +++ b/src/main/java/network/brightspots/rcv/TabulatorSession.java @@ -101,11 +101,11 @@ boolean convertToCdf() { checkConfigVersionMatchesApp(config); boolean conversionSuccess = false; - if (setUpLogging(config.getOutputDirectory()) && config.validate().isEmpty()) { + if (setUpLogging(config.getOutputDirectory(), "to-cdf") && config.validate().isEmpty()) { Logger.info("Converting CVR(s) to CDF..."); try { FileUtils.createOutputDirectory(config.getOutputDirectory()); - LoadedCvrData castVoteRecords = parseCastVoteRecords(config); + LoadedCvrData castVoteRecords = parseCastVoteRecords(config, false); Tabulator.SliceIdSet sliceIds = new Tabulator(castVoteRecords.getCvrs(), config).getEnabledSliceIds(); ResultsWriter writer = @@ -139,7 +139,15 @@ boolean convertToCdf() { LoadedCvrData parseAndCountCastVoteRecords() throws CastVoteRecordGenericParseException { ContestConfig config = ContestConfig.loadContestConfig(configPath); - return parseCastVoteRecords(config); + boolean setUpLoggingSuccess = setUpLogging(config.getOutputDirectory(), "check-ballot-counts"); + if (!setUpLoggingSuccess) { + Logger.severe("Failed to configure logger!"); + throw new CastVoteRecordGenericParseException(); + } + + LoadedCvrData castVoteRecords = parseCastVoteRecords(config, false); + Logger.removeTabulationFileLogging(); + return castVoteRecords; } // Returns a List of exception class names that were thrown while tabulating. @@ -153,7 +161,7 @@ List tabulate(String operatorName, LoadedCvrData expectedCvrData) { ContestConfig config = ContestConfig.loadContestConfig(configPath); checkConfigVersionMatchesApp(config); boolean tabulationSuccess = false; - boolean setUpLoggingSuccess = setUpLogging(config.getOutputDirectory()); + boolean setUpLoggingSuccess = setUpLogging(config.getOutputDirectory(), "tabulate"); if (operatorName == null || operatorName.isBlank()) { Logger.severe("Operator name is required for the audit logs!"); @@ -191,7 +199,7 @@ List tabulate(String operatorName, LoadedCvrData expectedCvrData) { // Read cast vote records and slice IDs from CVR files Set newWinnerSet; try { - LoadedCvrData castVoteRecords = parseCastVoteRecords(config); + LoadedCvrData castVoteRecords = parseCastVoteRecords(config, true); if (config.getSequentialWinners().isEmpty() && !castVoteRecords.metadataMatches(expectedCvrData)) { Logger.severe("CVR data has changed between loading the CVRs and reading them!"); @@ -228,7 +236,7 @@ List tabulate(String operatorName, LoadedCvrData expectedCvrData) { // normal operation (not multi-pass IRV, a.k.a. sequential multi-seat) // Read cast vote records and precinct IDs from CVR files try { - LoadedCvrData castVoteRecords = parseCastVoteRecords(config); + LoadedCvrData castVoteRecords = parseCastVoteRecords(config, true); if (!castVoteRecords.metadataMatches(expectedCvrData)) { Logger.severe("CVR data has changed between loading the CVRs and reading them!"); exceptionsEncountered.add(TabulationAbortedException.class.toString()); @@ -259,20 +267,20 @@ List tabulate(String operatorName) { Set loadSliceNamesFromCvrs(ContestConfig.TabulateBySlice slice, ContestConfig config) { try { - List castVoteRecords = parseCastVoteRecords(config).getCvrs(); + List castVoteRecords = parseCastVoteRecords(config, false).getCvrs(); return new Tabulator(castVoteRecords, config).getEnabledSliceIds().get(slice); } catch (TabulationAbortedException | CastVoteRecordGenericParseException e) { throw new RuntimeException(e); } } - private boolean setUpLogging(String outputDirectory) { + private boolean setUpLogging(String outputDirectory, String prefix) { boolean success = false; // cache outputPath for testing outputPath = outputDirectory; try { FileUtils.createOutputDirectory(outputDirectory); - Logger.addTabulationFileLogging(outputDirectory, timestampString); + Logger.addTabulationFileLogging(outputDirectory, timestampString + "_" + prefix); success = true; } catch (UnableToCreateDirectoryException | IOException exception) { Logger.severe("Failed to configure tabulation logger!\n%s", exception); @@ -302,7 +310,7 @@ private Set runTabulationForConfig( // parse CVR files referenced in the ContestConfig object into a list of CastVoteRecords // param: config object containing CVR file paths to parse // returns: list of parsed CVRs or null if an error was encountered - private LoadedCvrData parseCastVoteRecords(ContestConfig config) + private LoadedCvrData parseCastVoteRecords(ContestConfig config, boolean outputGenericCvr) throws CastVoteRecordGenericParseException { Logger.info("Parsing cast vote records..."); List castVoteRecords = new ArrayList<>(); @@ -373,16 +381,18 @@ private LoadedCvrData parseCastVoteRecords(ContestConfig config) } // Output the RCTab-CSV CVR - try { - ResultsWriter writer = - new ResultsWriter().setContestConfig(config).setTimestampString(timestampString); - this.convertedFilePath = - writer.writeRctabCvrCsv( - castVoteRecords, - perSourceDataForCsv, - config.getOutputDirectory()); - } catch (IOException exception) { - // error already logged in ResultsWriter + if (outputGenericCvr) { + try { + ResultsWriter writer = + new ResultsWriter().setContestConfig(config).setTimestampString(timestampString); + this.convertedFilePath = + writer.writeRctabCvrCsv( + castVoteRecords, + perSourceDataForCsv, + config.getOutputDirectory()); + } catch (IOException exception) { + // error already logged in ResultsWriter + } } if (encounteredSourceProblem) { From 2be886e2302f5aae84f24d8ec49ee47cafca39dc Mon Sep 17 00:00:00 2001 From: Armin Samii Date: Fri, 14 Jun 2024 21:15:28 -0400 Subject: [PATCH 2/5] fix broken test --- .../more_winners_than_candidates_config.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/resources/network/brightspots/rcv/test_data/more_winners_than_candidates/more_winners_than_candidates_config.json b/src/test/resources/network/brightspots/rcv/test_data/more_winners_than_candidates/more_winners_than_candidates_config.json index 2c29d7163..03ea6e58c 100644 --- a/src/test/resources/network/brightspots/rcv/test_data/more_winners_than_candidates/more_winners_than_candidates_config.json +++ b/src/test/resources/network/brightspots/rcv/test_data/more_winners_than_candidates/more_winners_than_candidates_config.json @@ -15,7 +15,7 @@ "firstVoteColumnIndex" : "2", "firstVoteRowIndex" : "2", "idColumnIndex" : "1", - "precinctColumnIndex" : "2", + "precinctColumnIndex" : "", "overvoteDelimiter" : "", "provider" : "ess", "overvoteLabel" : "overvote", From 4e25c2ef68da5ebe884f52b33fe2e4ff6a499a2d Mon Sep 17 00:00:00 2001 From: Armin Samii Date: Fri, 14 Jun 2024 21:35:55 -0400 Subject: [PATCH 3/5] fix compilation error --- src/main/java/network/brightspots/rcv/TabulatorSession.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/network/brightspots/rcv/TabulatorSession.java b/src/main/java/network/brightspots/rcv/TabulatorSession.java index 1ca99c8a9..e0b55f604 100644 --- a/src/main/java/network/brightspots/rcv/TabulatorSession.java +++ b/src/main/java/network/brightspots/rcv/TabulatorSession.java @@ -158,10 +158,10 @@ LoadedCvrData parseAndCountCastVoteRecords(BiConsumer progressUp throw new CastVoteRecordGenericParseException(); } - LoadedCvrData castVoteRecords = parseCastVoteRecords(config, false); - Logger.removeTabulationFileLogging(); Progress progress = new Progress(config, 0, progressUpdate); - return parseCastVoteRecords(config, progress); + LoadedCvrData castVoteRecords = parseCastVoteRecords(config, progress, false); + Logger.removeTabulationFileLogging(); + return castVoteRecords; } // Returns a List of exception class names that were thrown while tabulating. From f80c8a12e96f8f27ec5a9d96b9b15cfad89298a4 Mon Sep 17 00:00:00 2001 From: HEdingfield Date: Sat, 15 Jun 2024 09:39:14 -0700 Subject: [PATCH 4/5] Improve code clarity; minor refactoring. --- .../brightspots/rcv/ResultsWriter.java | 2 +- .../brightspots/rcv/TabulatorSession.java | 100 +++++++++--------- 2 files changed, 51 insertions(+), 51 deletions(-) diff --git a/src/main/java/network/brightspots/rcv/ResultsWriter.java b/src/main/java/network/brightspots/rcv/ResultsWriter.java index cec587b5b..9c7386597 100644 --- a/src/main/java/network/brightspots/rcv/ResultsWriter.java +++ b/src/main/java/network/brightspots/rcv/ResultsWriter.java @@ -554,7 +554,7 @@ void generateOverallSummaryFiles( // Note that the castVoteRecords list MUST be stable, as cvrSourceData // relies on its exact ordering to determine which source each record came from. // Returns the filepath written - String writeRctabCvrCsv( + String writeRcTabCvrCsv( List castVoteRecords, List cvrSourceData, String csvOutputFolder) diff --git a/src/main/java/network/brightspots/rcv/TabulatorSession.java b/src/main/java/network/brightspots/rcv/TabulatorSession.java index e0b55f604..5ebf6fa53 100644 --- a/src/main/java/network/brightspots/rcv/TabulatorSession.java +++ b/src/main/java/network/brightspots/rcv/TabulatorSession.java @@ -34,7 +34,6 @@ import java.util.Map; import java.util.Set; import java.util.function.BiConsumer; -import javafx.util.Pair; import network.brightspots.rcv.CastVoteRecord.CvrParseException; import network.brightspots.rcv.ContestConfig.Provider; import network.brightspots.rcv.ContestConfig.UnrecognizedProviderException; @@ -105,7 +104,8 @@ boolean convertToCdf(BiConsumer progressUpdate) { Progress progress = new Progress(config, 0, progressUpdate); - if (setUpLogging(config.getOutputDirectory(), "to-cdf") && config.validate().isEmpty()) { + if (setUpLogging(config.getOutputDirectory(), "convert-to-cdf") + && config.validate().isEmpty()) { Logger.info("Converting CVR(s) to CDF..."); try { FileUtils.createOutputDirectory(config.getOutputDirectory()); @@ -145,8 +145,8 @@ boolean convertToCdf(BiConsumer progressUpdate) { return conversionSuccess; } - boolean convertToCdf() { - return convertToCdf(null); + void convertToCdf() { + convertToCdf(null); } LoadedCvrData parseAndCountCastVoteRecords(BiConsumer progressUpdate) @@ -169,8 +169,10 @@ LoadedCvrData parseAndCountCastVoteRecords(BiConsumer progressUp // Note: An exception MUST be returned any time tabulation does not run. // In general, that means any Logger.severe in this function should be accompanied // by an exceptionsEncountered.add(...) call. - List tabulate(String operatorName, LoadedCvrData expectedCvrData, - BiConsumer progressUpdate) { + List tabulate( + String operatorName, + LoadedCvrData expectedCvrData, + BiConsumer progressUpdate) { Logger.info("Starting tabulation session..."); List exceptionsEncountered = new LinkedList<>(); ContestConfig config = ContestConfig.loadContestConfig(configPath); @@ -218,7 +220,7 @@ List tabulate(String operatorName, LoadedCvrData expectedCvrData, try { LoadedCvrData castVoteRecords = parseCastVoteRecords(config, progress, true); if (config.getSequentialWinners().isEmpty() - && !castVoteRecords.metadataMatches(expectedCvrData)) { + && !castVoteRecords.metadataMatches(expectedCvrData)) { Logger.severe("CVR data has changed between loading the CVRs and reading them!"); exceptionsEncountered.add(TabulationAbortedException.class.toString()); break; @@ -286,21 +288,21 @@ List tabulate(String operatorName) { Set loadSliceNamesFromCvrs(ContestConfig.TabulateBySlice slice, ContestConfig config) { Progress progress = new Progress(config, 0, null); try { - List castVoteRecords = parseCastVoteRecords( - config, progress, false).getCvrs(); + List castVoteRecords = + parseCastVoteRecords(config, progress, false).getCvrs(); return new Tabulator(castVoteRecords, config).getEnabledSliceIds().get(slice); } catch (TabulationAbortedException | CastVoteRecordGenericParseException e) { throw new RuntimeException(e); } } - private boolean setUpLogging(String outputDirectory, String prefix) { + private boolean setUpLogging(String outputDirectory, String filenamePrefix) { boolean success = false; // cache outputPath for testing outputPath = outputDirectory; try { FileUtils.createOutputDirectory(outputDirectory); - Logger.addTabulationFileLogging(outputDirectory, timestampString + "_" + prefix); + Logger.addTabulationFileLogging(outputDirectory, timestampString + "_" + filenamePrefix); success = true; } catch (UnableToCreateDirectoryException | IOException exception) { Logger.severe("Failed to configure tabulation logger!\n%s", exception); @@ -327,11 +329,18 @@ private Set runTabulationForConfig( return winners; } - // parse CVR files referenced in the ContestConfig object into a list of CastVoteRecords - // param: config object containing CVR file paths to parse - // returns: list of parsed CVRs or null if an error was encountered - private LoadedCvrData parseCastVoteRecords(ContestConfig config, Progress progress, - boolean outputGenericCvr) throws CastVoteRecordGenericParseException { + /** + * Parse CVR files referenced in the ContestConfig object into a list of CastVoteRecords. + * + * @param config Object containing CVR file paths to parse. + * @param progress Object tracking progress of parsing the CVRs. + * @param shouldOutputRcTabCvr Whether to output the simplified RCTab CVR CSV file. + * @return List of parsed CVRs or null if an error was encountered. + * @throws CastVoteRecordGenericParseException If any failure occurs when parsing CVRs. + */ + private LoadedCvrData parseCastVoteRecords( + ContestConfig config, Progress progress, boolean shouldOutputRcTabCvr) + throws CastVoteRecordGenericParseException { Logger.info("Parsing cast vote records..."); List castVoteRecords = new ArrayList<>(); boolean encounteredSourceProblem = false; @@ -341,7 +350,7 @@ private LoadedCvrData parseCastVoteRecords(ContestConfig config, Progress progre // At each iteration of the following loop, we add records from another source file. for (int sourceIndex = 0; sourceIndex < config.rawConfig.cvrFileSources.size(); ++sourceIndex) { - RawContestConfig.CvrSource source = config.rawConfig.cvrFileSources.get(sourceIndex); + RawContestConfig.CvrSource source = config.rawConfig.cvrFileSources.get(sourceIndex); String cvrPath = config.resolveConfigPath(source.getFilePath()); Provider provider = ContestConfig.getProvider(source); try { @@ -351,12 +360,9 @@ private LoadedCvrData parseCastVoteRecords(ContestConfig config, Progress progre reader.readCastVoteRecords(castVoteRecords); // Update the per-source data for the results writer - cvrSourceData.add(new ResultsWriter.CvrSourceData( - source, - reader, - sourceIndex, - startIndex, - castVoteRecords.size() - 1)); + cvrSourceData.add( + new ResultsWriter.CvrSourceData( + source, reader, sourceIndex, startIndex, castVoteRecords.size() - 1)); // Check for unrecognized candidates Map unrecognizedCandidateCounts = @@ -405,16 +411,13 @@ private LoadedCvrData parseCastVoteRecords(ContestConfig config, Progress progre progress.markFileRead(); } - // Output the RCTab-CSV CVR - if (outputGenericCvr) { + // Output the simplified RCTab CVR CSV file + if (shouldOutputRcTabCvr) { try { ResultsWriter writer = - new ResultsWriter().setContestConfig(config).setTimestampString(timestampString); + new ResultsWriter().setContestConfig(config).setTimestampString(timestampString); this.convertedFilePath = - writer.writeRctabCvrCsv( - castVoteRecords, - cvrSourceData, - config.getOutputDirectory()); + writer.writeRcTabCvrCsv(castVoteRecords, cvrSourceData, config.getOutputDirectory()); } catch (IOException exception) { // error already logged in ResultsWriter } @@ -447,14 +450,13 @@ static class UnrecognizedCandidatesException extends Exception { } } - static class CastVoteRecordGenericParseException extends Exception { - } + static class CastVoteRecordGenericParseException extends Exception {} /** - * A summary of the cast vote records that have been read. - * Manages CVR in memory, so you can retain metadata about the loaded CVRs without - * keeping them all in memory. Use .discard() to free up memory. After memory is freed, - * all other operations except for getCvrs() are still valid. + * A summary of the cast vote records that have been read. Manages CVR in memory, so you can + * retain metadata about the loaded CVRs without keeping them all in memory. Use .discard() to + * free up memory. After memory is freed, all other operations except for getCvrs() are still + * valid. */ public static class LoadedCvrData { public static final LoadedCvrData MATCHES_ALL = new LoadedCvrData(); @@ -462,12 +464,11 @@ public static class LoadedCvrData { private List cvrs; private final int numCvrs; - private List cvrSourcesData; + private final List cvrSourcesData; private boolean isDiscarded; private final boolean doesMatchAllMetadata; - public LoadedCvrData(List cvrs, - List cvrSourcesData) { + LoadedCvrData(List cvrs, List cvrSourcesData) { this.cvrs = cvrs; this.successfullyReadAll = cvrs != null; this.numCvrs = cvrs != null ? cvrs.size() : 0; @@ -477,8 +478,8 @@ public LoadedCvrData(List cvrs, } /** - * This constructor will cause metadataMatches to always return true, - * and contains no true statistics. + * This constructor will cause metadataMatches to always return true, and contains no true + * statistics. */ private LoadedCvrData() { this.cvrs = null; @@ -490,23 +491,23 @@ private LoadedCvrData() { } /** - * Currently only checks if the number of CVRs matches, but can be extended to ensure - * exact matches or meet other needs. + * Currently only checks if the number of CVRs matches, but can be extended to ensure exact + * matches or meet other needs. * * @param other The loaded CVRs to compare against * @return whether the metadata matches */ public boolean metadataMatches(LoadedCvrData other) { return other.doesMatchAllMetadata - || this.doesMatchAllMetadata - || other.numCvrs() == this.numCvrs(); + || this.doesMatchAllMetadata + || other.numCvrs() == this.numCvrs(); } public int numCvrs() { return numCvrs; } - public List getCvrSourcesData() { + List getCvrSourcesData() { return cvrSourcesData; } @@ -515,7 +516,7 @@ public void discard() { isDiscarded = true; } - public List getCvrs() { + List getCvrs() { if (isDiscarded) { throw new IllegalStateException("CVRs have been discarded from memory."); } @@ -525,11 +526,10 @@ public List getCvrs() { public void printSummary() { Logger.info("Cast Vote Record summary:"); for (ResultsWriter.CvrSourceData sourceData : cvrSourcesData) { - Logger.info("Source %d: %s", - sourceData.sourceIndex + 1, sourceData.source.getFilePath()); + Logger.info("Source %d: %s", sourceData.sourceIndex + 1, sourceData.source.getFilePath()); Logger.info(" uses provider: %s", sourceData.source.getProvider()); Logger.info(" read %d cast vote records", sourceData.getNumCvrs()); } } } -} \ No newline at end of file +} From 567287ffd92fa181e69f599b255362f7faf7ce43 Mon Sep 17 00:00:00 2001 From: Armin Samii Date: Fri, 21 Jun 2024 12:42:12 -0400 Subject: [PATCH 5/5] lint --- .../java/network/brightspots/rcv/TabulatorSession.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/network/brightspots/rcv/TabulatorSession.java b/src/main/java/network/brightspots/rcv/TabulatorSession.java index b313145ff..475f20df2 100644 --- a/src/main/java/network/brightspots/rcv/TabulatorSession.java +++ b/src/main/java/network/brightspots/rcv/TabulatorSession.java @@ -417,12 +417,12 @@ private LoadedCvrData parseCastVoteRecords( if (shouldOutputRcTabCvr) { try { ResultsWriter writer = - new ResultsWriter().setContestConfig(config).setTimestampString(timestampString); + new ResultsWriter().setContestConfig(config).setTimestampString(timestampString); this.convertedFilePath = - writer.writeRcTabCvrCsv( - castVoteRecords, - cvrSourceData, - config.getOutputDirectory()); + writer.writeRcTabCvrCsv( + castVoteRecords, + cvrSourceData, + config.getOutputDirectory()); } catch (IOException exception) { // error already logged in ResultsWriter }