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

Jump to bottom

Extend relative hash support to whole API v2 #7308

Merged
merged 12 commits into from
Aug 1, 2023
Merged

Extend relative hash support to whole API v2 #7308

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
3dc398f
Changes to the model API
adutra Jul 31, 2023
8746ea7
Changes to Service layer
adutra Jul 31, 2023
e4afc0b
Changes to Nessie specs
adutra Jul 31, 2023
243db21
Tests for relative hashes
adutra Jul 31, 2023
c8ffca1
Update CHANGELOG.md
adutra Jul 31, 2023
66b10eb
Attempt to fix NesqueIT failures
adutra Jul 31, 2023
ed99173
Fix validation messages
adutra Jul 31, 2023
6f4a1bc
Resolve hashes against DETACHED
adutra Aug 1, 2023
f1392f7
Add TODO for when Nessie lib will be >= 0.61
adutra Aug 1, 2023
5b214dc
Fix javadocs
adutra Aug 1, 2023
8e945f2
Address review comments
adutra Aug 1, 2023
4d41eea
More code reviews
adutra Aug 1, 2023
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ as necessary. Empty sections will not end in the release notes.
### Highlights

### Upgrade notes
- This release upgrades the Nessie API spec to 2.1.1.
snazy marked this conversation as resolved.
Show resolved Hide resolved

### Breaking changes

### New Features

### Changes
- Support for relative hashes has been standardized and is now allowed in all v2 endpoints

### Deprecations

Expand Down
12 changes: 12 additions & 0 deletions api/NESSIE-SPEC-2-0.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,18 @@ well-specified behaviours.

Refer to the [Nessie API documentation](./README.md) for the meaning of Nessie-specific terms.

# 2.1.1

* Released with Nessie version 0.67.0.
* Support for relative hashes was extended to the entire v2 API.
* Path parameters and request entities, such as `Reference`, `Merge` and `Transplant`, now
consistently support relative hashes.
* Ambiguous hashes (that is, hashes that are implicitly resolved against the current HEAD, e.g.
`~1`) are not allowed in writing operations.
* API v2 `GetReferenceByName` endpoint now returns a `BAD_REQUEST` error if the reference name
contains any hash (absolute or relative). Previously, the server would silently ignore the
hash and return the reference if it existed.

# 2.1.0

* Released with Nessie version 0.61.0.
Expand Down
13 changes: 11 additions & 2 deletions api/model/src/main/java/org/projectnessie/api/v2/http/HttpTreeApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

import com.fasterxml.jackson.annotation.JsonView;
import java.util.List;
import javax.validation.constraints.Pattern;
import javax.ws.rs.BeanParam;
import javax.ws.rs.Consumes;
import javax.ws.rs.DELETE;
Expand Down Expand Up @@ -71,6 +72,7 @@
import org.projectnessie.model.Reference;
import org.projectnessie.model.ReferencesResponse;
import org.projectnessie.model.SingleReferenceResponse;
import org.projectnessie.model.Validation;
import org.projectnessie.model.ser.Views;

