Skip to content

Add method to update code_owner_approval_required flag to Protected Branches API #870

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.

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

Merged
merged 4 commits into from
Dec 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ local.properties
.classpath
.settings/
.loadpath

### Visual Studio Code ###
.vscode

# External tool builders
.externalToolBuilders/
Expand All @@ -45,7 +48,7 @@ target/

# Test properties file for gitlab4j
test-gitlab4j.properties
!src/test/resoures/test-gitlab4j.properties
!src/test/resources/test-gitlab4j.properties

# git-changelog plugin #
.okhttpcache
36 changes: 36 additions & 0 deletions src/main/java/org/gitlab4j/api/AbstractApi.java
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,42 @@ protected Response head(Response.Status expectedStatus, MultivaluedMap<String, S
}
}

/**
* Perform an HTTP PATCH call with the specified query parameters and path objects, returning
* a ClientResponse instance with the data returned from the endpoint.
*
* @param expectedStatus the HTTP status that should be returned from the server
* @param queryParams multivalue map of request parameters
* @param pathArgs variable list of arguments used to build the URI
* @return a ClientResponse instance with the data returned from the endpoint
* @throws GitLabApiException if any exception occurs during execution
*/
protected Response patch(Response.Status expectedStatus, MultivaluedMap<String, String> queryParams, Object... pathArgs) throws GitLabApiException {
try {
return validate(getApiClient().patch(queryParams, pathArgs), expectedStatus);
} catch (Exception e) {
throw handle(e);
}
}

/**
* Perform an HTTP PATCH call with the specified query parameters and URL, returning
* a ClientResponse instance with the data returned from the endpoint.
*
* @param expectedStatus the HTTP status that should be returned from the server
* @param queryParams multivalue map of request parameters
* @param url the fully formed path to the GitLab API endpoint
* @return a ClientResponse instance with the data returned from the endpoint
* @throws GitLabApiException if any exception occurs during execution
*/
protected Response patch(Response.Status expectedStatus, MultivaluedMap<String, String> queryParams, URL url) throws GitLabApiException {
try {
return validate(getApiClient().patch(queryParams, url), expectedStatus);
} catch (Exception e) {
throw handle(e);
}
}

/**
* Perform an HTTP POST call with the specified form data and path objects, returning
* a ClientResponse instance with the data returned from the endpoint.
Expand Down
35 changes: 32 additions & 3 deletions src/main/java/org/gitlab4j/api/GitLabApiClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,8 @@ void enableRequestResponseLogging(Logger logger, Level level, int maxEntityLengt
* @param readTimeout the per request read timeout in milliseconds, can be null to use default
*/
void setRequestTimeout(Integer connectTimeout, Integer readTimeout) {
this.connectTimeout = connectTimeout;
this.readTimeout = readTimeout;
this.connectTimeout = connectTimeout;
this.readTimeout = readTimeout;
}

/**
Expand Down Expand Up @@ -470,6 +470,35 @@ protected Response head(MultivaluedMap<String, String> queryParams, URL url) {
return (invocation(url, queryParams).head());
}

/**
* Perform an HTTP PATCH call with the specified query parameters and path objects, returning
* a ClientResponse instance with the data returned from the endpoint.
*
* @param queryParams multivalue map of request parameters
* @param pathArgs variable list of arguments used to build the URI
* @return a ClientResponse instance with the data returned from the endpoint
* @throws IOException if an error occurs while constructing the URL
*/
protected Response patch(MultivaluedMap<String, String> queryParams, Object... pathArgs) throws IOException {
URL url = getApiUrl(pathArgs);
return (patch(queryParams, url));
}

