From 1f1f9d44292c62bcb0464df57e50674a5c49a910 Mon Sep 17 00:00:00 2001 From: Armin Samii Date: Mon, 29 Apr 2024 15:12:15 -0400 Subject: [PATCH] don't allow change in ballot stats between loading and tabulating --- .../brightspots/rcv/GuiConfigController.java | 34 ++++-- .../rcv/GuiTabulateController.java | 17 ++- .../java/network/brightspots/rcv/Main.java | 2 +- .../brightspots/rcv/TabulatorSession.java | 105 +++++++++++++++--- .../brightspots/rcv/GuiTabulationPopup.fxml | 2 +- .../brightspots/rcv/TabulatorTests.java | 6 +- 6 files changed, 130 insertions(+), 36 deletions(-) diff --git a/src/main/java/network/brightspots/rcv/GuiConfigController.java b/src/main/java/network/brightspots/rcv/GuiConfigController.java index 9d2d9ad8b..613d0b5ac 100644 --- a/src/main/java/network/brightspots/rcv/GuiConfigController.java +++ b/src/main/java/network/brightspots/rcv/GuiConfigController.java @@ -102,6 +102,7 @@ import network.brightspots.rcv.Tabulator.OvervoteRule; import network.brightspots.rcv.Tabulator.TiebreakMode; import network.brightspots.rcv.Tabulator.WinnerElectionMode; +import network.brightspots.rcv.TabulatorSession.LoadedCvrData; /** * View controller for config layout. @@ -508,9 +509,9 @@ public void menuItemTabulateClicked() { * Tabulate whatever is currently entered into the GUI. * Assumes GuiTabulateController checked all prerequisites. */ - public Service parseAndCountCastVoteRecords(String configPath) { + public Service parseAndCountCastVoteRecords(String configPath) { setGuiIsBusy(true); - Service service = new ReadCastVoteRecordsService(configPath); + Service service = new ReadCastVoteRecordsService(configPath); setUpAndStartService(service); return service; } @@ -520,10 +521,13 @@ public Service parseAndCountCastVoteRecords(String configPath) { * Assumes GuiTabulateController checked all prerequisites. */ public Service startTabulation( - String configPath, String operatorName, boolean deleteConfigOnCompletion) { + String configPath, + String operatorName, + boolean deleteConfigOnCompletion, + LoadedCvrData expectedCvrData) { setGuiIsBusy(true); TabulatorService service = new TabulatorService( - configPath, operatorName, deleteConfigOnCompletion); + configPath, operatorName, deleteConfigOnCompletion, expectedCvrData); setUpAndStartService(service); return service; } @@ -1754,10 +1758,16 @@ protected void setUpTaskCompletionTriggers(Task task, String failureMes // TabulatorService runs a tabulation in the background private static class TabulatorService extends ConfigReaderService { private final String operatorName; + private LoadedCvrData expectedCvrStatistics; - TabulatorService(String configPath, String operatorName, boolean deleteConfigOnCompletion) { + TabulatorService( + String configPath, + String operatorName, + boolean deleteConfigOnCompletion, + LoadedCvrData expectedCvrStatistics) { super(configPath, deleteConfigOnCompletion); this.operatorName = operatorName; + this.expectedCvrStatistics = expectedCvrStatistics; } @Override @@ -1767,7 +1777,7 @@ protected Task createTask() { @Override protected Boolean call() { TabulatorSession session = new TabulatorSession(configPath); - List errors = session.tabulate(operatorName); + List errors = session.tabulate(operatorName, expectedCvrStatistics); if (errors.isEmpty()) { succeeded(); } else { @@ -1806,7 +1816,7 @@ protected Boolean call() { } // ReadCastVoteRecordsService reads CVRs - private static class ReadCastVoteRecordsService extends Service { + private static class ReadCastVoteRecordsService extends Service { private final String configPath; ReadCastVoteRecordsService(String configPath) { @@ -1814,19 +1824,19 @@ private static class ReadCastVoteRecordsService extends Service { } @Override - protected Task createTask() { + protected Task createTask() { return new Task<>() { @Override - protected Integer call() { + protected LoadedCvrData call() { TabulatorSession session = new TabulatorSession(configPath); - int count = session.parseAndCountCastVoteRecords(); - if (count >= 0) { + LoadedCvrData cvrStatics = session.parseAndCountCastVoteRecords(); + if (cvrStatics.successfullyReadAll) { succeeded(); } else { Logger.warning("There were errors"); failed(); } - return count; + return cvrStatics; } }; } diff --git a/src/main/java/network/brightspots/rcv/GuiTabulateController.java b/src/main/java/network/brightspots/rcv/GuiTabulateController.java index 0639e1b42..ffe8dbd3c 100644 --- a/src/main/java/network/brightspots/rcv/GuiTabulateController.java +++ b/src/main/java/network/brightspots/rcv/GuiTabulateController.java @@ -32,6 +32,7 @@ import javafx.scene.text.Text; import javafx.stage.Stage; import javafx.stage.Window; +import network.brightspots.rcv.TabulatorSession.LoadedCvrData; /** View controller for tabulator layout. */ @SuppressWarnings("WeakerAccess") @@ -86,6 +87,11 @@ public class GuiTabulateController { */ private boolean lastTaskFailed = false; + /** + * Last-loaded CVR metadata, with the CVRs themselves discarded from memory. + */ + private LoadedCvrData lastLoadedCvrData; + @FXML private TextArea filepath; @FXML private Button saveButton; @FXML private Button tempSaveButton; @@ -144,7 +150,7 @@ public void buttonloadCvrsClicked(ActionEvent actionEvent) { } enableButtonsUpTo(null); - Service service = guiConfigController.parseAndCountCastVoteRecords(configPath); + Service service = guiConfigController.parseAndCountCastVoteRecords(configPath); // Dispatch a function that watches the service and updates the progress bar watchParseCvrServiceProgress(service); } @@ -163,7 +169,7 @@ public void buttonTabulateClicked(ActionEvent actionEvent) { } Service service = guiConfigController.startTabulation( - configPath, userNameField.getText(), useTemporaryConfigBeforeTabulation); + configPath, userNameField.getText(), useTemporaryConfigBeforeTabulation, lastLoadedCvrData); // Dispatch a function that watches the service and updates the progress bar watchTabulatorServiceProgress(service); } @@ -196,11 +202,14 @@ private void watchTabulatorServiceProgress(Service service) { watchGenericService(service, onSuceededEvent); } - private void watchParseCvrServiceProgress(Service service) { + private void watchParseCvrServiceProgress(Service service) { EventHandler onSuceededEvent = workerStateEvent -> { enableButtonsUpTo(tabulateButton); - numberOfBallots.setText("Number of Ballots: " + service.getValue()); + LoadedCvrData data = service.getValue(); + numberOfBallots.setText("Number of Ballots: " + data.numCvrs()); numberOfBallots.setOpacity(1); + data.discard(); + lastLoadedCvrData = data; }; watchGenericService(service, onSuceededEvent); diff --git a/src/main/java/network/brightspots/rcv/Main.java b/src/main/java/network/brightspots/rcv/Main.java index 4a3164c22..109c4fe6d 100644 --- a/src/main/java/network/brightspots/rcv/Main.java +++ b/src/main/java/network/brightspots/rcv/Main.java @@ -83,7 +83,7 @@ public static void main(String[] args) { session.convertToCdf(); } else { operatorName = operatorName.trim(); - session.tabulate(operatorName); + session.tabulate(operatorName, TabulatorSession.LoadedCvrData.MatchesAnything); } } diff --git a/src/main/java/network/brightspots/rcv/TabulatorSession.java b/src/main/java/network/brightspots/rcv/TabulatorSession.java index 3a56cd6b3..303afb571 100644 --- a/src/main/java/network/brightspots/rcv/TabulatorSession.java +++ b/src/main/java/network/brightspots/rcv/TabulatorSession.java @@ -105,18 +105,19 @@ boolean convertToCdf() { Logger.info("Converting CVR(s) to CDF..."); try { FileUtils.createOutputDirectory(config.getOutputDirectory()); - List castVoteRecords = parseCastVoteRecords(config); - if (castVoteRecords == null) { + LoadedCvrData castVoteRecords = parseCastVoteRecords(config); + if (!castVoteRecords.successfullyReadAll) { Logger.severe("Aborting conversion due to cast vote record errors!"); } else { - Set precinctIds = new Tabulator(castVoteRecords, config).getPrecinctIds(); + Set precinctIds = new Tabulator( + castVoteRecords.getCvrs(), config).getPrecinctIds(); ResultsWriter writer = new ResultsWriter() .setNumRounds(0) .setPrecinctIds(precinctIds) .setContestConfig(config) .setTimestampString(timestampString); - writer.generateCdfJson(castVoteRecords); + writer.generateCdfJson(castVoteRecords.getCvrs()); conversionSuccess = true; } } catch (IOException @@ -139,10 +140,9 @@ boolean convertToCdf() { return conversionSuccess; } - int parseAndCountCastVoteRecords() { + LoadedCvrData parseAndCountCastVoteRecords() { ContestConfig config = ContestConfig.loadContestConfig(configPath); - List cvrs = parseCastVoteRecords(config); - return cvrs == null ? -1 : cvrs.size(); + return parseCastVoteRecords(config); } // Returns a List of exception class names that were thrown while tabulating. @@ -150,7 +150,7 @@ int parseAndCountCastVoteRecords() { // 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) { + List tabulate(String operatorName, LoadedCvrData expectedCvrData) { Logger.info("Starting tabulation session..."); List exceptionsEncountered = new LinkedList<>(); ContestConfig config = ContestConfig.loadContestConfig(configPath); @@ -192,8 +192,14 @@ List tabulate(String operatorName) { Logger.info( "Beginning tabulation for seat #%d...", config.getSequentialWinners().size() + 1); // Read cast vote records and precinct IDs from CVR files - List castVoteRecords = parseCastVoteRecords(config); - if (castVoteRecords == null) { + LoadedCvrData castVoteRecords = parseCastVoteRecords(config); + if (config.getSequentialWinners().isEmpty() + && !castVoteRecords.metadataMatches(expectedCvrData)) { + Logger.severe("CVR Data has changed between loading the CVRs and reading them!"); + exceptionsEncountered.add(TabulationAbortedException.class.toString()); + break; + } + if (!castVoteRecords.successfullyReadAll) { String errorMessage = "Aborting tabulation due to cast vote record errors!"; exceptionsEncountered.add(errorMessage); Logger.severe(errorMessage); @@ -201,7 +207,7 @@ List tabulate(String operatorName) { } Set newWinnerSet; try { - newWinnerSet = runTabulationForConfig(config, castVoteRecords); + newWinnerSet = runTabulationForConfig(config, castVoteRecords.getCvrs()); } catch (TabulationAbortedException exception) { exceptionsEncountered.add(exception.getClass().toString()); Logger.severe(exception.getMessage()); @@ -230,14 +236,17 @@ List tabulate(String operatorName) { } else { // normal operation (not multi-pass IRV, a.k.a. sequential multi-seat) // Read cast vote records and precinct IDs from CVR files - List castVoteRecords = parseCastVoteRecords(config); - if (castVoteRecords == null) { + LoadedCvrData castVoteRecords = parseCastVoteRecords(config); + if (!castVoteRecords.metadataMatches(expectedCvrData)) { + Logger.severe("CVR Data has changed between loading the CVRs and reading them!"); + exceptionsEncountered.add(TabulationAbortedException.class.toString()); + } else if (!castVoteRecords.successfullyReadAll) { String errorMessage = "Aborting tabulation due to cast vote record errors!"; exceptionsEncountered.add(errorMessage); Logger.severe(errorMessage); } else { try { - runTabulationForConfig(config, castVoteRecords); + runTabulationForConfig(config, castVoteRecords.getCvrs()); tabulationSuccess = true; } catch (TabulationAbortedException exception) { exceptionsEncountered.add(exception.getClass().toString()); @@ -290,7 +299,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 List parseCastVoteRecords(ContestConfig config) { + private LoadedCvrData parseCastVoteRecords(ContestConfig config) { Logger.info("Parsing cast vote records..."); List castVoteRecords = new ArrayList<>(); boolean encounteredSourceProblem = false; @@ -381,7 +390,7 @@ private List parseCastVoteRecords(ContestConfig config) { } else { Logger.info("Parsed %d cast vote records successfully.", castVoteRecords.size()); } - return castVoteRecords; + return new LoadedCvrData(castVoteRecords); } static class UnrecognizedCandidatesException extends Exception { @@ -393,4 +402,68 @@ static class UnrecognizedCandidatesException extends Exception { this.candidateCounts = candidateCounts; } } + + /** + * 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 LoadedCvrData MatchesAnything = new LoadedCvrData(true); + public final boolean successfullyReadAll; + + private List cvrs; + private int numCvrs; + private boolean isDiscarded; + private boolean doesMatchAllMetadata; + + public LoadedCvrData(List cvrs) { + this.cvrs = cvrs; + this.successfullyReadAll = cvrs != null; + this.numCvrs = cvrs != null ? cvrs.size() : 0; + this.isDiscarded = false; + this.doesMatchAllMetadata = false; + } + + /** + * This constructor will cause metadataMatches to always return true. + */ + private LoadedCvrData(boolean doesMatchAllMetadata) { + this.cvrs = null; + this.successfullyReadAll = false; + this.numCvrs = 0; + this.isDiscarded = false; + this.doesMatchAllMetadata = true; + } + + /** + * 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(); + } + + public int numCvrs() { + return numCvrs; + } + + public void discard() { + cvrs = null; + isDiscarded = true; + } + + public List getCvrs() { + if (isDiscarded) { + throw new IllegalStateException("CVRs have been discarded from memory."); + } + return cvrs; + } + } } diff --git a/src/main/resources/network/brightspots/rcv/GuiTabulationPopup.fxml b/src/main/resources/network/brightspots/rcv/GuiTabulationPopup.fxml index 6d83bfcfe..be19babaa 100644 --- a/src/main/resources/network/brightspots/rcv/GuiTabulationPopup.fxml +++ b/src/main/resources/network/brightspots/rcv/GuiTabulationPopup.fxml @@ -101,7 +101,7 @@ -