Skip to content

Commit

Permalink
Fix v2 URL path mapping for getReferenceByName (#5596)
Browse files Browse the repository at this point in the history
* Use the right RegEx constant for resolving path parameters for
  GET /trees/{ref}

* Other endpoints appear to use the right RegEx constant already.

* Correct URL path parameter RegEx to account for tools encoding the
  `@` char as `%40`.

* Drop the spurious `?` at the end of the RegEx

Fixes #584
  • Loading branch information
dimas-b authored Dec 2, 2022
1 parent fea0958 commit b15777b
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ public class GetReferenceParams {
examples = {@ExampleObject(ref = "ref")})
@PathParam("ref")
@NotNull
@Pattern(regexp = Validation.REF_NAME_REGEX, message = Validation.REF_NAME_MESSAGE)
private String refName;
@Pattern(regexp = Validation.REF_NAME_PATH_REGEX, message = Validation.REF_NAME_MESSAGE)
private String ref;

@Parameter(
description =
Expand All @@ -56,8 +56,8 @@ public class GetReferenceParams {
public GetReferenceParams() {}

@Constructor
GetReferenceParams(@NotNull String refName, @Nullable FetchOption fetchOption) {
this.refName = refName;
GetReferenceParams(@NotNull String ref, @Nullable FetchOption fetchOption) {
this.ref = ref;
this.fetchOption = fetchOption;
}

Expand All @@ -66,8 +66,8 @@ public FetchOption fetchOption() {
return fetchOption;
}

public String getRefName() {
return refName;
public String getRef() {
return ref;
}

public static GetReferenceParamsBuilder builder() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public final class Validation {
"^((" + HASH_RAW_REGEX + ")|(" + REF_NAME_RAW_REGEX + "))$";
public static final String REF_NAME_PATH_REGEX =
"^(" + REF_NAME_RAW_REGEX + "(@(" + HASH_RAW_REGEX + ")?)?|@" + HASH_RAW_REGEX + ")$";
public static final String REF_NAME_PATH_ELEMENT_REGEX = "([^/]+|[^@]+@[^@/]*?)";
public static final String REF_NAME_PATH_ELEMENT_REGEX = "([^/]+|[^@]+(@|%40)[^@/]*)";

public static final Pattern HASH_PATTERN = Pattern.compile(HASH_REGEX);
public static final Pattern REF_NAME_PATTERN = Pattern.compile(REF_NAME_REGEX);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,21 @@
import io.restassured.RestAssured;
import io.restassured.http.ContentType;
import io.restassured.specification.RequestSpecification;
import java.io.IOException;
import java.net.HttpURLConnection;
import java.net.URI;
import java.net.URL;
import java.util.stream.Stream;
import org.assertj.core.api.InstanceOfAssertFactories;
import org.assertj.core.data.MapEntry;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.projectnessie.client.ext.NessieApiVersion;
import org.projectnessie.client.ext.NessieApiVersions;
import org.projectnessie.client.ext.NessieClientUri;
import org.projectnessie.model.Branch;
import org.projectnessie.model.CommitMeta;
import org.projectnessie.model.CommitResponse;
Expand Down Expand Up @@ -153,4 +159,27 @@ void testGetSeveralContents(String branchName) {

assertThat(entries).containsExactlyInAnyOrder(entry(key1, "loc1"), entry(key2, "loc2"));
}

/** Dedicated test for human-readable references in URL paths. */
@Test
void testBranchWithSlashInUrlPath(@NessieClientUri URI clientUri) throws IOException {
Branch branch = createBranch("test/branch/name1");

assertThat(
rest()
.get("trees/test/branch/name1@")
.then()
.statusCode(200)
.extract()
.as(SingleReferenceResponse.class)
.getReference())
.isEqualTo(branch);

// The above request encodes `@` as `%40`, now try the same URL with a literal `@` char to
// simulate handwritten `curl` requests.
URL url = new URL(clientUri + "/trees/test/branch/name1@");
HttpURLConnection conn = (HttpURLConnection) url.openConnection();
assertThat(conn.getResponseCode()).isEqualTo(200);
conn.disconnect();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@

import io.quarkus.test.junit.QuarkusIntegrationTest;
import io.quarkus.test.junit.TestProfile;
import org.junit.jupiter.api.extension.ExtendWith;
import org.projectnessie.jaxrs.tests.AbstractResteasyV2Test;
import org.projectnessie.quarkus.tests.profiles.QuarkusTestProfileInmemory;

@QuarkusIntegrationTest
@TestProfile(QuarkusTestProfileInmemory.class)
@ExtendWith(QuarkusNessieClientResolver.class)
public class ITResteasyV2InMemory extends AbstractResteasyV2Test {}
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,9 @@ public SingleReferenceResponse createReference(
@Override
public SingleReferenceResponse getReferenceByName(GetReferenceParams params)
throws NessieNotFoundException {
Reference reference = Reference.fromPathString(params.getRef(), Reference.ReferenceType.BRANCH);
return SingleReferenceResponse.builder()
.reference(tree().getReferenceByName(params.getRefName(), params.fetchOption()))
.reference(tree().getReferenceByName(reference.getName(), params.fetchOption()))
.build();
}

Expand Down

0 comments on commit b15777b

Please sign in to comment.