Skip to content

Commit

Permalink
Add for-write query parameter to Nessie API v2
Browse files Browse the repository at this point in the history
Adds the ability to ask Nessie to check for create/write access when requesting contents.

This allows an easier implementation of projectnessie#8736
  • Loading branch information
snazy committed Jun 20, 2024
1 parent e25e9a9 commit 3cb2b7a
Show file tree
Hide file tree
Showing 19 changed files with 99 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ public interface GetContentBuilder extends OnReferenceBuilder<GetContentBuilder>

GetContentBuilder keys(List<ContentKey> keys);

/** If set to {@code true}, check for write/create access in addition to read access. */
GetContentBuilder forWrite(boolean forWrite);

ContentResponse getSingle(@Valid @jakarta.validation.Valid ContentKey key)
throws NessieNotFoundException;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ public abstract class BaseGetContentBuilder extends BaseOnReferenceBuilder<GetCo
protected final ImmutableGetMultipleContentsRequest.Builder request =
ImmutableGetMultipleContentsRequest.builder();

protected boolean forWrite;

@Override
public GetContentBuilder key(ContentKey key) {
request.addRequestedKeys(key);
Expand All @@ -37,4 +39,10 @@ public GetContentBuilder keys(List<ContentKey> keys) {
request.addAllRequestedKeys(keys);
return this;
}

@Override
public GetContentBuilder forWrite(boolean forWrite) {
this.forWrite = forWrite;
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public abstract class BaseGetEntriesBuilder<PARAMS>
protected ContentKey prefixKey;
protected String filter;
protected boolean withContent;
protected boolean forWrite;

protected BaseGetEntriesBuilder(BiFunction<PARAMS, String, PARAMS> paramsForPage) {
this.paramsForPage = paramsForPage;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import java.util.Map;
import java.util.stream.Collectors;
import org.projectnessie.client.api.GetContentBuilder;
import org.projectnessie.client.builder.BaseGetContentBuilder;
import org.projectnessie.error.NessieNotFoundException;
import org.projectnessie.model.Content;
Expand Down Expand Up @@ -51,4 +52,12 @@ public GetMultipleContentsResponse getWithResponse() {
throw new UnsupportedOperationException(
"Extended contents response data is not available in API v1");
}

@Override
public GetContentBuilder forWrite(boolean forWrite) {
if (forWrite) {
throw new UnsupportedOperationException("forWrite is not available in API v1");
}
return super.forWrite(forWrite);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ public ContentResponse getSingle(ContentKey key) throws NessieNotFoundException
.path("trees/{ref}/contents/{key}")
.resolveTemplate("ref", Reference.toPathString(refName, hashOnRef))
.resolveTemplate("key", key.toPathString())
.queryParam("for-write", forWrite ? "true" : null)
.unwrap(NessieNotFoundException.class)
.get()
.readEntity(ContentResponse.class);
Expand All @@ -59,6 +60,7 @@ public GetMultipleContentsResponse getWithResponse() throws NessieNotFoundExcept
.newRequest()
.path("trees/{ref}/contents")
.resolveTemplate("ref", Reference.toPathString(refName, hashOnRef))
.queryParam("for-write", forWrite ? "true" : null)
.unwrap(NessieNotFoundException.class)
.post(request.build())
.readEntity(GetMultipleContentsResponse.class);
Expand Down
12 changes: 9 additions & 3 deletions api/model/src/main/java/org/projectnessie/api/v2/TreeApi.java
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,8 @@ CommitResponse commitMultipleOperations(
* @param key the {@link ContentKey}s to retrieve
* @param ref named-reference to retrieve the content for
* @param withDocumentation whether to return the documentation, if it exists.
* @param forWrite If set to 'true', access control checks will check for write/create privilege
* in addition to read.
* @return list of {@link GetMultipleContentsResponse.ContentWithKey}s
* @throws NessieNotFoundException if {@code ref} or {@code hashOnRef} does not exist
*/
Expand All @@ -341,11 +343,12 @@ ContentResponse getContent(
regexp = Validation.REF_NAME_PATH_REGEX,
message = Validation.REF_NAME_PATH_MESSAGE)
String ref,
boolean withDocumentation)
boolean withDocumentation,
boolean forWrite)
throws NessieNotFoundException;

/**
* Similar to {@link #getContent(ContentKey, String, boolean)}, but takes multiple {@link
* Similar to {@link #getContent(ContentKey, String, boolean, boolean)}, but takes multiple {@link
* ContentKey}s and returns the {@link Content} for the one or more {@link ContentKey}s in a
* named-reference (a {@link org.projectnessie.model.Branch} or {@link
* org.projectnessie.model.Tag}).
Expand All @@ -358,6 +361,8 @@ ContentResponse getContent(
* @param ref named-reference to retrieve the content for
* @param request the {@link ContentKey}s to retrieve
* @param withDocumentation whether to return the documentation, if it exists.
* @param forWrite If set to 'true', access control checks will check for write/create privilege
* in addition to read.
* @return list of {@link GetMultipleContentsResponse.ContentWithKey}s
* @throws NessieNotFoundException if {@code ref} or {@code hashOnRef} does not exist
*/
Expand All @@ -373,6 +378,7 @@ GetMultipleContentsResponse getMultipleContents(
String ref,
@Valid @jakarta.validation.Valid @NotNull @jakarta.validation.constraints.NotNull
GetMultipleContentsRequest request,
boolean withDocumentation)
boolean withDocumentation,
boolean forWrite)
throws NessieNotFoundException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ public interface ApiDoc {
String WITH_DOC_PARAMETER_DESCRIPTION =
"Whether to return the documentation, if it exists. Default is to not return the documentation.";

String FOR_WRITE_PARAMETER_DESCRIPTION =
"If set to 'true', access control checks will check for write/create privilege in addition to read.";

String CHECKED_REF_DESCRIPTION =
"Specifies a named branch or tag reference with its expected HEAD 'hash' value.\n"
+ "\n"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static org.projectnessie.api.v2.doc.ApiDoc.CHECKED_REF_DESCRIPTION;
import static org.projectnessie.api.v2.doc.ApiDoc.CHECKED_REF_INFO;
import static org.projectnessie.api.v2.doc.ApiDoc.COMMIT_BRANCH_DESCRIPTION;
import static org.projectnessie.api.v2.doc.ApiDoc.FOR_WRITE_PARAMETER_DESCRIPTION;
import static org.projectnessie.api.v2.doc.ApiDoc.KEY_PARAMETER_DESCRIPTION;
import static org.projectnessie.api.v2.doc.ApiDoc.MERGE_TRANSPLANT_BRANCH_DESCRIPTION;
import static org.projectnessie.api.v2.doc.ApiDoc.PAGING_INFO;
Expand Down Expand Up @@ -612,7 +613,11 @@ ContentResponse getContent(
@Parameter(description = WITH_DOC_PARAMETER_DESCRIPTION)
@QueryParam("with-doc")
@jakarta.ws.rs.QueryParam("with-doc")
boolean withDocumentation)
boolean withDocumentation,
@Parameter(description = FOR_WRITE_PARAMETER_DESCRIPTION)
@QueryParam("for-write")
@jakarta.ws.rs.QueryParam("for-write")
boolean forWrite)
throws NessieNotFoundException;

@GET
Expand Down Expand Up @@ -687,7 +692,11 @@ GetMultipleContentsResponse getSeveralContents(
@Parameter(description = WITH_DOC_PARAMETER_DESCRIPTION)
@QueryParam("with-doc")
@jakarta.ws.rs.QueryParam("with-doc")
boolean withDocumentation)
boolean withDocumentation,
@Parameter(description = FOR_WRITE_PARAMETER_DESCRIPTION)
@QueryParam("for-write")
@jakarta.ws.rs.QueryParam("for-write")
boolean forWrite)
throws NessieNotFoundException;

@Override
Expand Down Expand Up @@ -760,7 +769,11 @@ GetMultipleContentsResponse getMultipleContents(
@Parameter(description = WITH_DOC_PARAMETER_DESCRIPTION)
@QueryParam("with-doc")
@jakarta.ws.rs.QueryParam("with-doc")
boolean withDocumentation)
boolean withDocumentation,
@Parameter(description = FOR_WRITE_PARAMETER_DESCRIPTION)
@QueryParam("for-write")
@jakarta.ws.rs.QueryParam("for-write")
boolean forWrite)
throws NessieNotFoundException;

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,14 @@ private ContentService resource() {
@JsonView(Views.V1.class)
public Content getContent(ContentKey key, String ref, String hashOnRef)
throws NessieNotFoundException {
return resource().getContent(key, ref, hashOnRef, false).getContent();
return resource().getContent(key, ref, hashOnRef, false, false).getContent();
}

@Override
@JsonView(Views.V1.class)
public GetMultipleContentsResponse getMultipleContents(
String ref, String hashOnRef, GetMultipleContentsRequest request)
throws NessieNotFoundException {
return resource().getMultipleContents(ref, hashOnRef, request.getRequestedKeys(), false);
return resource().getMultipleContents(ref, hashOnRef, request.getRequestedKeys(), false, false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -337,34 +337,38 @@ private static Reference.ReferenceType parseReferenceType(String type) {

@JsonView(Views.V2.class)
@Override
public ContentResponse getContent(ContentKey key, String ref, boolean withDocumentation)
public ContentResponse getContent(
ContentKey key, String ref, boolean withDocumentation, boolean forWrite)
throws NessieNotFoundException {
ParsedReference reference = parseRefPathString(ref);
return content()
.getContent(key, reference.name(), reference.hashWithRelativeSpec(), withDocumentation);
.getContent(
key, reference.name(), reference.hashWithRelativeSpec(), withDocumentation, forWrite);
}

@JsonView(Views.V2.class)
@Override
public GetMultipleContentsResponse getSeveralContents(
String ref, List<String> keys, boolean withDocumentation) throws NessieNotFoundException {
String ref, List<String> keys, boolean withDocumentation, boolean forWrite)
throws NessieNotFoundException {
ImmutableGetMultipleContentsRequest.Builder request = GetMultipleContentsRequest.builder();
keys.forEach(k -> request.addRequestedKeys(ContentKey.fromPathString(k)));
return getMultipleContents(ref, request.build(), withDocumentation);
return getMultipleContents(ref, request.build(), withDocumentation, forWrite);
}

@JsonView(Views.V2.class)
@Override
public GetMultipleContentsResponse getMultipleContents(
String ref, GetMultipleContentsRequest request, boolean withDocumentation)
String ref, GetMultipleContentsRequest request, boolean withDocumentation, boolean forWrite)
throws NessieNotFoundException {
ParsedReference reference = parseRefPathString(ref);
return content()
.getMultipleContents(
reference.name(),
reference.hashWithRelativeSpec(),
request.getRequestedKeys(),
withDocumentation);
withDocumentation,
forWrite);
}

@JsonView(Views.V2.class)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,11 @@ public ContentApiImpl(

@Override
public ContentResponse getContent(
ContentKey key, String namedRef, String hashOnRef, boolean withDocumentation)
ContentKey key,
String namedRef,
String hashOnRef,
boolean withDocumentation,
boolean forWrite)
throws NessieNotFoundException {
try {
ResolvedHash ref =
Expand Down Expand Up @@ -86,7 +90,11 @@ public ContentResponse getContent(

@Override
public GetMultipleContentsResponse getMultipleContents(
String namedRef, String hashOnRef, List<ContentKey> keys, boolean withDocumentation)
String namedRef,
String hashOnRef,
List<ContentKey> keys,
boolean withDocumentation,
boolean forWrite)
throws NessieNotFoundException {
try {
ResolvedHash ref =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ ContentResponse getContent(
regexp = HASH_OR_RELATIVE_COMMIT_SPEC_REGEX,
message = HASH_OR_RELATIVE_COMMIT_SPEC_MESSAGE)
String hashOnRef,
boolean withDocumentation)
boolean withDocumentation,
boolean forWrite)
throws NessieNotFoundException;

GetMultipleContentsResponse getMultipleContents(
Expand All @@ -61,6 +62,7 @@ GetMultipleContentsResponse getMultipleContents(
message = HASH_OR_RELATIVE_COMMIT_SPEC_MESSAGE)
String hashOnRef,
@Valid @Size @jakarta.validation.constraints.Size(min = 1) List<ContentKey> keys,
boolean withDocumentation)
boolean withDocumentation,
boolean forWrite)
throws NessieNotFoundException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ public Map<Check, String> check() {
.isInstanceOf(AccessCheckException.class)
.hasMessageContaining(READ_MSG);
soft.assertThatThrownBy(
() -> contentApi().getContent(key, ref.getName(), ref.getHash(), false))
() -> contentApi().getContent(key, ref.getName(), ref.getHash(), false, false))
.describedAs("ref='%s', getContent", ref)
.isInstanceOf(AccessCheckException.class)
.hasMessageContaining(ENTITIES_MSG);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ public void commitLogPaging() throws BaseNessieClientServerException {
Put op;
try {
Content existing =
contentApi().getContent(key, branch.getName(), currentHash, false).getContent();
contentApi().getContent(key, branch.getName(), currentHash, false, false).getContent();
op = Put.of(key, IcebergTable.of("some-file-" + i, 42, 42, 42, 42, existing.getId()));
} catch (NessieNotFoundException notFound) {
op = Put.of(key, IcebergTable.of("some-file-" + i, 42, 42, 42, 42));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,8 @@ public void verifyContentAndOperationTypesIndividually(
// Compare content on HEAD commit with the committed content
soft.assertThat(
contentApi()
.getContent(fixedContentKey, committed.getName(), committed.getHash(), false))
.getContent(
fixedContentKey, committed.getName(), committed.getHash(), false, false))
.extracting(ContentResponse::getContent)
.extracting(this::clearIdOnContent)
.isEqualTo(put.getContent());
Expand All @@ -318,7 +319,8 @@ public void verifyContentAndOperationTypesIndividually(
soft.assertThatThrownBy(
() ->
contentApi()
.getContent(fixedContentKey, committed.getName(), committed.getHash(), false))
.getContent(
fixedContentKey, committed.getName(), committed.getHash(), false, false))
.isInstanceOf(NessieNotFoundException.class);

// Compare operation on HEAD commit with the committed operation
Expand All @@ -340,7 +342,8 @@ public void verifyContentAndOperationTypesIndividually(
// Compare content on HEAD commit with the committed content
soft.assertThat(
contentApi()
.getContent(fixedContentKey, committed.getName(), committed.getHash(), false))
.getContent(
fixedContentKey, committed.getName(), committed.getHash(), false, false))
.extracting(ContentResponse::getContent)
.extracting(this::clearIdOnContent)
.isEqualTo(contentAndOperationType.prepare.getContent());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ public void testUnknownHashesOnValidNamedRefs() throws BaseNessieClientServerExc
assertThatThrownBy(
() ->
contentApi()
.getContent(ContentKey.of("table0"), branch.getName(), invalidHash, false))
.getContent(
ContentKey.of("table0"), branch.getName(), invalidHash, false, false))
.isInstanceOf(NessieNotFoundException.class)
.hasMessageContaining(String.format("Commit '%s' not found", invalidHash));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ private void mergeTransplant(
table1 =
(IcebergTable)
contentApi()
.getContent(key1, committed1.getName(), committed1.getHash(), false)
.getContent(key1, committed1.getName(), committed1.getHash(), false, false)
.getContent();

Branch committed2 =
Expand Down Expand Up @@ -496,7 +496,7 @@ public void mergeWithNamespaces(ReferenceMode refMode) throws BaseNessieClientSe
table1 =
(IcebergTable)
contentApi()
.getContent(key1, committed1.getName(), committed1.getHash(), false)
.getContent(key1, committed1.getName(), committed1.getHash(), false, false)
.getContent();

Branch committed2 =
Expand Down Expand Up @@ -663,7 +663,8 @@ public void mergeRecreateTableNoConflict() throws BaseNessieClientServerExceptio

ContentResponse tableOnRootAfterMerge =
contentApi()
.getContent(setup.key, rootAfterMerge.getName(), rootAfterMerge.getHash(), false);
.getContent(
setup.key, rootAfterMerge.getName(), rootAfterMerge.getHash(), false, false);

soft.assertThat(setup.tableOnWork.getContent().getId())
.isEqualTo(tableOnRootAfterMerge.getContent().getId());
Expand Down Expand Up @@ -728,7 +729,7 @@ private MergeRecreateTableSetup setupMergeRecreateTable()
soft.assertThat(root).isNotEqualTo(lastRoot);

ContentResponse tableOnRoot =
contentApi().getContent(key, root.getName(), root.getHash(), false);
contentApi().getContent(key, root.getName(), root.getHash(), false, false);
soft.assertThat(tableOnRoot.getEffectiveReference()).isEqualTo(root);

Branch work = createBranch("recreateBranch", root);
Expand All @@ -748,7 +749,7 @@ private MergeRecreateTableSetup setupMergeRecreateTable()
soft.assertThat(work).isNotEqualTo(lastWork);

ContentResponse tableOnWork =
contentApi().getContent(key, work.getName(), work.getHash(), false);
contentApi().getContent(key, work.getName(), work.getHash(), false, false);
soft.assertThat(tableOnWork.getEffectiveReference()).isEqualTo(work);

soft.assertThat(tableOnWork.getContent().getId())
Expand Down
Loading

0 comments on commit 3cb2b7a

Please sign in to comment.