Skip to content

Commit

Permalink
check if ranking is allowed rather than using unacceptable default
Browse files Browse the repository at this point in the history
  • Loading branch information
artoonie committed Jan 23, 2024
1 parent 7b45e7e commit 998b780
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 22 deletions.
4 changes: 2 additions & 2 deletions src/main/java/network/brightspots/rcv/BaseCvrReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ abstract void readCastVoteRecords(List<CastVoteRecord> castVoteRecords)
throws CastVoteRecord.CvrParseException, IOException;

// Individual contests may have a different value than what the config allows.
public Integer getMaxRankingsAllowed(String contestId) {
return config.getMaxRankingsAllowed();
public boolean isRankingAllowed(int rank, String contestId) {
return config.isRankingAllowed(rank);
}

// Any reader-specific validations can override this function.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,10 @@ void readCastVoteRecords(List<CastVoteRecord> castVoteRecords)
choiceName = Tabulator.UNDECLARED_WRITE_IN_OUTPUT_LABEL;
}
Integer rank = Integer.parseInt(choiceFields[RcvChoiceHeaderField.RANK.ordinal()]);
if (rank > this.config.getMaxRankingsAllowed()) {
if (!this.config.isRankingAllowed(rank)) {
Logger.severe(
"Rank: %d exceeds max rankings allowed in config: %d",
rank, this.config.getMaxRankingsAllowed());
"Rank: %d exceeds max rankings allowed in config: %s",
rank, this.config.getMaxRankingsAllowedAsString());
throw new CvrParseException();
}
columnIndexToRanking.put(columnIndex, new Pair<>(rank, choiceName));
Expand Down
41 changes: 34 additions & 7 deletions src/main/java/network/brightspots/rcv/ContestConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -678,9 +678,9 @@ private void validateRules() {
Logger.severe("Invalid winnerElectionMode!");
}

