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.

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

[JENKINS-65845] Don't link to deleted builds #1659

Merged
merged 2 commits into from
Jan 15, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -599,16 +599,16 @@ public String getDisplayName() {
}

/**
* Returns a new sub page for the selected link.
* Returns a new subpage for the selected link.
*
* @param link
* the link to identify the sub page to show
* the link to identify the subpage to show
* @param request
* Stapler request
* @param response
* Stapler response
*
* @return the new sub page
* @return the new subpage
*/
@SuppressWarnings("unused") // Called by jelly view
public Object getDynamic(final String link, final StaplerRequest request, final StaplerResponse response) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import j2html.tags.DomContent;

import org.jvnet.localizer.Localizable;
import hudson.model.Job;
import hudson.model.Run;

import io.jenkins.plugins.analysis.core.util.QualityGateStatus;
Expand Down Expand Up @@ -115,7 +116,7 @@
* @return the age builder
*/
protected DefaultAgeBuilder getAgeBuilder(final Run<?, ?> owner, final String url) {
return new DefaultAgeBuilder(owner.getNumber(), url);
return new DefaultAgeBuilder(owner.getNumber(), url, owner.getParent());
}

/**
Expand Down Expand Up @@ -170,7 +171,8 @@
return this;
}

@Override @Generated
@Override
@Generated
public String toString() {
return String.format("%s: %s", getId(), getName());
}
Expand All @@ -190,7 +192,8 @@
* @return the name of the side panel link
* @deprecated use {@link #getLinkName()}
*/
@Deprecated @Generated
@Deprecated
@Generated
public String getRawLinkName() {
if (StringUtils.isNotBlank(name)) {
return Messages.Tool_Link_Name(name);
Expand Down Expand Up @@ -397,28 +400,53 @@
/**
* Computes the age of a build as a hyperlink.
*/
@SuppressWarnings("DeprecatedIsStillUsed")
public static class DefaultAgeBuilder implements AgeBuilder {
private final int currentBuild;
private final int currentBuildNumber;
private final String resultUrl;
@CheckForNull
private Job<?, ?> owner;

/**
* Creates a new instance of {@link DefaultAgeBuilder}.
*
* @param currentBuild
* @param currentBuildNumber
* number of the current build
* @param resultUrl
* URL to the results
* @deprecated use {@link #DefaultAgeBuilder(int, String, Job)}
*/
public DefaultAgeBuilder(final int currentBuild, final String resultUrl) {
this.currentBuild = currentBuild;
@Deprecated
public DefaultAgeBuilder(final int currentBuildNumber, final String resultUrl) {
this.currentBuildNumber = currentBuildNumber;
this.resultUrl = resultUrl;
}

/**
* Creates a new instance of {@link DefaultAgeBuilder}.
*
* @param currentBuildNumber
* number of the current build
* @param resultUrl
* URL to the results
* @param job
* the job
*/
public DefaultAgeBuilder(final int currentBuildNumber, final String resultUrl, final Job<?, ?> job) {
this(currentBuildNumber, resultUrl);

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation Note

Invoking
DefaultAgeBuilder.DefaultAgeBuilder
should be avoided because it has been deprecated.

this.owner = job;
}

@Override
public String apply(final Integer referenceBuild) {
if (referenceBuild >= currentBuild) {
if (referenceBuild >= currentBuildNumber) {
return "1"; // fallback
}
var referenceBuildId = String.valueOf(referenceBuild);
if (owner != null && owner.getBuild(referenceBuildId) == null) {
return computeAge(referenceBuild); // plain link
}
else {
String cleanUrl = StringUtils.stripEnd(resultUrl, "/");
int subDetailsCount = StringUtils.countMatches(cleanUrl, "/");
Expand All @@ -432,7 +460,7 @@
}

private String computeAge(final int buildNumber) {
return String.valueOf(currentBuild - buildNumber + 1);
return String.valueOf(currentBuildNumber - buildNumber + 1);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import edu.hm.hafner.analysis.IssueBuilder;
import edu.hm.hafner.analysis.Severity;

import hudson.model.Job;
import hudson.model.Run;

import io.jenkins.plugins.analysis.core.model.StaticAnalysisLabelProvider.DefaultAgeBuilder;
Expand Down Expand Up @@ -96,8 +97,15 @@
* @return a {@link DefaultAgeBuilder} stub
*/
protected DefaultAgeBuilder createAgeBuilder() {
return new DefaultAgeBuilder(1, "url");
return new DefaultAgeBuilder(1, "url", createProject());
}

private Job<?, ?> createProject() {
var job = mock(Job.class);
var run = mock(Run.class);
when(job.getBuild(anyString())).thenReturn(run);
return job;
}

Check warning on line 108 in plugin/src/test/java/io/jenkins/plugins/analysis/core/model/AbstractDetailsModelTest.java

View check run for this annotation

ci.jenkins.io / CPD

CPD

LOW: Found duplicated code.
Raw output
<pre><code>return new DefaultAgeBuilder(1, &#34;url&#34;, createProject()); } private Job&lt;?, ?&gt; createProject() { var job &#61; mock(Job.class); var run &#61; mock(Run.class); when(job.getBuild(anyString())).thenReturn(run); return job; }</code></pre>

protected void assertThatDetailedColumnContains(final DetailedCell<String> actualColumn,
final String expectedDisplayName, final String expectedSortOrder) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,15 @@

import edu.hm.hafner.analysis.IssueBuilder;

import hudson.model.Job;
import hudson.model.Run;

import io.jenkins.plugins.analysis.core.model.StaticAnalysisLabelProvider.AgeBuilder;
import io.jenkins.plugins.analysis.core.model.StaticAnalysisLabelProvider.CompositeLocalizable;
import io.jenkins.plugins.analysis.core.model.StaticAnalysisLabelProvider.DefaultAgeBuilder;

import static io.jenkins.plugins.analysis.core.assertions.Assertions.*;
import static org.mockito.Mockito.*;

/**
* Tests the class {@link StaticAnalysisLabelProvider}.
Expand Down Expand Up @@ -97,14 +101,21 @@
class AgeBuilderTest {
@Test
void shouldCreateAgeLinkForFirstBuild() {
AgeBuilder builder = new DefaultAgeBuilder(1, "checkstyle/");
AgeBuilder builder = new DefaultAgeBuilder(1, "checkstyle/", createProject());

assertThat(builder.apply(1)).isEqualTo("1");
}

private Job<?, ?> createProject() {
var job = mock(Job.class);
var run = mock(Run.class);
when(job.getBuild(anyString())).thenReturn(run);
return job;
}

Check warning on line 114 in plugin/src/test/java/io/jenkins/plugins/analysis/core/model/StaticAnalysisLabelProviderTest.java

View check run for this annotation

ci.jenkins.io / CPD

CPD

LOW: Found duplicated code.
Raw output
<pre><code>return new DefaultAgeBuilder(1, &#34;url&#34;, createProject()); } private Job&lt;?, ?&gt; createProject() { var job &#61; mock(Job.class); var run &#61; mock(Run.class); when(job.getBuild(anyString())).thenReturn(run); return job; }</code></pre>

@Test
void shouldCreateAgeLinkForPreviousBuilds() {
AgeBuilder builder = new DefaultAgeBuilder(10, "checkstyle/");
AgeBuilder builder = new DefaultAgeBuilder(10, "checkstyle/", createProject());
assertThat(builder.apply(1))
.isEqualTo("<a href=\"../../1/checkstyle\">10</a>");
assertThat(builder.apply(9))
Expand All @@ -113,9 +124,17 @@
.isEqualTo("1");
}

@Test @Issue("JENKINS-65845")
void shouldCreatePlainTextForDeletedBuilds() {
AgeBuilder builder = new DefaultAgeBuilder(10, "checkstyle/", mock(Job.class));
assertThat(builder.apply(1)).isEqualTo("10");
assertThat(builder.apply(9)).isEqualTo("2");
assertThat(builder.apply(10)).isEqualTo("1");
}

@Test
void shouldCreateAgeLinkForSubDetails() {
AgeBuilder builder = new DefaultAgeBuilder(10, "checkstyle/package.1234/");
AgeBuilder builder = new DefaultAgeBuilder(10, "checkstyle/package.1234/", createProject());
assertThat(builder.apply(1))
.isEqualTo("<a href=\"../../../1/checkstyle\">10</a>");
assertThat(builder.apply(9))
Expand Down
Loading