Skip to content

Commit

Permalink
don't allow change in ballot stats between loading and tabulating
Browse files Browse the repository at this point in the history
  • Loading branch information
artoonie committed Apr 29, 2024
1 parent 842fa95 commit 1f1f9d4
Show file tree
Hide file tree
Showing 6 changed files with 130 additions and 36 deletions.
34 changes: 22 additions & 12 deletions src/main/java/network/brightspots/rcv/GuiConfigController.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -508,9 +509,9 @@ public void menuItemTabulateClicked() {
* Tabulate whatever is currently entered into the GUI.
* Assumes GuiTabulateController checked all prerequisites.
*/
public Service<Integer> parseAndCountCastVoteRecords(String configPath) {
public Service<LoadedCvrData> parseAndCountCastVoteRecords(String configPath) {
setGuiIsBusy(true);
Service<Integer> service = new ReadCastVoteRecordsService(configPath);
Service<LoadedCvrData> service = new ReadCastVoteRecordsService(configPath);
setUpAndStartService(service);
return service;
}
Expand All @@ -520,10 +521,13 @@ public Service<Integer> parseAndCountCastVoteRecords(String configPath) {
* Assumes GuiTabulateController checked all prerequisites.
*/
public Service<Boolean> 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;
}
Expand Down Expand Up @@ -1754,10 +1758,16 @@ protected void setUpTaskCompletionTriggers(Task<Boolean> 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
Expand All @@ -1767,7 +1777,7 @@ protected Task<Boolean> createTask() {
@Override
protected Boolean call() {
TabulatorSession session = new TabulatorSession(configPath);
List<String> errors = session.tabulate(operatorName);
List<String> errors = session.tabulate(operatorName, expectedCvrStatistics);
if (errors.isEmpty()) {
succeeded();
} else {
Expand Down Expand Up @@ -1806,27 +1816,27 @@ protected Boolean call() {
}

// ReadCastVoteRecordsService reads CVRs
private static class ReadCastVoteRecordsService extends Service<Integer> {
private static class ReadCastVoteRecordsService extends Service<LoadedCvrData> {
private final String configPath;

ReadCastVoteRecordsService(String configPath) {
this.configPath = configPath;
}

@Override
protected Task<Integer> createTask() {
protected Task<LoadedCvrData> 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;
}
};
}
Expand Down
17 changes: 13 additions & 4 deletions src/main/java/network/brightspots/rcv/GuiTabulateController.java
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -144,7 +150,7 @@ public void buttonloadCvrsClicked(ActionEvent actionEvent) {
}

enableButtonsUpTo(null);
Service<Integer> service = guiConfigController.parseAndCountCastVoteRecords(configPath);
Service<LoadedCvrData> service = guiConfigController.parseAndCountCastVoteRecords(configPath);
// Dispatch a function that watches the service and updates the progress bar
watchParseCvrServiceProgress(service);
}
Expand All @@ -163,7 +169,7 @@ public void buttonTabulateClicked(ActionEvent actionEvent) {
}

Service<Boolean> 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);
}
Expand Down Expand Up @@ -196,11 +202,14 @@ private void watchTabulatorServiceProgress(Service<Boolean> service) {
watchGenericService(service, onSuceededEvent);
}

private void watchParseCvrServiceProgress(Service<Integer> service) {
private void watchParseCvrServiceProgress(Service<LoadedCvrData> service) {
EventHandler<WorkerStateEvent> 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);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/network/brightspots/rcv/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
105 changes: 89 additions & 16 deletions src/main/java/network/brightspots/rcv/TabulatorSession.java
Original file line number Diff line number Diff line change
Expand Up @@ -105,18 +105,19 @@ boolean convertToCdf() {
Logger.info("Converting CVR(s) to CDF...");
try {
FileUtils.createOutputDirectory(config.getOutputDirectory());
List<CastVoteRecord> 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<String> precinctIds = new Tabulator(castVoteRecords, config).getPrecinctIds();
Set<String> 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
Expand All @@ -139,18 +140,17 @@ boolean convertToCdf() {
return conversionSuccess;
}

int parseAndCountCastVoteRecords() {
LoadedCvrData parseAndCountCastVoteRecords() {
ContestConfig config = ContestConfig.loadContestConfig(configPath);
List<CastVoteRecord> cvrs = parseCastVoteRecords(config);
return cvrs == null ? -1 : cvrs.size();
return parseCastVoteRecords(config);
}

// Returns a List of exception class names that were thrown while tabulating.
// Operator name is required for the audit logs.
// 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<String> tabulate(String operatorName) {
List<String> tabulate(String operatorName, LoadedCvrData expectedCvrData) {
Logger.info("Starting tabulation session...");
List<String> exceptionsEncountered = new LinkedList<>();
ContestConfig config = ContestConfig.loadContestConfig(configPath);
Expand Down Expand Up @@ -192,16 +192,22 @@ List<String> 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<CastVoteRecord> 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);
break;
}
Set<String> newWinnerSet;
try {
newWinnerSet = runTabulationForConfig(config, castVoteRecords);
newWinnerSet = runTabulationForConfig(config, castVoteRecords.getCvrs());
} catch (TabulationAbortedException exception) {
exceptionsEncountered.add(exception.getClass().toString());
Logger.severe(exception.getMessage());
Expand Down Expand Up @@ -230,14 +236,17 @@ List<String> 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<CastVoteRecord> 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());
Expand Down Expand Up @@ -290,7 +299,7 @@ private Set<String> 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<CastVoteRecord> parseCastVoteRecords(ContestConfig config) {
private LoadedCvrData parseCastVoteRecords(ContestConfig config) {
Logger.info("Parsing cast vote records...");
List<CastVoteRecord> castVoteRecords = new ArrayList<>();
boolean encounteredSourceProblem = false;
Expand Down Expand Up @@ -381,7 +390,7 @@ private List<CastVoteRecord> 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 {
Expand All @@ -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<CastVoteRecord> cvrs;
private int numCvrs;
private boolean isDiscarded;
private boolean doesMatchAllMetadata;

public LoadedCvrData(List<CastVoteRecord> 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<CastVoteRecord> getCvrs() {
if (isDiscarded) {
throw new IllegalStateException("CVRs have been discarded from memory.");
}
return cvrs;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@
<ProgressBar fx:id="progressBar" layoutX="19.0" layoutY="429.0" prefHeight="20.0" prefWidth="563.0" progress="0.0" />
<HBox layoutX="21.0" layoutY="367.0" prefHeight="42.0" prefWidth="556.0" spacing="20.0">
<children>
<Button fx:id="loadCvrButton" defaultButton="true" disable="true" mnemonicParsing="false" onAction="#buttonloadCvrsClicked" prefHeight="42.0" prefWidth="200.0" text="Load CVRs" />
<Button fx:id="loadCvrButton" defaultButton="true" disable="true" mnemonicParsing="false" onAction="#buttonloadCvrsClicked" prefHeight="42.0" prefWidth="200.0" text="Check Ballot Counts" />
<Text layoutY="21.0" strokeType="OUTSIDE" strokeWidth="0.0" text="&gt;" textAlignment="CENTER" translateY="11.0" />
<Button fx:id="tabulateButton" defaultButton="true" disable="true" layoutX="10.0" layoutY="10.0" mnemonicParsing="false" onAction="#buttonTabulateClicked" prefHeight="42.0" prefWidth="200.0" text="Tabulate" />
<Text strokeType="OUTSIDE" strokeWidth="0.0" text="&gt;" textAlignment="CENTER" translateY="11.0" />
Expand Down
6 changes: 4 additions & 2 deletions src/test/java/network/brightspots/rcv/TabulatorTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,8 @@ private static void runTabulationTest(String stem, String expectedException) {

Logger.info("Running tabulation test: %s\nTabulating config file: %s...", stem, configPath);
TabulatorSession session = new TabulatorSession(configPath);
List<String> exceptionsEncountered = session.tabulate("Automated test");
List<String> exceptionsEncountered = session.tabulate("Automated test",
TabulatorSession.LoadedCvrData.MatchesAnything);
if (expectedException != null) {
assertTrue(exceptionsEncountered.contains(expectedException));
} else {
Expand Down Expand Up @@ -240,7 +241,8 @@ private static void runConvertToCdfTest(String stem) {
private static void runConvertToCsvTest(String stem) {
String configPath = getTestFilePath(stem, "_config.json");
TabulatorSession session = new TabulatorSession(configPath);
session.tabulate("Automated test");
session.tabulate("Automated test",
TabulatorSession.LoadedCvrData.MatchesAnything);

String expectedPath = getTestFilePath(stem, "_expected.csv");
assertTrue(fileCompare(session.getConvertedFilePath(), expectedPath));
Expand Down

0 comments on commit 1f1f9d4

Please sign in to comment.