@Consumes(MediaType.APPLICATION_JSON)
Expand Down Expand Up @@ -268,8 +270,8 @@ EntriesResponse getEntries(
@jakarta.ws.rs.GET
@Produces(MediaType.APPLICATION_JSON)
@jakarta.ws.rs.Produces(jakarta.ws.rs.core.MediaType.APPLICATION_JSON)
@Path("{ref}/history")
@jakarta.ws.rs.Path("{ref}/history")
@Path("{ref:" + REF_NAME_PATH_ELEMENT_REGEX + "}/history")
@jakarta.ws.rs.Path("{ref:" + REF_NAME_PATH_ELEMENT_REGEX + "}/history")
@Operation(
summary = "Get commit log for a particular reference",
description =
Expand Down Expand Up @@ -603,6 +605,7 @@ ContentResponse getContent(
@JsonView(Views.V2.class)
GetMultipleContentsResponse getSeveralContents(
@Parameter(
schema = @Schema(pattern = REF_NAME_PATH_ELEMENT_REGEX),
description = REF_PARAMETER_DESCRIPTION,
examples = {
@ExampleObject(ref = "ref"),
Expand All @@ -624,6 +627,12 @@ GetMultipleContentsResponse getSeveralContents(
@ExampleObject(ref = "refDefault"),
@ExampleObject(ref = "refDetached"),
})
@Pattern(
regexp = Validation.REF_NAME_PATH_REGEX,
message = Validation.REF_NAME_PATH_MESSAGE)
@jakarta.validation.constraints.Pattern(
regexp = Validation.REF_NAME_PATH_REGEX,
message = Validation.REF_NAME_PATH_MESSAGE)
@PathParam("ref")
@jakarta.ws.rs.PathParam("ref")
String ref,
Expand Down
12 changes: 8 additions & 4 deletions api/model/src/main/java/org/projectnessie/api/v2/params/CommitLogParams.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,16 @@
*/
package org.projectnessie.api.v2.params;

import static org.projectnessie.model.Validation.HASH_OR_RELATIVE_COMMIT_SPEC_MESSAGE;
import static org.projectnessie.model.Validation.HASH_OR_RELATIVE_COMMIT_SPEC_REGEX;

import javax.annotation.Nullable;
import javax.validation.constraints.Pattern;
import javax.ws.rs.QueryParam;
import org.eclipse.microprofile.openapi.annotations.media.ExampleObject;
import org.eclipse.microprofile.openapi.annotations.parameters.Parameter;
import org.immutables.builder.Builder.Constructor;
import org.projectnessie.model.FetchOption;
import org.projectnessie.model.Validation;

/**
* The purpose of this class is to include optional parameters that can be passed to {@code
Expand All @@ -35,10 +37,12 @@ public class CommitLogParams extends AbstractParams<CommitLogParams> {

@Nullable
@jakarta.annotation.Nullable
@Pattern(regexp = Validation.HASH_REGEX, message = Validation.HASH_MESSAGE)
@Pattern(
regexp = HASH_OR_RELATIVE_COMMIT_SPEC_REGEX,
message = HASH_OR_RELATIVE_COMMIT_SPEC_MESSAGE)
@jakarta.validation.constraints.Pattern(
regexp = Validation.HASH_REGEX,
message = Validation.HASH_MESSAGE)
regexp = HASH_OR_RELATIVE_COMMIT_SPEC_REGEX,
message = HASH_OR_RELATIVE_COMMIT_SPEC_MESSAGE)
@Parameter(
description =
"Hash on the given ref to identify the commit where the operation of fetching the log "
Expand Down
23 changes: 14 additions & 9 deletions api/model/src/main/java/org/projectnessie/api/v2/params/Merge.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@
import static org.projectnessie.api.v2.doc.ApiDoc.FROM_REF_NAME_DESCRIPTION;
import static org.projectnessie.api.v2.doc.ApiDoc.KEY_MERGE_MODES_DESCRIPTION;
import static org.projectnessie.api.v2.doc.ApiDoc.RETURN_CONFLICTS_AS_RESULT_DESCRIPTION;
import static org.projectnessie.model.Validation.validateHash;
import static org.projectnessie.model.Validation.validateNoRelativeSpec;
import static org.projectnessie.model.Validation.validateHashOrRelativeSpec;

import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonInclude.Include;
Expand Down Expand Up @@ -89,24 +88,30 @@ public interface Merge extends BaseMergeTransplant {
@JsonInclude(Include.NON_NULL)
CommitMeta getCommitMeta();

/**
* The hash of the commit from {@linkplain #getFromRefName() the source ref} to merge.
*
* <p>Since Nessie spec 2.1.1, the hash can be absolute or relative.
*/
@NotBlank
@jakarta.validation.constraints.NotBlank
@Pattern(regexp = Validation.HASH_REGEX, message = Validation.HASH_MESSAGE)
@Pattern(
regexp = Validation.HASH_OR_RELATIVE_COMMIT_SPEC_REGEX,
message = Validation.HASH_OR_RELATIVE_COMMIT_SPEC_MESSAGE)
@jakarta.validation.constraints.Pattern(
regexp = Validation.HASH_REGEX,
message = Validation.HASH_MESSAGE)
regexp = Validation.HASH_OR_RELATIVE_COMMIT_SPEC_REGEX,
message = Validation.HASH_OR_RELATIVE_COMMIT_SPEC_MESSAGE)
String getFromHash();

/**
* Validation rule using {@link org.projectnessie.model.Validation#validateHash(String)}
* (String)}.
* Validation rule using {@link
* org.projectnessie.model.Validation#validateHashOrRelativeSpec(String)} (String)}.
*/
@Value.Check
default void checkHash() {
String hash = getFromHash();
if (hash != null) {
validateNoRelativeSpec(hash);
validateHash(hash);
validateHashOrRelativeSpec(hash);
}
}
}
27 changes: 20 additions & 7 deletions api/model/src/main/java/org/projectnessie/api/v2/params/Transplant.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,20 @@
import static org.projectnessie.api.v2.doc.ApiDoc.FROM_REF_NAME_DESCRIPTION;
import static org.projectnessie.api.v2.doc.ApiDoc.KEY_MERGE_MODES_DESCRIPTION;
import static org.projectnessie.api.v2.doc.ApiDoc.RETURN_CONFLICTS_AS_RESULT_DESCRIPTION;
import static org.projectnessie.model.Validation.validateHash;
import static org.projectnessie.model.Validation.validateNoRelativeSpec;
import static org.projectnessie.model.Validation.validateHashOrRelativeSpec;

import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import java.util.List;
import javax.annotation.Nullable;
import javax.validation.constraints.NotNull;
import javax.validation.constraints.Pattern;
import javax.validation.constraints.Size;
import org.eclipse.microprofile.openapi.annotations.enums.SchemaType;
import org.eclipse.microprofile.openapi.annotations.media.Schema;
import org.eclipse.microprofile.openapi.annotations.media.SchemaProperty;
import org.immutables.value.Value;
import org.projectnessie.model.Validation;

@Schema(
type = SchemaType.OBJECT,
Expand Down Expand Up @@ -71,23 +72,35 @@ public interface Transplant extends BaseMergeTransplant {
@jakarta.validation.constraints.Size(min = 1)
String getMessage();

/**
* Lists the hashes of commits that should be transplanted into the target branch.
snazy marked this conversation as resolved.
Show resolved Hide resolved
*
* <p>Since Nessie spec 2.1.1, hashes can be absolute or relative.
*/
@NotNull
@jakarta.validation.constraints.NotNull
@Size
@jakarta.validation.constraints.Size(min = 1)
List<String> getHashesToTransplant();
List<
@Pattern(
Copy link
Member

Choose a reason for hiding this comment

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

Just noting: I think I came across a constellation where annotated generic types didn't work. But since there's a test for this, it seems to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I manually validated that this annotation works both with Jaxrs/Weld and Quarkus.

(For Jaxrs, I added Hibernate Validator recently in #7193, for Quarkus it's already present in quarkus-common.)

The problem is, the bean is being validated by the @Check method too, and that method is executed first.

If you remove the @Check method, then you would observe how Hibernate Validator validates that:

transplantCommitsIntoBranch.transplant.hashesToTransplant[0].<list element>: Hash with optional relative part must consist of either ... [etc.]

To be fair, I think we don't need the @Check method anymore, and besides, the HV error message is more explicit because it mentions where the problem was found. But maybe keeping it is safer, because we own the validation code. Wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

Since HV is doing the job, I'm okay removing the check-method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK but let's do it in a follow-up PR. I fear that some tests could fail, e.g. if they expect an exact error message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#7316

regexp = Validation.HASH_OR_RELATIVE_COMMIT_SPEC_REGEX,
message = Validation.HASH_OR_RELATIVE_COMMIT_SPEC_MESSAGE)
@jakarta.validation.constraints.Pattern(
regexp = Validation.HASH_OR_RELATIVE_COMMIT_SPEC_REGEX,
message = Validation.HASH_OR_RELATIVE_COMMIT_SPEC_MESSAGE)
String>
getHashesToTransplant();

/**
* Validation rule using {@link org.projectnessie.model.Validation#validateHash(String)}
* (String)}.
* Validation rule using {@link
* org.projectnessie.model.Validation#validateHashOrRelativeSpec(String)} (String)}.
*/
@Value.Check
default void checkHashes() {
List<String> hashes = getHashesToTransplant();
if (hashes != null) {
for (String hash : hashes) {
validateNoRelativeSpec(hash);
validateHash(hash);
validateHashOrRelativeSpec(hash);
}
}
}
Expand Down
15 changes: 12 additions & 3 deletions api/model/src/main/java/org/projectnessie/model/Detached.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@
*/
package org.projectnessie.model;

import static org.projectnessie.model.Validation.validateHash;
import static org.projectnessie.model.Validation.validateHashOrRelativeSpec;

import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonTypeName;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import javax.annotation.Nullable;
import javax.validation.constraints.NotEmpty;
import javax.validation.constraints.Pattern;
import org.eclipse.microprofile.openapi.annotations.enums.SchemaType;
import org.eclipse.microprofile.openapi.annotations.media.Schema;
import org.immutables.value.Value;
Expand Down Expand Up @@ -52,6 +53,12 @@ default String getName() {
@NotEmpty
@jakarta.validation.constraints.NotEmpty
@Value.Parameter(order = 1)
@Pattern(
regexp = Validation.HASH_OR_RELATIVE_COMMIT_SPEC_REGEX,
message = Validation.HASH_OR_RELATIVE_COMMIT_SPEC_MESSAGE)
@jakarta.validation.constraints.Pattern(
regexp = Validation.HASH_OR_RELATIVE_COMMIT_SPEC_REGEX,
message = Validation.HASH_OR_RELATIVE_COMMIT_SPEC_MESSAGE)
String getHash();

@Nullable
Expand All @@ -60,11 +67,13 @@ default String getName() {
@Value.Parameter(order = 2)
ReferenceMetadata getMetadata();

/** Validation rule using {@link Validation#validateReferenceName(String)}. */
/** Validation rule using {@link Validation#validateHashOrRelativeSpec(String)}. */
@Value.Check
@Override
default void checkHash() {
validateHash(getHash());
// Note: a detached hash must have a start commit ID (absolute part); this is going to be
// checked by the service layer.
validateHashOrRelativeSpec(getHash());
}

@Override
Expand Down
9 changes: 6 additions & 3 deletions api/model/src/main/java/org/projectnessie/model/Reference.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/
package org.projectnessie.model;

import static org.projectnessie.model.Validation.validateHash;
import static org.projectnessie.model.Validation.validateHashOrRelativeSpec;

import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonInclude;
Expand Down Expand Up @@ -78,12 +78,15 @@ public interface Reference extends Base {
message = Validation.HASH_OR_RELATIVE_COMMIT_SPEC_MESSAGE)
String getHash();

/** Validation rule using {@link org.projectnessie.model.Validation#validateHash(String)}. */
/**
* Validation rule using {@link
* org.projectnessie.model.Validation#validateHashOrRelativeSpec(String)}.
*/
@Value.Check
default void checkHash() {
String hash = getHash();
if (hash != null) {
validateHash(hash);
validateHashOrRelativeSpec(hash);
}
}

Expand Down
Loading