Skip to content

Added numeric getById to RunMap and deprecate String API #26096

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.

Sign up for GitHub

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

Open
somiljain2006 wants to merge 17 commits into
base: master
Choose a base branch
from
+52 −10
Open

Added numeric getById to RunMap and deprecate String API #26096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
8c0a09a
Add numeric getById to RunMap and deprecate String API
somiljain2006 Jan 9, 2026
bab178b
Apply suggestion from @strangelookingnerd
somiljain2006 Jan 9, 2026
2c89075
Apply suggestion from @strangelookingnerd
somiljain2006 Jan 9, 2026
82c9403
Apply suggestion from @strangelookingnerd
somiljain2006 Jan 9, 2026
0ed7d9c
Apply suggestion from @strangelookingnerd
somiljain2006 Jan 9, 2026
e12ba9b
Fixed formatting errors
somiljain2006 Jan 9, 2026
bc15cbe
Trigger CI rerun
somiljain2006 Jan 10, 2026
dc93f89
Update core/src/main/java/hudson/model/RunMap.java
somiljain2006 Jan 12, 2026
eb5382f
Merge branch 'master' into Runmap-issue
somiljain2006 Jan 12, 2026
c7e5c9d
Reverting the misunderstood change
somiljain2006 Jan 12, 2026
ad7145b
Merge branch 'master' into Runmap-issue
somiljain2006 Jan 12, 2026
c95f973
Use optimized int lookup in LazyBuildMixIn
somiljain2006 Feb 4, 2026
c02c8bd
Merge branch 'master' into Runmap-issue
somiljain2006 Feb 4, 2026
32ef88f
Revert parsing logic in LazyBuildMixIn and suppress deprecation
somiljain2006 Feb 4, 2026
ba2aafc
Fied formatting errors
somiljain2006 Feb 4, 2026
499b4b2
Remove unnecessary deprecation annotation
somiljain2006 Feb 4, 2026
fd55f8e
Revert "Remove unnecessary deprecation annotation"
somiljain2006 Feb 4, 2026
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
28 changes: 24 additions & 4 deletions core/src/main/java/hudson/model/RunMap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
*
* <p>
* This class is multi-thread safe by using copy-on-write technique,
* and it also updates the bi-directional links within {@link Run}
* and it also updates the bidirectional links within {@link Run}
* accordingly.
*
* @author Kohsuke Kawaguchi
Expand Down Expand Up @@ -223,10 +223,31 @@ public R put(R r) {
return super._put(r);
}

