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.

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

Jump to bottom

JENKINS-70275 git scm owner is reinitialized if null whenever accessi… #374

Merged
merged 3 commits into from Mar 26, 2023
Merged

JENKINS-70275 git scm owner is reinitialized if null whenever accessi… #374

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
359fb62
JENKINS-70275 git scm owner is reinitialized if null whenever accessi…
mhenschke-atl Feb 2, 2023
8c93cc6
Updated name for get initialized scm method
mhenschke-atl Feb 3, 2023
97e80ee
JENKSIN-70275 removed uninitialized self link reference
mhenschke-atl Feb 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/main/java/com/atlassian/bitbucket/jenkins/internal/scm/BitbucketSCMSource.java
View file
Open in desktop
Expand Up @@ -181,6 +181,9 @@ public BitbucketSCMRepository getBitbucketSCMRepository() {
CustomGitSCMSource getAndInitializeGitSCMSourceIfNull() {
if (gitSCMSource == null) {
initializeGitScmSource();
}
if (getOwner() != null && gitSCMSource.getOwner() == null) {
Copy link

Choose a reason for hiding this comment

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

Does this only have to happen after the scm source is initialized or are there other scenarios where we might need to set the owner? If the former, should we put this code as part of initializeGitScmSource?

Copy link
Author

Choose a reason for hiding this comment

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

The GitSCMSource always needs an owner, so it does need to be in the initialization too. It's done as part of constructing the new source:

        gitSCMSource = new CustomGitSCMSource(remoteConfig.getUrl(), repository);
        gitSCMSource.setBrowser(new BitbucketServer(selfLink));
        gitSCMSource.setCredentialsId(repository.getCloneCredentialsId());
        gitSCMSource.setOwner(getOwner());
        gitSCMSource.setTraits(traits);
        gitSCMSource.setId(getId() + "-git-scm");

One thing to note here is we also set a browser which is not serialized with the rest of the source. I'm actually not 100% sure of what that browser is doing- theoretically it generates links out to source browsers but we already do that manually with BitbucketLink. That said I'm happy to include a null check for it as well, for completeness.

Copy link

Choose a reason for hiding this comment

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

The browser provides the links for when you view the changeset in Jenkins. Would be good to include it as well. Either in this PR or in a separate PR

Copy link
Author

Choose a reason for hiding this comment

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

The browser is a slightly trickier task, as it relies on the self-link which we don't currently persist anywhere. I might tackle this in a separate PR, it's a larger change and unrelated to this issue.

Copy link

Choose a reason for hiding this comment

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

I am slightly concerned that we're removing the browser but... I think we can address that in a separate PR before release.

gitSCMSource.setOwner(getOwner());
}
return gitSCMSource;
}
Expand Down
22 changes: 18 additions & 4 deletions src/test/java/com/atlassian/bitbucket/jenkins/internal/scm/BitbucketSCMSourceTest.java
View file
Open in desktop
Expand Up @@ -14,10 +14,7 @@
import hudson.model.Action;
import hudson.model.Actionable;
import jenkins.branch.MultiBranchProject;
import jenkins.scm.api.SCMHead;
import jenkins.scm.api.SCMHeadEvent;
import jenkins.scm.api.SCMSourceDescriptor;
import jenkins.scm.api.SCMSourceEvent;
import jenkins.scm.api.*;
import jenkins.scm.api.metadata.PrimaryInstanceMetadataAction;

import org.apache.commons.lang3.StringUtils;
Expand Down Expand Up @@ -216,6 +213,23 @@ public void testInitializeSCMSourceHTTP() {
CustomGitSCMSource gitSource = source.getAndInitializeGitSCMSourceIfNull();
assertThat(gitSource.getRemote(), equalTo(HTTP_CLONE_LINK));
}

@Test
public void testGetAndInitializeGitSCMSourceSetsOwnerIfNull() {
BitbucketSCMSource source = new SCMSourceBuilder(CREDENTIAL_ID)
.serverId(SERVER_ID)
.projectName(PROJECT_NAME)
.repositoryName(REPOSITORY_NAME)
.build();
CustomGitSCMSource gitSource = source.getAndInitializeGitSCMSourceIfNull(); // Initializes with no owner
assertThat(gitSource.getOwner(), equalTo(null));

SCMSourceOwner owner = mock(SCMSourceOwner.class);
source.setOwner(owner); // This is handled by Jenkins during typical execution

gitSource = source.getAndInitializeGitSCMSourceIfNull();
assertThat(gitSource.getOwner(), equalTo(owner));
}

@Test
public void testInitializeSCMSourceSSH() {
Expand Down