Skip to content

Added missing attributes for ProjectApi #1237

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 6 commits into from
Apr 14, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
22 changes: 22 additions & 0 deletions gitlab4j-api/src/main/java/org/gitlab4j/api/ProjectApi.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.gitlab4j.api.models.ApprovalRuleParams;
import org.gitlab4j.api.models.AuditEvent;
import org.gitlab4j.api.models.Badge;
import org.gitlab4j.api.models.ContainerExpirationPolicyAttributes;
import org.gitlab4j.api.models.CustomAttribute;
import org.gitlab4j.api.models.Event;
import org.gitlab4j.api.models.FileUpload;
Expand Down Expand Up @@ -4727,4 +4728,25 @@ public List<Iteration> listProjectIterations(Object projectIdOrPath, IterationFi
get(Response.Status.OK, queryParams, "projects", getProjectIdOrPath(projectIdOrPath), "iterations");
return (response.readEntity(new GenericType<List<Iteration>>() {}));
}

/**
* Uploads and sets the project container expiration policy for the specified project.
*
* <pre><code>GitLab Endpoint: PUT /projects/:id</code></pre>
*
* @param projectIdOrPath the project in the form of an Long(ID), String(path), or Project instance, required
* @param containerExpirationPolicyAttributes the ContainerExpirationPolicyAttributes instance
* @return the updated Project instance
* @throws GitLabApiException if any exception occurs
*/
public Project setProjectContainerExpirationPolicy(
Object projectIdOrPath, ContainerExpirationPolicyAttributes containerExpirationPolicyAttributes)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why aren't you using just ContainerExpirationPolicy as parameter?

Inside the method, you could create a new Project and pass the parameter to it with withContainerExpirationPolicy(..)

If you serialise that project instance you will get the same result as with containerExpirationPolicyAttributes.toString() (since all the other fields will be null)

Copy link
Contributor Author

@yarisvt yarisvt Apr 4, 2025

Choose a reason for hiding this comment

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

I tried that before, but it does not work, I keep getting this error: container_expiration_policy_attributes is invalid

According to the documentation, container_expiration_policy_attributes is used in the create/edit API to modify the policy, while container_expiration_policy is used in the list API to list the policy.

Also, there is an example which updates the policy, which uses --header 'Content-Type: application/json;charset=UTF-8'

I think it doesn't work with withContainerExpirationPolicy(..). since that request eventually uses --header 'Content-Type: application/x-www-form-urlencoded'. But the container_expiration_policy_attributes is serialized as JSON:

