RFR: 1603: Make labels handling consistent in all Issue implementations

[Erik Joelsson]

On Fri, 30 Sep 2022 20:18:42 GMT, Zhao Song <duke at openjdk.org> wrote:

> Make TestIssue and JiraIssue update the cache when updating labels.

issuetracker/src/main/java/org/openjdk/skara/issuetracker/jira/JiraIssue.java line 287:

> 285:     @Override
> 286:     public void setLabels(List<String> labels) {
> 287:         this.labels = null;

Both `GitHubPullRequest` and `GitHubMergeRequest` update the internal `labels` field in this method instead of making it null.

issuetracker/src/main/java/org/openjdk/skara/issuetracker/jira/JiraIssue.java line 301:

> 299:     @Override
> 300:     public List<Label> labels() {
> 301:         if(labels == null){

Suggestion:

        if (labels == null) {

issuetracker/src/main/java/org/openjdk/skara/issuetracker/jira/JiraIssue.java line 302:

> 300:     public List<Label> labels() {
> 301:         if(labels == null){
> 302:             json = request.get("").execute();

I don't think it's a good idea to have `JiraIssue::labels` sometimes update the whole json object. Just fetch it to a local variable and keep the `json` field final.

test/src/main/java/org/openjdk/skara/test/TestIssue.java line 50:

> 48:     protected final String title;
> 49:     protected final State state;
> 50:     // Mimic JiraIssue where labels are part of the main JSON object

This comment is now outdated.
Suggestion:

    // Labels are cached but still kept up to date

test/src/main/java/org/openjdk/skara/test/TestIssue.java line 200:

> 198:     @Override
> 199:     public void setLabels(List<String> labels) {
> 200:         this.labels = null;

This method should update the `labels` field instead of setting it to null.

-------------

PR: https://git.openjdk.org/skara/pull/1387