/**
* Lookup a build by its numeric ID.
*
* @since TODO
Comment on lines +228 to +229
Copy link

Copilot AI Jan 12, 2026 •
edited by MarkEWaite
Loading

Choose a reason for hiding this comment

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

The @since tag is set to "TODO". This should be updated to the actual version number before merging. Please replace "TODO" with the appropriate Jenkins version number for this release.

Suggested change
*
* @since TODO

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@MarkEWaite MarkEWaite Jan 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

This comment by GitHub Copilot is incorrect. In the Jenkins project, we use @since TODO as a marker so that the release number can be updated after the first release that includes the new code.

It also would be much better if the markdown generated by GitHub Copilot used @since to show it is code, instead of referring to a GitHub user of that name. I corrected that mistake in the Copilot comment.

*/
@CheckForNull
@Override public R getById(String id) {
public R getById(int id) {
Copy link
Contributor

@strangelookingnerd strangelookingnerd Jan 16, 2026

Choose a reason for hiding this comment

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

While I don't think there is anything wrong with this change I wonder why it's not implemented in the AbstractLazyLazyRunMap or we just point the deprecation notice to #getByNumber?

@jglick Since you mentioned this in #10456 (comment) - any suggestions from you here?

Copy link
Member

@jglick jglick Feb 4, 2026

Choose a reason for hiding this comment

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

Dates to #1379.

return getByNumber(id);
}


/**
* Lookup a build by a String ID.
*
* @deprecated Use {@link #getById(int)} instead.
* Jenkins build IDs were sometimes date-time strings.
* This method preserves backward compatibility by attempting to parse
* the ID as an integer and returning null if parsing fails.
*/
@Deprecated
@CheckForNull
@Override
public R getById(String id) {
try {
return getByNumber(Integer.parseInt(id));
return getById(Integer.parseInt(id));
} catch (NumberFormatException e) { // see https://issues.jenkins.io/browse/JENKINS-75476
return null;
}
Expand Down Expand Up @@ -307,7 +328,6 @@ protected Class<R> getBuildClass() {

/**
* Backward compatibility method that notifies {@link RunMap} of who the owner is.
*
* Traditionally, this method blocked and loaded all the build records on the disk,
* but now all the actual loading happens lazily.
*
Expand Down
1 change: 1 addition & 0 deletions core/src/main/java/jenkins/model/lazy/LazyBuildMixIn.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ public final void removeRun(RunT run) {
/**
* Suitable for {@link Job#getBuild}.
*/
@SuppressWarnings("deprecation")
public final RunT getBuild(String id) {
return builds.getById(id);
}
Expand Down
33 changes: 27 additions & 6 deletions test/src/test/java/hudson/model/RunMapTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Objects;
import java.util.SortedMap;
import java.util.Spliterator;
import java.util.TreeMap;
Expand Down Expand Up @@ -86,7 +87,9 @@ void reloadWhileBuildIsInQueue() throws Exception {
// Note that the bug does not reproduce simply from p.doReload(), since in that case Job identity remains intact:
r.jenkins.reload();
p = r.jenkins.getItemByFullName("p", FreeStyleProject.class);
assertNotNull(p);
FreeStyleBuild b1 = p.getLastBuild();
assertNotNull(b1);
assertEquals(1, b1.getNumber());
/* Currently fails since Run.project is final. But anyway that is not the problem:
assertEquals(p, b1.getParent());
Expand All @@ -95,17 +98,17 @@ void reloadWhileBuildIsInQueue() throws Exception {
assertEquals(1, items.length);
assertEquals(p, items[0].task); // the real issue: assignBuildNumber was being called on the wrong Job
QueueTaskFuture<Queue.Executable> b2f = items[0].getFuture();
b1.getExecutor().interrupt();
Objects.requireNonNull(b1.getExecutor()).interrupt();
r.assertBuildStatus(Result.ABORTED, r.waitForCompletion(b1));
FreeStyleBuild b2 = (FreeStyleBuild) b2f.waitForStart();
assertEquals(2, b2.getNumber());
assertEquals(p, b2.getParent());
b2.getExecutor().interrupt();
Objects.requireNonNull(b2.getExecutor()).interrupt();
r.assertBuildStatus(Result.ABORTED, r.waitForCompletion(b2));
FreeStyleBuild b3 = p.scheduleBuild2(0).waitForStart();
assertEquals(3, b3.getNumber());
assertEquals(p, b3.getParent());
b3.getExecutor().interrupt();
Objects.requireNonNull(b3.getExecutor()).interrupt();
r.assertBuildStatus(Result.ABORTED, r.waitForCompletion(b3));
}

Expand Down Expand Up @@ -166,7 +169,7 @@ void stream() throws Exception {
}
RunMap<FreeStyleBuild> runMap = p._getRuns();

assertEquals(builds.size(), runMap.entrySet().size());
assertEquals(builds.size(), runMap.size());
assertTrue(
runMap.entrySet().stream().spliterator().hasCharacteristics(Spliterator.DISTINCT));
assertTrue(
Expand Down Expand Up @@ -209,7 +212,7 @@ void runLoadCounterFirst() throws Exception {
}
assertEquals(
10,
RunLoadCounter.assertMaxLoads(p, 2, () -> p.getBuilds().stream().findFirst().orElse(null).number).intValue());
RunLoadCounter.assertMaxLoads(p, 2, () -> Objects.requireNonNull(p.getBuilds().stream().findFirst().orElse(null)).number).intValue());
}

@Test
Expand All @@ -220,6 +223,24 @@ void runLoadCounterLimit() throws Exception {
}
assertEquals(
6,
RunLoadCounter.assertMaxLoads(p, 6, () -> Streams.findLast(p.getBuilds().stream().limit(5)).orElse(null).number).intValue());
RunLoadCounter.assertMaxLoads(p, 6, () -> Objects.requireNonNull(
Streams.findLast(p.getBuilds().stream().limit(5)).orElse(null)).number).intValue());
}

@SuppressWarnings("deprecation")
@Issue("JENKINS-75465")
@Test
void getByIdLegacyAndNumeric() throws Exception {
FreeStyleProject p = r.createFreeStyleProject();
FreeStyleBuild b1 = r.buildAndAssertSuccess(p);
RunMap<FreeStyleBuild> runs = p._getRuns();

assertSame(b1, runs.getById(b1.getNumber()), "getById(int) should find the build");
assertNull(runs.getById(999), "getById(int) should return null for missing ID");
assertSame(b1, runs.getById(String.valueOf(b1.getNumber())), "getById(String) should find the build via parsing");
assertNull(runs.getById("not-a-number"), "getById(String) should return null for non-numeric input");
assertNull(runs.getById(""), "getById(String) should return null for empty string");
assertNull(runs.getById("2007-11-12_12-23-45"), "getById(String) should return null for historical date-time IDs");
assertNull(runs.getById(" " + b1.getNumber()), "getById(String) must strictly follow Integer.parseInt behavior (no trimming)");
}
}
Loading