{
  "cadence" : "1month",
  "enabled" : true,
  "keep_n" : 100,
  "older_than" : "7d",
  "name_regex" : ".*-development",
  "name_regex_keep" : ".*-main"
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

The gitlab REST API is very flexible and accept the request to be either a JSON body, a regular HTTP Form Data, query parameters, …

I understand why you need a different method setProjectContainerExpirationPolicy(..) because here you are sending a project update request where the Project is serialised as Body.

My point was that ContainerExpirationPolicyAttributes model is not really necessary.

You could do something like (not tested):

    public Project setProjectContainerExpirationPolicy(Object projectIdOrPath, ContainerExpirationPolicy policy) {
Project request = new Project().withContainerExpirationPolicy(policy);
Response response = put(
                Response.Status.OK,
                request.toString(),
                "projects",
                getProjectIdOrPath(projectIdOrPath));
        return (response.readEntity(Project.class))
    }

I don't understand why this would not work.

For me request.toString() with request being a Project instance is the same as your containerExpirationPolicyAttributes.toString() where your containerExpirationPolicyAttributes instance is of type ContainerExpirationPolicyAttributes.

And it removes the need for a ContainerExpirationPolicyAttributes.

Copy link
Contributor Author

@yarisvt yarisvt Apr 9, 2025

Choose a reason for hiding this comment

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

Right, I now see that there is also withParam(String name, Map<String, ?> variables, boolean required) method in GitLabApiForm which correctly sets form fields for a Map. I can just use that with container_expiration_policy_attributes, that also works.

throws GitLabApiException {
Response response = put(
Response.Status.OK,
containerExpirationPolicyAttributes.toString(),
"projects",
getProjectIdOrPath(projectIdOrPath));
return (response.readEntity(Project.class));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,26 @@

import org.gitlab4j.models.utils.JacksonJson;

import com.fasterxml.jackson.annotation.JsonProperty;

public class ContainerExpirationPolicy implements Serializable {
private static final long serialVersionUID = 1L;

private String cadence;
private Boolean enabled;

@JsonProperty("keep_n")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Currently the way the models are written, we are not using @JsonProperty. There was an attempt in #1198, but it never was finished.

For consistency reason, I would not add this now.

Copy link
Contributor Author

@yarisvt yarisvt Apr 4, 2025

Choose a reason for hiding this comment

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

I had to use @JsonProperty, so that the snake_case variant is used, instead of the camelCase.

With @JsonProperty:

{
  "cadence" : "1month",
  "enabled" : true,
  "keep_n" : 100,
  "older_than" : "7d",
  "name_regex" : ".*-development",
  "name_regex_keep" : ".*-main"
}

Without @JsonProperty:

{
    "cadence" : "1month",
    "enabled" : true,
    "keepN" : 100,
    "olderThan" : "7d",
    "nameRegex" : ".*-development",
    "nameRegexKeep" : ".*-main"
  }

Additionally, I also saw it used in Project itself:

    @JsonProperty("_links")
    private Map<String, String> links;

private Integer keepN;

@JsonProperty("older_than")
private String olderThan;

@JsonProperty("name_regex")
private String nameRegex;

@JsonProperty("name_regex_keep")
private String nameRegexKeep;

private String nextRunAt;

public String getCadence() {
Expand All @@ -23,6 +34,11 @@ public void setCadence(String cadence) {
this.cadence = cadence;
}

public ContainerExpirationPolicy withCadence(String cadence) {
this.cadence = cadence;
return this;
}

public Boolean getEnabled() {
return enabled;
}
Expand All @@ -31,6 +47,11 @@ public void setEnabled(Boolean enabled) {
this.enabled = enabled;
}

public ContainerExpirationPolicy withEnabled(Boolean enabled) {
this.enabled = enabled;
return this;
}

public Integer getKeepN() {
return keepN;
}
Expand All @@ -39,6 +60,11 @@ public void setKeepN(Integer keepN) {
this.keepN = keepN;
}

public ContainerExpirationPolicy withKeepN(Integer keepN) {
this.keepN = keepN;
return this;
}

public String getOlderThan() {
return olderThan;
}
Expand All @@ -47,6 +73,11 @@ public void setOlderThan(String olderThan) {
this.olderThan = olderThan;
}

public ContainerExpirationPolicy withOlderThan(String olderThan) {
this.olderThan = olderThan;
return this;
}

public String getNameRegex() {
return nameRegex;
}
Expand All @@ -55,6 +86,11 @@ public void setNameRegex(String nameRegex) {
this.nameRegex = nameRegex;
}

public ContainerExpirationPolicy withNameRegex(String nameRegex) {
this.nameRegex = nameRegex;
return this;
}

public String getNameRegexKeep() {
return nameRegexKeep;
}
Expand All @@ -63,6 +99,11 @@ public void setNameRegexKeep(String nameRegexKeep) {
this.nameRegexKeep = nameRegexKeep;
}

public ContainerExpirationPolicy withNameRegexKeep(String nameRegexKeep) {
this.nameRegexKeep = nameRegexKeep;
return this;
}

public String getNextRunAt() {
return nextRunAt;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package org.gitlab4j.api.models;

import java.io.Serializable;

import org.gitlab4j.models.utils.JacksonJson;

import com.fasterxml.jackson.annotation.JsonProperty;

public class ContainerExpirationPolicyAttributes implements Serializable {
private static final long serialVersionUID = 1L;

@JsonProperty("container_expiration_policy_attributes")
private ContainerExpirationPolicy containerExpirationPolicyAttributes;

public ContainerExpirationPolicy getContainerExpirationPolicyAttributes() {
return containerExpirationPolicyAttributes;
}

public void setContainerExpirationPolicyAttributes(ContainerExpirationPolicy containerExpirationPolicyAttributes) {
this.containerExpirationPolicyAttributes = containerExpirationPolicyAttributes;
}

public ContainerExpirationPolicyAttributes withContainerExpirationPolicyAttributes(
ContainerExpirationPolicy containerExpirationPolicyAttributes) {
this.containerExpirationPolicyAttributes = containerExpirationPolicyAttributes;
return this;
}

@Override
public String toString() {
return (JacksonJson.toJsonString(this));
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you update the file project.json?
https://github.com/gitlab4j/gitlab4j-api/blob/fe14e34f0ab11e9f06f52765f86fbc0d72caa3dd/gitlab4j-models/src/test/resources/org/gitlab4j/models/project.json
Ideally with values coming from an existing from a real REST call.

This way we would we would ensure that your changes are correctly serialized and deserialized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some properties are only configurable with a Premium/Ultimate gitlab server, which I unfortenately do not have access to, so I've used an accepted value based on the documentation:

option type value I used
use_custom_template boolean false
external_authorization_classification_label string "label"
mirror_trigger_builds boolean false
only_allow_merge_if_all_status_checks_passed boolean false
group_with_project_templates_id integer 1

Copy link
Collaborator

Choose a reason for hiding this comment

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

Some properties are only configurable with a Premium/Ultimate gitlab server, which I unfortenately do not have access to

Yes this is a pity… even for working on gitlab4j there is no good way to access such a license (or I didn't asked at the correct place)

Original file line number Diff line number Diff line change
Expand Up @@ -1282,6 +1282,11 @@ public void setContainerExpirationPolicy(ContainerExpirationPolicy containerExpi
this.containerExpirationPolicy = containerExpirationPolicy;
}

public Project withContainerExpirationPolicy(ContainerExpirationPolicy containerExpirationPolicy) {
this.containerExpirationPolicy = containerExpirationPolicy;
return this;
}

public Boolean getServiceDeskEnabled() {
return serviceDeskEnabled;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
"enabled" : true,
"keep_n" : 1,
"older_than" : "14d",
"name_regex" : "",
"name_regex" : ".*-development",
"name_regex_keep" : ".*-main",
"next_run_at" : "2022-06-25T17:11:26.865Z"
},
Expand Down