if (getMaxRankingsAllowed() == null
if (getMaxRankingsAllowedAsString() == null
|| (getNumDeclaredCandidates() >= 1
&& getMaxRankingsAllowed() < MIN_MAX_RANKINGS_ALLOWED)) {
&& !isRankingAllowed(MIN_MAX_RANKINGS_ALLOWED))) {
validationErrors.add(ValidationError.RULES_MAX_RANKINGS_ALLOWED_INVALID);
Logger.severe(
"maxRankingsAllowed must either be \"%s\" or an integer from %d to %d!",
Expand Down Expand Up @@ -988,11 +988,38 @@ private String getMaxRankingsAllowedRaw() {
return rawConfig.rules.maxRankingsAllowed;
}

Integer getMaxRankingsAllowed() {
return stringToIntWithOption(
getMaxRankingsAllowedRaw(),
MAX_RANKINGS_ALLOWED_NUM_CANDIDATES_OPTION,
getNumDeclaredCandidates());
// Because the max rank can be set to "max", there's no reasonable value we can
// return outward. Returning MAX_VALUE could cause integer overflows if
// the caller tries to do arithmetic with the result (e.g. convert to a max column)
// Instead of returning the max rank, only allow checking via this function,
// or returning the value as a string for audit logging.
boolean isRankingAllowed(int rank) {
if (isMaxRankingsSetToMaximum()) {
return true;
}

return rank <= Integer.parseInt(getMaxRankingsAllowedRaw());
}

// There are times when it is necessary to grab the max ranking, though:
// notably, when writing outputs up until the max ranking, or when
// reading inputs where a "blank" indicates an undeclared write-in.
// Handle that, but ensure the caller has checked if it's set to "max"
Integer getMaxRankingsAllowedWhenNotSetToMaximum() {
if (isMaxRankingsSetToMaximum()) {
throw new RuntimeException(
"Do not call this function without first checking isMaxRankingsSetToMaximum!");
}

return Integer.parseInt(getMaxRankingsAllowedRaw());
}

boolean isMaxRankingsSetToMaximum() {
return getMaxRankingsAllowedRaw().equals(MAX_RANKINGS_ALLOWED_NUM_CANDIDATES_OPTION);
}

String getMaxRankingsAllowedAsString() {
return getMaxRankingsAllowedRaw();
}

boolean isBatchEliminationEnabled() {
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/network/brightspots/rcv/DominionCvrReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,8 @@ public void runAdditionalValidations(List<CastVoteRecord> castVoteRecords)
}

@Override
public Integer getMaxRankingsAllowed(String contestId) {
return contests.get(contestId).getMaxRanks();
public boolean isRankingAllowed(int rank, String contestId) {
return rank <= contests.get(contestId).getMaxRanks();
}

private void validateNamesAreInContest(List<CastVoteRecord> castVoteRecords)
Expand Down
16 changes: 13 additions & 3 deletions src/main/java/network/brightspots/rcv/ResultsWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,8 @@ void generateOverallSummaryFiles(
// returns the filepath written
String writeGenericCvrCsv(
List<CastVoteRecord> castVoteRecords,
Integer numRanks,
CvrSource source,
BaseCvrReader reader,
String csvOutputFolder,
String inputFilepath,
String contestId,
Expand Down Expand Up @@ -581,7 +582,11 @@ String writeGenericCvrCsv(
csvPrinter.print("Record Id");
csvPrinter.print("Precinct");
csvPrinter.print("Precinct Portion");
for (int rank = 1; rank <= numRanks; rank++) {
int maxRank = 0;
for (CastVoteRecord castVoteRecord : castVoteRecords) {
maxRank = Math.max(maxRank, castVoteRecord.candidateRankings.maxRankingNumber());
}
for (int rank = 1; rank <= maxRank; rank++) {
String label = String.format("Rank %d", rank);
csvPrinter.print(label);
}
Expand All @@ -602,7 +607,7 @@ String writeGenericCvrCsv(
} else {
csvPrinter.print(castVoteRecord.getPrecinctPortion());
}
printRankings(undeclaredWriteInLabel, numRanks, csvPrinter, castVoteRecord);
printRankings(undeclaredWriteInLabel, maxRank, reader, source, csvPrinter, castVoteRecord);
csvPrinter.println();
}
// finalize the file
Expand All @@ -624,12 +629,17 @@ String writeGenericCvrCsv(
private void printRankings(
String undeclaredWriteInLabel,
Integer maxRanks,
BaseCvrReader reader,
CvrSource source,
CSVPrinter csvPrinter,
CastVoteRecord castVoteRecord)
throws IOException {
// for each rank determine what candidate id, overvote, or undervote occurred and print it
for (int rank = 1; rank <= maxRanks; rank++) {
if (castVoteRecord.candidateRankings.hasRankingAt(rank)) {
if (reader.isRankingAllowed(rank, source.getContestId())) {
break;
}
CandidatesAtRanking candidates = castVoteRecord.candidateRankings.get(rank);
if (candidates.count() == 1) {
String selection = candidates.get(0);
Expand Down
6 changes: 4 additions & 2 deletions src/main/java/network/brightspots/rcv/StreamingCvrReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,9 @@ private void beginCvr() {
// complete construction of new CVR object
private void endCvr() {
// handle any empty cells which may appear at the end of this row
handleEmptyCells(config.getMaxRankingsAllowed() + 1);
if (!config.isMaxRankingsSetToMaximum()) {
handleEmptyCells(config.getMaxRankingsAllowedWhenNotSetToMaximum() + 1);
}
String computedCastVoteRecordId =
String.format("%s-%d", ResultsWriter.sanitizeStringForOutput(excelFileName), cvrIndex);

Expand Down Expand Up @@ -215,7 +217,7 @@ private void cvrCell(int col, String cellData) {

// see if this column is in the ranking range
if (col >= firstVoteColumnIndex
&& col < firstVoteColumnIndex + config.getMaxRankingsAllowed()) {
&& config.isRankingAllowed(col - firstVoteColumnIndex)) {
int currentRank = col - firstVoteColumnIndex + 1;
// handle any empty cells which may exist between this cell and any previous one
handleEmptyCells(currentRank);
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/network/brightspots/rcv/Tabulator.java
Original file line number Diff line number Diff line change
Expand Up @@ -1132,8 +1132,8 @@ && isCandidateContinuing(cvr.getCurrentRecipientOfVote())) {
// if this is the last ranking we are out of rankings and must exhaust this cvr
// determine if the reason is skipping too many ranks, or no continuing candidates
if (rank == cvr.candidateRankings.maxRankingNumber()) {
if (config.getMaxSkippedRanksAllowed() != Integer.MAX_VALUE
&& config.getMaxRankingsAllowed() - rank > config.getMaxSkippedRanksAllowed()) {
if (!config.isMaxRankingsSetToMaximum()
&& config.getMaxRankingsAllowedWhenNotSetToMaximum() - rank > config.getMaxSkippedRanksAllowed()) {
recordSelectionForCastVoteRecord(
cvr, roundTally, null, StatusForRound.INACTIVE_BY_UNDERVOTE, "");
} else {
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/network/brightspots/rcv/TabulatorSession.java
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,8 @@ private List<CastVoteRecord> parseCastVoteRecords(ContestConfig config) {
this.convertedFilePath =
writer.writeGenericCvrCsv(
castVoteRecords,
reader.getMaxRankingsAllowed(source.getContestId()),
source,
reader,
config.getOutputDirectory(),
source.getFilePath(),
source.getContestId(),
Expand Down

0 comments on commit 998b780

Please sign in to comment.