Modernize to Jenkins 2.479 and Jakarta EE 9 #134

mtughan · 2024-11-21T15:01:59Z

Adapter methods are added for old overrides.
Switch to JUnit 5 for tests

Required for the Scriptler plugin to modernize as well without breaking SSH compatibility.

Testing done

Unit tests updated and run.

Submitter checklist

Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
Ensure that the pull request title represents the desired changelog entry
Please describe what you did
Link to relevant issues in GitHub or Jira
Link to relevant pull requests, esp. upstream and downstream changes
Ensure you have provided tests - that demonstrates feature works or fixes the issue

mtughan · 2024-11-21T16:41:34Z

Trying to figure out why SpotBugs is errantly flagging a null issue where one doesn’t exist and why it’s not suppressing.

src/main/java/org/jenkinsci/plugins/gitserver/CSRFExclusionImpl.java

mtughan · 2024-12-11T14:05:08Z

@jenkinsci/git-server-plugin-developers, please review when able.

pom.xml

basil

Thanks for the PR!

src/main/java/org/jenkinsci/plugins/gitserver/CSRFExclusionImpl.java

jglick · 2025-02-11T16:10:44Z

src/main/java/org/jenkinsci/plugins/gitserver/ChannelTransport.java

@@ -56,6 +55,7 @@ public FetchConnection openFetch() throws NotSupportedException, TransportExcept
        } catch (IOException e) {
            throw new TransportException("Failed to open a fetch connection",e);
        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();


Does not appear to be related to the stated PR goal?

Was flagged by the SonarQube for IDE in my IntelliJ IDEA (rule java:S1242). I've been fixing a bunch of other issues like this and so wanted to fix it here too. However, if you'd prefer it to be dropped, I can do so. I'm not stuck on getting that in.

basil · 2025-02-20T15:22:34Z

src/main/java/org/jenkinsci/plugins/gitserver/HttpGitRepository.java

@@ -60,11 +63,27 @@ public HttpGitRepository() {
     *
     * @see ReceivePackFactory#create(Object, Repository)
     */
-    public abstract ReceivePack createReceivePack(HttpServletRequest context, Repository db) throws ServiceNotEnabledException, ServiceNotAuthorizedException;
+    @SuppressWarnings({"deprecated", "java:S1874"})


Coming back to this PR, this is the only part I feel uneasy about. It implements the compatibility layer in the opposite way that Jenkins core did in jenkinsci/jenkins#9672, by having the new functionality delegate to the old rather than having the old functionality delegate to the new. This also makes it harder to eventually remove the old functionality. For consistency, I would want to see this compatibility layer implemented the same way as jenkinsci/jenkins#9672 or jenkinsci/cloudbees-folder-plugin#444.

Hmm, weird. I don't remember why I did this now. I'll flip it around.

BTW, the closest analog I can find for this method (as I needed to change the signature of an abstract method, not a concrete method) is ReconfigurableDescribable in jenkinsci/jenkins#9672, as it's an interface and one of the interface methods needed to change. In that scenario, the interface method was changed into 2 default methods that both check if the other is present. I will take this approach, changing the abstract method into the 2 concrete methods I've already done, but by having both check for the presence of the other.

ParameterDefinition is an example of an abstract method that doesn't throw an exception. For one that does, View#submit is an example.

Thanks, I missed those ones. I think what I've gotten is quite close to View#submit.

* Adapter methods are added for old overrides. * Switch to JUnit 5 for tests

mtughan

Apologies for the larger diff between the commits. I discovered that Spotless hadn't been run on the plugin code, so I made one more commit before mine that runs Spotless first.

mtughan · 2025-02-21T15:11:36Z

src/main/java/org/jenkinsci/plugins/gitserver/HttpGitRepository.java

@@ -60,11 +63,27 @@ public HttpGitRepository() {
     *
     * @see ReceivePackFactory#create(Object, Repository)
     */
-    public abstract ReceivePack createReceivePack(HttpServletRequest context, Repository db) throws ServiceNotEnabledException, ServiceNotAuthorizedException;
+    @SuppressWarnings({"deprecated", "java:S1874"})


BTW, the closest analog I can find for this method (as I needed to change the signature of an abstract method, not a concrete method) is ReconfigurableDescribable in jenkinsci/jenkins#9672, as it's an interface and one of the interface methods needed to change. In that scenario, the interface method was changed into 2 default methods that both check if the other is present. I will take this approach, changing the abstract method into the 2 concrete methods I've already done, but by having both check for the presence of the other.

mtughan requested a review from a team as a code owner November 21, 2024 15:02

mtughan force-pushed the modernize-2.479 branch from b3662b9 to 9549307 Compare November 21, 2024 15:52

mtughan marked this pull request as draft November 21, 2024 16:40

mtughan force-pushed the modernize-2.479 branch from 9549307 to e35fb41 Compare November 21, 2024 17:39

mtughan marked this pull request as ready for review November 21, 2024 17:48

mtughan force-pushed the modernize-2.479 branch from e35fb41 to ba97a8c Compare November 21, 2024 18:04

mtughan marked this pull request as draft November 21, 2024 18:53

mtughan force-pushed the modernize-2.479 branch from ba97a8c to 91bdae0 Compare November 21, 2024 19:15

mtughan commented Nov 21, 2024

View reviewed changes

src/main/java/org/jenkinsci/plugins/gitserver/CSRFExclusionImpl.java Outdated Show resolved Hide resolved

mtughan marked this pull request as ready for review November 21, 2024 19:34

mtughan force-pushed the modernize-2.479 branch 2 times, most recently from 90bd3d1 to 0d96613 Compare November 22, 2024 15:18

mtughan force-pushed the modernize-2.479 branch 2 times, most recently from bfb36e4 to f4a410b Compare December 2, 2024 19:23

basil added the internal label Jan 17, 2025

basil reviewed Jan 26, 2025

View reviewed changes

pom.xml Show resolved Hide resolved

mtughan force-pushed the modernize-2.479 branch from d70a302 to aec904d Compare January 27, 2025 14:25

basil approved these changes Jan 27, 2025

View reviewed changes

jglick reviewed Feb 11, 2025

View reviewed changes

src/main/java/org/jenkinsci/plugins/gitserver/CSRFExclusionImpl.java Outdated Show resolved Hide resolved

jglick reviewed Feb 11, 2025

View reviewed changes

mtughan force-pushed the modernize-2.479 branch from aec904d to 6096dc6 Compare February 14, 2025 17:37

basil reviewed Feb 20, 2025

View reviewed changes

Run Spotless Apply

cdd7ffc

mtughan force-pushed the modernize-2.479 branch from 6096dc6 to dfca147 Compare February 21, 2025 15:30

Modernize to Jenkins 2.479 and Jakarta EE 9

238480b

* Adapter methods are added for old overrides. * Switch to JUnit 5 for tests

mtughan force-pushed the modernize-2.479 branch from dfca147 to 238480b Compare February 21, 2025 15:35

mtughan commented Feb 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modernize to Jenkins 2.479 and Jakarta EE 9 #134

Modernize to Jenkins 2.479 and Jakarta EE 9 #134

mtughan commented Nov 21, 2024

mtughan commented Nov 21, 2024

mtughan commented Dec 11, 2024

basil left a comment

jglick Feb 11, 2025

mtughan Feb 14, 2025

basil Feb 20, 2025 •

edited

Loading

mtughan Feb 21, 2025

mtughan Feb 21, 2025

basil Feb 21, 2025

mtughan Feb 21, 2025

mtughan left a comment

mtughan Feb 21, 2025

Modernize to Jenkins 2.479 and Jakarta EE 9 #134

Are you sure you want to change the base?

Modernize to Jenkins 2.479 and Jakarta EE 9 #134

Conversation

mtughan commented Nov 21, 2024

Testing done

Submitter checklist

mtughan commented Nov 21, 2024

mtughan commented Dec 11, 2024

basil left a comment

Choose a reason for hiding this comment

jglick Feb 11, 2025

Choose a reason for hiding this comment

mtughan Feb 14, 2025

Choose a reason for hiding this comment

basil Feb 20, 2025 • edited Loading

Choose a reason for hiding this comment

mtughan Feb 21, 2025

Choose a reason for hiding this comment

mtughan Feb 21, 2025

Choose a reason for hiding this comment

basil Feb 21, 2025

Choose a reason for hiding this comment

mtughan Feb 21, 2025

Choose a reason for hiding this comment

mtughan left a comment

Choose a reason for hiding this comment

mtughan Feb 21, 2025

Choose a reason for hiding this comment

basil Feb 20, 2025 •

edited

Loading