Skip to content

Commit

Permalink
rename as suggested in review
Browse files Browse the repository at this point in the history
Signed-off-by: Craig Disselkoen <cdiss@amazon.com>
  • Loading branch information
cdisselkoen committed May 2, 2024
1 parent c777ba4 commit 9350281
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.collect.ImmutableList;

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
Expand All @@ -32,24 +31,24 @@ public final class ValidationResponse {
@JsonProperty("type")
public final SuccessOrFailure type;
/** This will be present if and only if `type` is `Success`. */
public final Optional<ValidationResults> results;
public final Optional<ValidationSuccessResponse> success;
/**
* This will be present if and only if `type` is `Failure`.
*
* These errors are not validation errors (those would be
* reported in `results`), but rather higher-level errors, like
* reported in `success`), but rather higher-level errors, like
* a failure to parse or to call the validator.
*/
public final Optional<ImmutableList<DetailedError>> errors;
/**
* Other warnings not associated with particular policies.
* For instance, warnings about your schema itself.
* These warnings can be produced regardless of whether we have `results` or
* `errors`.
* These warnings can be produced regardless of whether `type` is
* `Success` or `Failure`.
*/
public final ImmutableList<DetailedError> warnings;

public static final class ValidationResults {
public static final class ValidationSuccessResponse {
/** Validation errors associated with particular policies. */
@JsonProperty("validation_errors")
public final ImmutableList<ValidationError> validationErrors;
Expand All @@ -58,7 +57,7 @@ public static final class ValidationResults {
public final ImmutableList<ValidationError> validationWarnings;

@JsonCreator
public ValidationResults(
public ValidationSuccessResponse(
@JsonProperty("validation_errors") Optional<List<ValidationError>> validationErrors,
@JsonProperty("validation_warnings") Optional<List<ValidationError>> validationWarnings) {
// note that ImmutableSet.copyOf() attempts to avoid a full copy when possible; see https://github.com/google/guava/wiki/ImmutableCollectionsExplained
Expand Down Expand Up @@ -93,9 +92,9 @@ public ValidationResponse(
this.type = type;
this.errors = errors.map((list) -> ImmutableList.copyOf(list));
if (type == SuccessOrFailure.Success) {
this.results = Optional.of(new ValidationResults(validationErrors, validationWarnings));
this.success = Optional.of(new ValidationSuccessResponse(validationErrors, validationWarnings));
} else {
this.results = Optional.empty();
this.success = Optional.empty();
}
if (warnings.isPresent()) {
this.warnings = ImmutableList.copyOf(warnings.get());
Expand All @@ -117,8 +116,8 @@ public enum SuccessOrFailure {
* prior to even calling the validator.
*/
public boolean validationPassed() {
if (results.isPresent()) {
return results.get().validationErrors.isEmpty();
if (success.isPresent()) {
return success.get().validationErrors.isEmpty();
} else {
// higher-level errors are present
return false;
Expand All @@ -127,8 +126,8 @@ public boolean validationPassed() {

/** Readable string representation. */
public String toString() {
if (results.isPresent()) {
return "ValidationResponse(validationErrors = " + results.get().validationErrors + ", validationWarnings = " + results.get().validationWarnings + ")";
if (success.isPresent()) {
return "ValidationResponse(validationErrors = " + success.get().validationErrors + ", validationWarnings = " + success.get().validationWarnings + ")";
} else {
return "ValidationResponse(errors = " + errors.get() + ")";
}
Expand Down
12 changes: 6 additions & 6 deletions CedarJava/src/test/java/com/cedarpolicy/ValidationTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import com.cedarpolicy.model.ValidationRequest;
import com.cedarpolicy.model.ValidationResponse;
import com.cedarpolicy.model.ValidationResponse.SuccessOrFailure;
import com.cedarpolicy.model.ValidationResponse.ValidationResults;
import com.cedarpolicy.model.ValidationResponse.ValidationSuccessResponse;
import com.cedarpolicy.model.schema.Schema;
import java.util.HashMap;
import java.util.List;
Expand Down Expand Up @@ -104,12 +104,12 @@ private ValidationResponse whenValidated() {

private void thenIsValid(ValidationResponse response) {
assertEquals(response.type, SuccessOrFailure.Success);
final ValidationResults results = assertDoesNotThrow(() -> response.results.get());
final ValidationSuccessResponse success = assertDoesNotThrow(() -> response.success.get());
assertTrue(
results.validationErrors.isEmpty(),
success.validationErrors.isEmpty(),
() -> {
String errors =
response.results.get().validationErrors.stream()
response.success.get().validationErrors.stream()
.map(note ->
String.format("in policy %s: %s", note.getPolicyId(), note.getError()))
.collect(Collectors.joining("\n"));
Expand All @@ -119,9 +119,9 @@ private void thenIsValid(ValidationResponse response) {

private void thenIsNotValid(ValidationResponse response) {
assertEquals(response.type, SuccessOrFailure.Success);
final ValidationResults results = assertDoesNotThrow(() -> response.results.get());
final ValidationSuccessResponse success = assertDoesNotThrow(() -> response.success.get());
assertFalse(
results.validationErrors.isEmpty(),
success.validationErrors.isEmpty(),
() -> {
return "Expected validation errors but did not find any";
}
Expand Down

0 comments on commit 9350281

Please sign in to comment.