Skip to content

Commit

Permalink
Improve continuous export failure recovery by moving the export opera…
Browse files Browse the repository at this point in the history
…tion "progress marker" from USB drive to VxScan store (#4213)

* Move export operation progress marker from USB drive to VxScan store

* Update scanner stores

* Update libs/backend tests

* Update apps/scan/backend tests

* Move SQLite boolean helpers to shared location and use
  • Loading branch information
arsalansufi authored Nov 15, 2023
1 parent 585e196 commit a45b064
Show file tree
Hide file tree
Showing 17 changed files with 257 additions and 182 deletions.
47 changes: 20 additions & 27 deletions apps/admin/backend/src/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,11 @@ import { join } from 'path';
import { Buffer } from 'buffer';
import { v4 as uuid } from 'uuid';
import {
OfficialCandidateNameLookup,
asSqliteBool,
fromSqliteBool,
getOfficialCandidateNameLookup,
OfficialCandidateNameLookup,
SqliteBool,
} from '@votingworks/utils';
import {
CastVoteRecordFileRecord,
Expand Down Expand Up @@ -97,16 +100,6 @@ function convertSqliteTimestampToIso8601(
return new Date(sqliteTimestamp).toISOString();
}

type SqlBool = 0 | 1;

function asSqlBool(bool: boolean): SqlBool {
return bool ? 1 : 0;
}

function fromSqlBool(sqlBool: SqlBool): boolean {
return sqlBool === 1;
}

function asQueryPlaceholders(list: unknown[]): string {
const questionMarks = list.map(() => '?');
return `(${questionMarks.join(', ')})`;
Expand All @@ -124,7 +117,7 @@ interface WriteInTallyRow {
interface CastVoteRecordVoteAdjudication {
contestId: ContestId;
optionId: ContestOptionId;
isVote: SqlBool;
isVote: SqliteBool;
}

/**
Expand Down Expand Up @@ -212,7 +205,7 @@ export class Store {
id: Id;
electionData: string;
createdAt: string;
isOfficialResults: SqlBool;
isOfficialResults: SqliteBool;
}>
).map((r) => ({
id: r.id,
Expand Down Expand Up @@ -244,7 +237,7 @@ export class Store {
id: Id;
electionData: string;
createdAt: string;
isOfficialResults: SqlBool;
isOfficialResults: SqliteBool;
}
| undefined;
if (!result) {
Expand Down Expand Up @@ -772,7 +765,7 @@ export class Store {
`,
id,
electionId,
asSqlBool(isTestMode),
asSqliteBool(isTestMode),
filename,
exportedTimestamp,
JSON.stringify([]),
Expand Down Expand Up @@ -899,7 +892,7 @@ export class Store {
cvr.precinctId,
cvrSheetNumber,
serializedVotes,
asSqlBool(isBlankSheet(cvr.votes))
asSqliteBool(isBlankSheet(cvr.votes))
);
}

Expand Down Expand Up @@ -1059,7 +1052,7 @@ export class Store {
side,
contestId,
optionId,
asSqlBool(isUnmarked)
asSqliteBool(isUnmarked)
);

return id;
Expand Down Expand Up @@ -1895,8 +1888,8 @@ export class Store {
castVoteRecordId: Id;
contestId: ContestId;
optionId: ContestOptionId;
isInvalid: SqlBool;
isUnmarked: SqlBool;
isInvalid: SqliteBool;
isUnmarked: SqliteBool;
officialCandidateId: string | null;
writeInCandidateId: Id | null;
adjudicatedAt: Iso8601Timestamp | null;
Expand All @@ -1914,7 +1907,7 @@ export class Store {
status: 'adjudicated',
adjudicationType: 'official-candidate',
candidateId: row.officialCandidateId,
isUnmarked: fromSqlBool(row.isUnmarked),
isUnmarked: fromSqliteBool(row.isUnmarked),
});
}

Expand All @@ -1928,7 +1921,7 @@ export class Store {
status: 'adjudicated',
adjudicationType: 'write-in-candidate',
candidateId: row.writeInCandidateId,
isUnmarked: fromSqlBool(row.isUnmarked),
isUnmarked: fromSqliteBool(row.isUnmarked),
});
}

Expand All @@ -1941,7 +1934,7 @@ export class Store {
optionId: row.optionId,
status: 'adjudicated',
adjudicationType: 'invalid',
isUnmarked: fromSqlBool(row.isUnmarked),
isUnmarked: fromSqliteBool(row.isUnmarked),
});
}

Expand All @@ -1952,7 +1945,7 @@ export class Store {
contestId: row.contestId,
optionId: row.optionId,
status: 'pending',
isUnmarked: fromSqlBool(row.isUnmarked),
isUnmarked: fromSqliteBool(row.isUnmarked),
});
});
}
Expand Down Expand Up @@ -2099,13 +2092,13 @@ export class Store {
cvrId: Id;
contestId: Id;
optionId: Id;
isVote: SqlBool;
isVote: SqliteBool;
}>;

return row
? {
...row,
isVote: fromSqlBool(row.isVote),
isVote: fromSqliteBool(row.isVote),
}
: undefined;
}
Expand Down Expand Up @@ -2150,7 +2143,7 @@ export class Store {
cvrId,
contestId,
optionId,
asSqlBool(isVote)
asSqliteBool(isVote)
);
}

Expand Down Expand Up @@ -2477,7 +2470,7 @@ export class Store {
set is_official_results = ?
where id = ?
`,
asSqlBool(isOfficialResults),
asSqliteBool(isOfficialResults),
electionId
);
}
Expand Down
4 changes: 4 additions & 0 deletions apps/central-scan/backend/src/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ function dateTimeFromNoOffsetSqliteDate(noOffsetSqliteDate: string): DateTime {
export class Store {
private constructor(private readonly client: DbClient) {}

// Used by shared CVR export logic in libs/backend
// eslint-disable-next-line vx/gts-no-public-class-fields
readonly scannerType = 'central';

getDbPath(): string {
return this.client.getDatabasePath();
}
Expand Down
10 changes: 8 additions & 2 deletions apps/scan/backend/schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,19 @@ create table sheets (
);

create table system_settings (
-- enforce singleton table
-- Enforce singleton table
id integer primary key check (id = 1),
data text not null -- JSON blob
);

create table is_continuous_export_operation_in_progress (
-- Enforce singleton table
id integer primary key check (id = 1),
is_continuous_export_operation_in_progress boolean not null
);

create table export_directory_name (
-- enforce singleton table
-- Enforce singleton table
id integer primary key check (id = 1),
export_directory_name text not null
);
Expand Down
26 changes: 13 additions & 13 deletions apps/scan/backend/src/app_config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,9 @@ test("if there's only one precinct in the election, it's selected automatically
test('continuous CVR export', async () => {
await withApp(
{},
async ({ apiClient, mockAuth, mockScanner, mockUsbDrive }) => {
async ({ apiClient, mockAuth, mockScanner, mockUsbDrive, workspace }) => {
await configureApp(apiClient, mockAuth, mockUsbDrive, { testMode: true });
await scanBallot(mockScanner, mockUsbDrive, apiClient, 0);
await scanBallot(mockScanner, apiClient, workspace.store, 0);

const exportDirectoryPaths = await getCastVoteRecordExportDirectoryPaths(
mockUsbDrive.usbDrive
Expand Down Expand Up @@ -216,10 +216,10 @@ test('continuous CVR export', async () => {
test('continuous CVR export, including polls closing, followed by a full export', async () => {
await withApp(
{},
async ({ apiClient, mockAuth, mockScanner, mockUsbDrive }) => {
async ({ apiClient, mockAuth, mockScanner, mockUsbDrive, workspace }) => {
await configureApp(apiClient, mockAuth, mockUsbDrive, { testMode: true });
await scanBallot(mockScanner, mockUsbDrive, apiClient, 0);
await scanBallot(mockScanner, mockUsbDrive, apiClient, 1);
await scanBallot(mockScanner, apiClient, workspace.store, 0);
await scanBallot(mockScanner, apiClient, workspace.store, 1);

expect(
await apiClient.exportCastVoteRecordsToUsbDrive({
Expand All @@ -239,9 +239,9 @@ test('continuous CVR export, including polls closing, followed by a full export'
test('CVR resync', async () => {
await withApp(
{},
async ({ apiClient, mockAuth, mockScanner, mockUsbDrive }) => {
async ({ apiClient, mockAuth, mockScanner, mockUsbDrive, workspace }) => {
await configureApp(apiClient, mockAuth, mockUsbDrive, { testMode: true });
await scanBallot(mockScanner, mockUsbDrive, apiClient, 0);
await scanBallot(mockScanner, apiClient, workspace.store, 0);

// When a CVR resync is required, the CVR resync modal appears on the "insert your ballot"
// screen, i.e. the screen displayed when no card is inserted
Expand Down Expand Up @@ -300,8 +300,8 @@ test('ballot batching', async () => {
}

// Scan two ballots, which should have the same batch
await scanBallot(mockScanner, mockUsbDrive, apiClient, 0);
await scanBallot(mockScanner, mockUsbDrive, apiClient, 1);
await scanBallot(mockScanner, apiClient, workspace.store, 0);
await scanBallot(mockScanner, apiClient, workspace.store, 1);
let batchIds = getBatchIds();
expect(getCvrIds()).toHaveLength(2);
expect(batchIds).toHaveLength(1);
Expand Down Expand Up @@ -338,8 +338,8 @@ test('ballot batching', async () => {
});

// Confirm there is a new, second batch distinct from the first
await scanBallot(mockScanner, mockUsbDrive, apiClient, 2);
await scanBallot(mockScanner, mockUsbDrive, apiClient, 3);
await scanBallot(mockScanner, apiClient, workspace.store, 2);
await scanBallot(mockScanner, apiClient, workspace.store, 3);
batchIds = getBatchIds();
expect(getCvrIds()).toHaveLength(4);
expect(batchIds).toHaveLength(2);
Expand Down Expand Up @@ -376,8 +376,8 @@ test('ballot batching', async () => {
});

// Confirm there is a third batch, distinct from the second
await scanBallot(mockScanner, mockUsbDrive, apiClient, 4);
await scanBallot(mockScanner, mockUsbDrive, apiClient, 5);
await scanBallot(mockScanner, apiClient, workspace.store, 4);
await scanBallot(mockScanner, apiClient, workspace.store, 5);
batchIds = getBatchIds();
expect(getCvrIds()).toHaveLength(6);
expect(batchIds).toHaveLength(3);
Expand Down
8 changes: 4 additions & 4 deletions apps/scan/backend/src/app_results.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@ beforeEach(() => {
test('end-to-end tabulated results', async () => {
await withApp(
{},
async ({ apiClient, mockScanner, mockUsbDrive, mockAuth }) => {
async ({ apiClient, mockScanner, mockUsbDrive, mockAuth, workspace }) => {
await configureApp(apiClient, mockAuth, mockUsbDrive, { testMode: true });

// scan a few ballots
await scanBallot(mockScanner, mockUsbDrive, apiClient, 0);
await scanBallot(mockScanner, mockUsbDrive, apiClient, 1);
await scanBallot(mockScanner, mockUsbDrive, apiClient, 2);
await scanBallot(mockScanner, apiClient, workspace.store, 0);
await scanBallot(mockScanner, apiClient, workspace.store, 1);
await scanBallot(mockScanner, apiClient, workspace.store, 2);

const allResults = await apiClient.getScannerResultsByParty();
expect(allResults).toHaveLength(1);
Expand Down
4 changes: 2 additions & 2 deletions apps/scan/backend/src/app_usb_drive.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ test('ejectUsbDrive', async () => {
test('doesUsbDriveRequireCastVoteRecordSync is properly populated', async () => {
await withApp(
{},
async ({ apiClient, mockAuth, mockUsbDrive, mockScanner }) => {
async ({ apiClient, mockAuth, mockUsbDrive, mockScanner, workspace }) => {
await configureApp(apiClient, mockAuth, mockUsbDrive, { testMode: true });
const mountedUsbDriveStatus = {
status: 'mounted',
Expand All @@ -57,7 +57,7 @@ test('doesUsbDriveRequireCastVoteRecordSync is properly populated', async () =>
mountedUsbDriveStatus
);

await scanBallot(mockScanner, mockUsbDrive, apiClient, 0);
await scanBallot(mockScanner, apiClient, workspace.store, 0);
await expect(apiClient.getUsbDriveStatus()).resolves.toEqual(
mountedUsbDriveStatus
);
Expand Down
8 changes: 8 additions & 0 deletions apps/scan/backend/src/scanners/custom/state_machine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -434,11 +434,15 @@ async function recordAcceptedSheet(
const { sheetId } = interpretation;
store.withTransaction(() => {
storeInterpretedSheet(store, sheetId, interpretation);

// If we're storing an accepted sheet that needed review, that means that it was "adjudicated"
// (i.e. the voter said to count it without changing anything).
if (interpretation.type === 'NeedsReviewSheet') {
store.adjudicateSheet(sheetId);
}

// Gets reset to false within exportCastVoteRecordsToUsbDrive
store.setIsContinuousExportOperationInProgress(true);
});
(
await exportCastVoteRecordsToUsbDrive(
Expand All @@ -460,10 +464,14 @@ async function recordRejectedSheet(
const { sheetId } = interpretation;
store.withTransaction(() => {
storeInterpretedSheet(store, sheetId, interpretation);

// We want to keep rejected ballots in the store, but not count them. We accomplish this by
// "deleting" them, which just marks them as deleted and is how we indicate that an interpreted
// ballot wasn't counted.
store.deleteSheet(sheetId);

// Gets reset to false within exportCastVoteRecordsToUsbDrive
store.setIsContinuousExportOperationInProgress(true);
});
(
await exportCastVoteRecordsToUsbDrive(
Expand Down
10 changes: 10 additions & 0 deletions apps/scan/backend/src/store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,16 @@ test('getBallotsCounted', () => {
expect(store.getBallotsCounted()).toEqual(1);
});

test('isContinuousExportOperationInProgress and setIsContinuousExportOperationInProgress', () => {
const store = Store.memoryStore();

expect(store.isContinuousExportOperationInProgress()).toEqual(false);
store.setIsContinuousExportOperationInProgress(true);
expect(store.isContinuousExportOperationInProgress()).toEqual(true);
store.setIsContinuousExportOperationInProgress(false);
expect(store.isContinuousExportOperationInProgress()).toEqual(false);
});

test('getExportDirectoryName and setExportDirectoryName', () => {
const store = Store.memoryStore();

Expand Down
Loading

0 comments on commit a45b064

Please sign in to comment.