Skip to content

Commit

Permalink
Fix OpenAPI spec for DiffApi (#5584)
Browse files Browse the repository at this point in the history
* Avoid using fake {f} blocks in the query path spec.
  Both Swagger and OpenAPI Generator flag that as a validation error:
  "Declared path parameter "f" needs to be defined as a path
  parameter at either the path or operation level"

* Move @PathParam annotations for DiffParams from fields to setters
  and use custom parsing code for breaking "ref with hash" parameters
  into the ref name + hash.
  • Loading branch information
dimas-b authored Nov 29, 2022
1 parent e175ca0 commit 0bf34cd
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ public interface HttpDiffApi extends DiffApi {

@GET
@Produces(MediaType.APPLICATION_JSON)
@Path(
"{fromRef : [^*]+}{f : [*]?}{fromHashOnRef : ([^.]*)?}...{toRef : [^*]+}{t : [*]?}{toHashOnRef : ([^.]*)?}")
@Path("{fromRefWithHash}...{toRefWithHash}")
@Operation(
summary = "Get a diff for two given references",
description =
Expand All @@ -49,9 +48,9 @@ public interface HttpDiffApi extends DiffApi {
+ "\n"
+ "Examples: \n"
+ " diffs/main...myBranch\n"
+ " diffs/main...myBranch*1234567890123456\n"
+ " diffs/main*1234567890123456...myBranch\n"
+ " diffs/main*1234567890123456...myBranch*1234567890123456\n")
+ " diffs/main...myBranch\\*1234567890123456\n"
+ " diffs/main\\*1234567890123456...myBranch\n"
+ " diffs/main\\*1234567890123456...myBranch\\*1234567890123456\n")
@APIResponses({
@APIResponse(
responseCode = "200",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,36 +28,34 @@ public class DiffParams {

public static final String HASH_OPTIONAL_REGEX = "(" + Validation.HASH_REGEX + ")?";

private static final char HASH_SEPARATOR = '*';

@NotNull
@Pattern(regexp = Validation.REF_NAME_REGEX, message = Validation.REF_NAME_MESSAGE)
@Parameter(
description = "The 'from' reference to start the diff from",
examples = {@ExampleObject(ref = "ref")})
@PathParam("fromRef")
private String fromRef;

@Nullable
@Pattern(regexp = HASH_OPTIONAL_REGEX, message = Validation.HASH_MESSAGE)
@Parameter(
description = "Optional hash on the 'from' reference to start the diff from",
examples = {@ExampleObject(ref = "hash")})
@PathParam("fromHashOnRef")
private String fromHashOnRef;

@NotNull
@Pattern(regexp = Validation.REF_NAME_REGEX, message = Validation.REF_NAME_MESSAGE)
@Parameter(
description = "The 'to' reference to end the diff at.",
examples = {@ExampleObject(ref = "ref")})
@PathParam("toRef")
private String toRef;

@Nullable
@Pattern(regexp = HASH_OPTIONAL_REGEX, message = Validation.HASH_MESSAGE)
@Parameter(
description = "Optional hash on the 'to' reference to end the diff at.",
examples = {@ExampleObject(ref = "hash")})
@PathParam("toHashOnRef")
private String toHashOnRef;

public DiffParams() {}
Expand All @@ -74,6 +72,28 @@ public DiffParams() {}
this.toHashOnRef = toHashOnRef;
}

@PathParam("fromRefWithHash")
public void setFromRefWithHash(String value) {
this.fromRef = parseRefName(value);
this.fromHashOnRef = parseHash(value);
}

@PathParam("toRefWithHash")
public void setToRefWithHash(String value) {
this.toRef = parseRefName(value);
this.toHashOnRef = parseHash(value);
}

private String parseRefName(String param) {
int idx = param.indexOf(HASH_SEPARATOR);
return idx == 0 ? null : idx < 0 ? param : param.substring(0, idx);
}

private String parseHash(String param) {
int idx = param.indexOf(HASH_SEPARATOR);
return idx < 0 ? null : param.substring(idx + 1);
}

public String getFromRef() {
return fromRef;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.parallel.Execution;
import org.junit.jupiter.api.parallel.ExecutionMode;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;

@Execution(ExecutionMode.CONCURRENT)
public class DiffParamsTest {
Expand Down Expand Up @@ -69,4 +71,24 @@ public void testValidation() {
.isInstanceOf(IllegalStateException.class)
.hasMessage("Cannot build DiffParams, some of required attributes are not set [fromRef]");
}

@ParameterizedTest
@CsvSource({
"main*1122334455667788,main,1122334455667788",
"main,main,",
"main/,main/,",
"main/*1122334455667788,main/,1122334455667788",
"*1122334455667788,,1122334455667788",
"*,,",
"main*,main,",
})
public void testParameterParsing(String param, String expectedRefName, String expectedHash) {
DiffParams diffParams = new DiffParams();
diffParams.setFromRefWithHash(param);
diffParams.setToRefWithHash(param);
assertThat(diffParams.getFromRef()).isEqualTo(expectedRefName);
assertThat(diffParams.getToRef()).isEqualTo(expectedRefName);
assertThat(diffParams.getFromHashOnRef()).isEqualTo(expectedHash);
assertThat(diffParams.getToHashOnRef()).isEqualTo(expectedHash);
}
}

0 comments on commit 0bf34cd

Please sign in to comment.