Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated - and greatly simplified - GenerateAssertionsResponse. Now ju… #175

Merged
merged 2 commits into from
Aug 9, 2024
Merged

Updated - and greatly simplified - GenerateAssertionsResponse. Now ju… #175

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1e5a59a
Updated - and greatly simplified - GenerateAssertionsResponse. Now ju…
vteague Aug 7, 2024
9720366
Improvements from @michelleblom's review.
vteague Aug 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 16 additions & 12 deletions ...democracydevelopers/corla/communication/responseFromRaire/GenerateAssertionsResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,29 @@

package au.org.democracydevelopers.corla.communication.responseFromRaire;

import java.util.Objects;

/**
* The success response when a ContestRequest is sent to raire's generate-assertions endpoint. This
* simply returns the winner, as calculated by raire, along with the name of the contest for which
* the initial request was made.
* This record is identical to the record of the same name in raire-service. Used for
* The response when a ContestRequest is sent to raire's generate-assertions endpoint.
* This class is identical to the record of the same name in raire-service. Used for
* deserialization.
* All four states of the two booleans are possible - for example, generation may succeed, but
* receive a TIME_OUT_TRIMMING_ASSERTIONS warning, in which case retry will be true.
*/
public final class GenerateAssertionsResponse {
public String contestName;
public String winner;
public boolean succeeded;
public boolean retry;

/**
* @param contestName The name of the contest.
* @param winner The winner of the contest, as calculated by raire.
*/
public GenerateAssertionsResponse(String contestName, String winner) {

/**
* All args constructor.
* @param contestName The name of the contest.
* @param succeeded Whether assertion generation succeeded.
* @param retry Whether it is worth retrying assertion generation.
*/
public GenerateAssertionsResponse(String contestName, boolean succeeded, boolean retry) {
this.contestName = contestName;
this.winner = winner;
this.succeeded = succeeded;
this.retry = retry;
}
}
50 changes: 0 additions & 50 deletions ...opers/corla/communication/responseToColoradoRla/GenerateAssertionsResponseWithErrors.java
View file
Open in desktop

This file was deleted.

61 changes: 23 additions & 38 deletions ...e-project/src/main/java/au/org/democracydevelopers/corla/endpoint/GenerateAssertions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@

import au.org.democracydevelopers.corla.communication.requestToRaire.GenerateAssertionsRequest;
import au.org.democracydevelopers.corla.communication.responseFromRaire.GenerateAssertionsResponse;
import au.org.democracydevelopers.corla.communication.responseFromRaire.RaireServiceErrors;
import au.org.democracydevelopers.corla.communication.responseToColoradoRla.GenerateAssertionsResponseWithErrors;
import com.google.gson.JsonSyntaxException;
import org.apache.http.HttpResponse;
import org.apache.http.HttpStatus;
Expand Down Expand Up @@ -120,7 +118,7 @@ public String endpointBody(final Request the_request, final Response the_respons
final String prefix = "[endpointBody]";
LOGGER.debug(String.format("%s %s.", prefix, "Received Generate Assertions request"));

final List<GenerateAssertionsResponseWithErrors> responseData;
final List<GenerateAssertionsResponse> responseData;

final String raireUrl = Main.properties().getProperty(RAIRE_URL, "") + RAIRE_ENDPOINT;

Expand Down Expand Up @@ -177,22 +175,20 @@ public String endpointBody(final Request the_request, final Response the_respons
* - Gather all the IRVContestResults
* - For each IRV contest, make a request to the raire-service get-assertions endpoint of the right format type
* - Collate all the results into a list.
*
* @param IRVContestResults the collection of all IRV ContestResults.
* @param timeLimitSeconds the time limit for raire assertion generation, per contest.
* @param raireUrl the url where the raire-service is running.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing these input arguments can be final?

*/
protected List<GenerateAssertionsResponseWithErrors> generateAllAssertions(List<ContestResult> IRVContestResults,
double timeLimitSeconds, String raireUrl) {
protected List<GenerateAssertionsResponse> generateAllAssertions(List<ContestResult> IRVContestResults,
double timeLimitSeconds, String raireUrl) {
final String prefix = "[generateAllAssertions]";
LOGGER.debug(String.format("%s %s.", prefix, "Generating assertions for all IRV contests"));

final List<GenerateAssertionsResponseWithErrors> responseData = new ArrayList<>();

// Iterate through all IRV Contests, sending a request to the raire-service for each one's assertions and
for (final ContestResult cr : IRVContestResults) {
GenerateAssertionsResponseWithErrors response = generateAssertionsUpdateWinners(IRVContestResults, cr.getContestName(), timeLimitSeconds, raireUrl);
responseData.add(response);
}
// Iterate through all IRV Contests, sending a request to the raire-service for each one's assertions
final List<GenerateAssertionsResponse> responseData = IRVContestResults.stream().map(
r -> generateAssertionsUpdateWinners(IRVContestResults, r.getContestName(), timeLimitSeconds, raireUrl)
).toList();

LOGGER.debug(String.format("%s %s.", prefix, "Completed assertion generation for all IRV contests"));
return responseData;
Expand All @@ -202,21 +198,23 @@ protected List<GenerateAssertionsResponseWithErrors> generateAllAssertions(List<
* The main work of this endpoint - sends the appropriate request for a single contest, and
* updates stored data with the result. There are two expected kinds of responses from raire:
* - a success response with a winner, or
* - an INTERNAL_SERVER_ERROR response with a reason, e.g. TIED_WINNERS or NO_VOTES_PRESENT.
* These are expected to happen occasionally because of the data.
* - an error response with a reason, e.g. TIED_WINNERS or NO_VOTES_PRESENT.
* These are expected to happen occasionally because of the data, and are saved in the
* GenerateAssertionsSummary table.
* Other errors, such as a BAD_REQUEST response or a failure to parse raire's response, indicate
* programming or configuration errors. These are logged.
* programming or configuration errors. These are logged and an exception is thrown.
*
* @param IRVContestResults The list of all ContestResults for IRV contests. Note that these do
* not have the correct winners or losers for IRV.
* @param contestName The name of the contest.
* @param timeLimitSeconds The time limit allowed for raire to compute the assertions (not
* counting time taken to retrieve vote data from the database).
* @param raireUrl The url of the raire service.
* @return The GenerateAssertionsResponseWithErrors, which usually contains a
* winner but may instead be UNKNOWN_WINNER and an error message.
* @return The GenerateAssertionsResponseWithErrors, which usually contains a
* winner but may instead be UNKNOWN_WINNER and an error message.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check input arguments for what can be final.

*/
protected GenerateAssertionsResponseWithErrors generateAssertionsUpdateWinners(List<ContestResult> IRVContestResults,
String contestName, double timeLimitSeconds, String raireUrl) {
protected GenerateAssertionsResponse generateAssertionsUpdateWinners(List<ContestResult> IRVContestResults,
String contestName, double timeLimitSeconds, String raireUrl) {
final String prefix = "[generateAssertionsUpdateWinners]";
LOGGER.debug(String.format("%s %s %s.", prefix, "Generating assertions for contest ", contestName));

Expand Down Expand Up @@ -248,30 +246,17 @@ protected GenerateAssertionsResponseWithErrors generateAssertionsUpdateWinners(L

// Interpret the response.
final int statusCode = raireResponse.getStatusLine().getStatusCode();
final boolean gotRaireError = raireResponse.containsHeader(RaireServiceErrors.ERROR_CODE_KEY);

if (statusCode == HttpStatus.SC_OK && !gotRaireError) {
// OK response. Return the winner.
if (statusCode == HttpStatus.SC_OK) {

LOGGER.debug(String.format("%s %s %s.", prefix, "OK response received from RAIRE for",
contestName));
// OK response, which may indicate either that assertion generation succeeded, or that it
// failed and raire generated a useful error. Return raire's response.
GenerateAssertionsResponse responseFromRaire = Main.GSON.fromJson(EntityUtils.toString(raireResponse.getEntity()),
GenerateAssertionsResponse.class);

LOGGER.debug(String.format("%s %s %s.", prefix,
"Completed assertion generation for contest", contestName));
return new GenerateAssertionsResponseWithErrors(contestName, responseFromRaire.winner, "");

} else if (statusCode == HttpStatus.SC_INTERNAL_SERVER_ERROR && gotRaireError) {
// Error response about a specific contest, e.g. "TIED_WINNERS". Return the error.

final String code = raireResponse.getFirstHeader(RaireServiceErrors.ERROR_CODE_KEY).getValue();
LOGGER.debug(String.format("%s %s %s.", prefix, "Error response " + code,
"received from RAIRE for " + contestName));

LOGGER.debug(String.format("%s %s %s.", prefix,
"Error response for assertion generation for contest ", contestName));
return new GenerateAssertionsResponseWithErrors(cr.getContestName(), UNKNOWN_WINNER, code);
LOGGER.debug(String.format("%s %s %s %s.", prefix, responseFromRaire.succeeded ? "Success" : "Failure",
"response for raire assertion generation for contest", contestName));
return responseFromRaire;

} else {
// Something went wrong with the connection, e.g. 404 or a Bad Request. Cannot continue.
Expand Down
62 changes: 35 additions & 27 deletions ...ject/src/test/java/au/org/democracydevelopers/corla/endpoint/GenerateAssertionsTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@

import au.org.democracydevelopers.corla.communication.requestToRaire.GenerateAssertionsRequest;
import au.org.democracydevelopers.corla.communication.responseFromRaire.GenerateAssertionsResponse;
import au.org.democracydevelopers.corla.communication.responseFromRaire.RaireServiceErrors;
import au.org.democracydevelopers.corla.communication.responseToColoradoRla.GenerateAssertionsResponseWithErrors;

import static au.org.democracydevelopers.corla.endpoint.GenerateAssertions.UNKNOWN_WINNER;
import static au.org.democracydevelopers.corla.util.testUtils.*;
Expand All @@ -50,6 +48,8 @@

import static com.github.tomakehurst.wiremock.client.WireMock.*;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertTrue;
import static org.testng.Assert.assertFalse;

/**
* Test the GetAssertions endpoint, both CSV and JSON versions. The response is supposed to be a zip file containing
Expand Down Expand Up @@ -81,13 +81,19 @@ public class GenerateAssertionsTests extends TestClassWithDatabase {
* Mock response for Boulder Mayoral '23
*/
private final static GenerateAssertionsResponse boulderResponse
= new GenerateAssertionsResponse(boulderMayoral, "Aaron Brockett");
= new GenerateAssertionsResponse(boulderMayoral, true, false);

/**
* Mock response for tinyExample1 contest
*/
private final static GenerateAssertionsResponse tinyIRVResponse
= new GenerateAssertionsResponse(tinyIRV, "Alice");
= new GenerateAssertionsResponse(tinyIRV, true, false);

/**
* Mock response for tiedIRV contest
*/
private final static GenerateAssertionsResponse tiedIRVResponse
= new GenerateAssertionsResponse(tiedIRV, false, false);

/**
* Request for Boulder Mayoral '23
Expand Down Expand Up @@ -181,7 +187,8 @@ public void initMocks() {
tiedIRVContestResult.addContests(Set.of(tiedIRVContest));

// Default raire server. You can instead run the real raire service and set baseUrl accordingly,
// though the tests of invalid/uninterpretable data will fail.
// though the tests of invalid/uninterpretable data will fail, and of course you have to have
// appropriate contests in the database.
wireMockRaireServer.start();
baseUrl = wireMockRaireServer.baseUrl();
String badUrl = baseUrl + badEndpoint;
Expand All @@ -200,14 +207,13 @@ public void initMocks() {
.withStatus(HttpStatus.SC_OK)
.withHeader("Content-Type", "application/json")
.withBody(gson.toJson(tinyIRVResponse))));
// Mock a TIED_WINNERS response to the tiedIRV contest.
// Mock failed, don't redo response to the tiedIRV contest.
stubFor(post(urlEqualTo(raireGenerateAssertionsEndpoint))
.withRequestBody(equalToJson(gson.toJson(tiedIRVRequest)))
.willReturn(aResponse()
.withStatus(HttpStatus.SC_INTERNAL_SERVER_ERROR)
.withStatus(HttpStatus.SC_OK)
.withHeader("Content-Type", "application/json")
.withHeader(RaireServiceErrors.ERROR_CODE_KEY,
RaireServiceErrors.RaireErrorCodes.TIED_WINNERS.toString())));
.withBody(gson.toJson(tiedIRVResponse))));
// Mock a 404 for badUrl.
stubFor(post(urlEqualTo(badUrl))
.withRequestBody(equalToJson(gson.toJson(tinyIRVRequest)))
Expand All @@ -221,7 +227,6 @@ public void initMocks() {
.withStatus(HttpStatus.SC_OK)
.withHeader("Content-Type", "application/json")
.withBody(gson.toJson(tinyIRVCandidates))));

// Mock an OK response with invalid json.
stubFor(post(urlEqualTo(invalidResponseEndpoint))
.withRequestBody(equalToJson(gson.toJson(tinyIRVRequest)))
Expand All @@ -239,57 +244,60 @@ public void closeMocks() {

/**
* Calls the single-contest version of the endpoint for the boulder Mayor '23 example, checks
* for the right winner.
* that generation succeeded and does not recommend retry.
*/
@Test
public void rightBoulderIRVWinner() {
testUtils.log(LOGGER, "rightBoulderIRVWinner");

GenerateAssertions endpoint = new GenerateAssertions();
GenerateAssertionsResponseWithErrors result = endpoint.generateAssertionsUpdateWinners(
GenerateAssertionsResponse result = endpoint.generateAssertionsUpdateWinners(
mockedIRVContestResults, boulderRequest.contestName, boulderRequest.timeLimitSeconds,
baseUrl + raireGenerateAssertionsEndpoint);

assertEquals(result.contestName, boulderMayoral);
assertEquals(result.winner, "Aaron Brockett");
assertTrue(result.succeeded);
assertFalse(result.retry);
}

/**
* Calls the single-contest version of the endpoint for the tied contests, checks that the
* winner is unknown and the TIED_WINNERS error is returned.
* Calls the single-contest version of the endpoint for the tied contests, checks that
* assertion generation fails and does not recommend retry.
*/
@Test
public void tiedWinnersCorrectlyRecorded() {
testUtils.log(LOGGER, "tiedWinnersCorrectlyRecorded");
public void tiedWinnersFailsNoRetry() {
testUtils.log(LOGGER, "tiedWinnersFailsNoRetry");

GenerateAssertions endpoint = new GenerateAssertions();
GenerateAssertionsResponseWithErrors result = endpoint.generateAssertionsUpdateWinners(
GenerateAssertionsResponse result = endpoint.generateAssertionsUpdateWinners(
List.of(tiedIRVContestResult), tiedIRV, tiedIRVRequest.timeLimitSeconds,
baseUrl + raireGenerateAssertionsEndpoint);

assertEquals(result.contestName, tiedIRV);
assertEquals(result.winner, UNKNOWN_WINNER);
assertEquals(result.raireError ,RaireServiceErrors.RaireErrorCodes.TIED_WINNERS.toString());
assertFalse(result.succeeded);
assertFalse(result.retry);
}

/**
* Calls the generateAssertions main endpoint function and checks that the right winners are
* returned, for the two example contests (Boulder and TinyIRV).
* Calls the generateAssertions main endpoint function for the two example contests (Boulder and TinyIRV).
* and checks that both succeed and do not recommend retry.
*/
@Test
public void rightWinners() {
testUtils.log(LOGGER, "rightWinners");
public void successAsExpected() {
testUtils.log(LOGGER, "successAsExpected");

GenerateAssertions endpoint = new GenerateAssertions();
List<GenerateAssertionsResponseWithErrors> results
List<GenerateAssertionsResponse> results
= endpoint.generateAllAssertions(mockedIRVContestResults, boulderRequest.timeLimitSeconds,
baseUrl + raireGenerateAssertionsEndpoint);

assertEquals(results.size(), 2);
assertEquals(results.get(0).contestName, boulderMayoral);
assertEquals(results.get(0).winner, "Aaron Brockett");
assertTrue(results.get(0).succeeded);
assertFalse(results.get(0).retry);
assertEquals(results.get(1).contestName, tinyIRV);
assertEquals(results.get(1).winner, "Alice");
assertTrue(results.get(1).succeeded);
assertFalse(results.get(1).retry);
}

/**
Expand Down
Loading