/**
* Perform an HTTP PATCH call with the specified query parameters and URL, returning
* a ClientResponse instance with the data returned from the endpoint.
*
* @param queryParams multivalue map of request parameters
* @param url the fully formed path to the GitLab API endpoint
* @return a ClientResponse instance with the data returned from the endpoint
*/
protected Response patch(MultivaluedMap<String, String> queryParams, URL url) {
Entity<?> empty = Entity.text("");
// use "X-HTTP-Method-Override" header on POST to override to unsupported PATCH
return (invocation(url, queryParams)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hello thanks for your PR.

I did not check if it works, but I think you can use invocation(url, queryParams).method("PATCH", empty);

Can you try with my proposal ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, @jabby. In fact I tried that first (after not finding a native patch() method) and found it doesn't work with the current client, and instead returns the following exception:

org.gitlab4j.api.GitLabApiException: java.net.ProtocolException: Invalid HTTP method: PATCH

There is some discussion of other workarounds for the underlying bug, but this approach seemed sufficient and less problematic to me. I'm open to pushback on that.

Also, to help in the future and at least exercise the method, I've added a test handling the current tested version with comments on what should happen with Premium.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any other questions on this, @jabby ?

.header("X-HTTP-Method-Override", "PATCH").post(empty));
}

/**
* Perform an HTTP POST call with the specified form data and path objects, returning
* a ClientResponse instance with the data returned from the endpoint.
Expand Down Expand Up @@ -918,7 +947,7 @@ public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngi
// Ignore differences between given hostname and certificate hostname
HostnameVerifier hostnameVerifier = new HostnameVerifier() {
@Override
public boolean verify(String hostname, SSLSession session) {
public boolean verify(String hostname, SSLSession session) {
return true;
}
};
Expand Down
52 changes: 38 additions & 14 deletions src/main/java/org/gitlab4j/api/ProtectedBranchesApi.java
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,8 @@ public ProtectedBranch protectBranch(Object projectIdOrPath, String branchName,
* @throws GitLabApiException if any exception occurs
*/
public ProtectedBranch protectBranch(Object projectIdOrPath, String branchName,
AccessLevel pushAccessLevel, AccessLevel mergeAccessLevel, AccessLevel unprotectAccessLevel,
Boolean codeOwnerApprovalRequired) throws GitLabApiException {
AccessLevel pushAccessLevel, AccessLevel mergeAccessLevel, AccessLevel unprotectAccessLevel,
Boolean codeOwnerApprovalRequired) throws GitLabApiException {
Form formData = new GitLabApiForm()
.withParam("name", branchName, true)
.withParam("push_access_level", pushAccessLevel)
Expand Down Expand Up @@ -184,10 +184,10 @@ public ProtectedBranch protectBranch(Object projectIdOrPath, String branchName,
* @throws GitLabApiException if any exception occurs
*/
public ProtectedBranch protectBranch(Object projectIdOrPath, String branchName,
Integer allowedToPushUserId, Integer allowedToMergeUserId, Integer allowedToUnprotectUserId,
Boolean codeOwnerApprovalRequired) throws GitLabApiException {
Integer allowedToPushUserId, Integer allowedToMergeUserId, Integer allowedToUnprotectUserId,
Boolean codeOwnerApprovalRequired) throws GitLabApiException {

Form formData = new GitLabApiForm()
Form formData = new GitLabApiForm()
.withParam("name", branchName, true)
.withParam("allowed_to_push[][user_id]", allowedToPushUserId)
.withParam("allowed_to_merge[][user_id]", allowedToMergeUserId)
Expand Down Expand Up @@ -215,22 +215,46 @@ public ProtectedBranch protectBranch(Object projectIdOrPath, String branchName,
* @throws GitLabApiException if any exception occurs
*/
public ProtectedBranch protectBranch(Object projectIdOrPath, String branchName,
AllowedTo allowedToPush, AllowedTo allowedToMerge, AllowedTo allowedToUnprotect,
Boolean codeOwnerApprovalRequired) throws GitLabApiException {
AllowedTo allowedToPush, AllowedTo allowedToMerge, AllowedTo allowedToUnprotect,
Boolean codeOwnerApprovalRequired) throws GitLabApiException {

GitLabApiForm formData = new GitLabApiForm()
.withParam("name", branchName, true)
.withParam("code_owner_approval_required", codeOwnerApprovalRequired);

if (allowedToPush != null)
allowedToPush.getForm(formData, "allowed_to_push");
if (allowedToMerge != null)
allowedToMerge.getForm(formData, "allowed_to_merge");
if (allowedToUnprotect != null)
allowedToUnprotect.getForm(formData, "allowed_to_unprotect");
if (allowedToPush != null)
allowedToPush.getForm(formData, "allowed_to_push");
if (allowedToMerge != null)
allowedToMerge.getForm(formData, "allowed_to_merge");
if (allowedToUnprotect != null)
allowedToUnprotect.getForm(formData, "allowed_to_unprotect");

Response response = post(Response.Status.CREATED, formData.asMap(),
Response response = post(Response.Status.CREATED, formData.asMap(),
"projects", getProjectIdOrPath(projectIdOrPath), "protected_branches");
return (response.readEntity(ProtectedBranch.class));
}

/**
* Sets the code_owner_approval_required flag on the specified protected branch.
*
* <p>NOTE: This method is only available in GitLab Premium or higher.</p>
*
* <pre><code>GitLab Endpoint: PATCH /projects/:id/protected_branches/:branch_name?code_owner_approval_required=true</code></pre>
*
* @param projectIdOrPath the project in the form of an Long(ID), String(path), or Project instance
* @param branchName the name of the branch to protect, can be a wildcard
* @param codeOwnerApprovalRequired prevent pushes to this branch if it matches an item in the CODEOWNERS file.
* @return the branch info for the protected branch
* @throws GitLabApiException if any exception occurs
*/
public ProtectedBranch setCodeOwnerApprovalRequired(Object projectIdOrPath, String branchName,
Boolean codeOwnerApprovalRequired) throws GitLabApiException {
Form formData = new GitLabApiForm()
.withParam("code_owner_approval_required", codeOwnerApprovalRequired);

Response response = patch(Response.Status.OK, formData.asMap(),
"projects", this.getProjectIdOrPath(projectIdOrPath),
"protected_branches", urlEncode(branchName));
return (response.readEntity(ProtectedBranch.class));
}
}
20 changes: 19 additions & 1 deletion src/test/java/org/gitlab4j/api/TestProtectedBranchesApi.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package org.gitlab4j.api;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrowsExactly;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assumptions.assumeTrue;

Expand Down Expand Up @@ -45,7 +47,7 @@ public class TestProtectedBranchesApi extends AbstractIntegrationTest {

@BeforeAll
public static void setup() {
// Must setup the connection to the GitLab test server and get the test Project instance
// Must setup the connection to the GitLab test server and get the test Project instance
gitLabApi = baseTestSetup();
testProject = getTestProject();
}
Expand Down Expand Up @@ -124,4 +126,20 @@ public void testProtectBranch() throws GitLabApiException {
assertTrue(branches.stream()
.anyMatch((protectedBranch) -> protectedBranch.getName().equals(TEST_BRANCH_NAME)));
}

@Test
public void testSetCodeOwnerApprovalRequired() throws GitLabApiException {

assumeTrue(testProject != null);

ProtectedBranch branch = gitLabApi.getProtectedBranchesApi().getProtectedBranch(testProject, TEST_BRANCH_NAME);
assertNotNull(branch);
// current version returns null, but will return boolean (false) with newer Premium
assertFalse(branch.getCodeOwnerApprovalRequired() != null);

// current version returns 404, but will return branch with "code_owner_approval_required = true" with newer Premium
GitLabApiException gae = assertThrowsExactly(GitLabApiException.class,
() -> gitLabApi.getProtectedBranchesApi().setCodeOwnerApprovalRequired(testProject, TEST_BRANCH_NAME, true));
assertTrue(gae.getHttpStatus() == 404);
}
}