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-35227] added support for AdditionalCredentials to SubversionSCMSource #189

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

[JENKINS-35227] added support for AdditionalCredentials to SubversionSCMSource #189

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3685c38
added support for AdditionalCredentials to SubversionSCMSource and fixed
Jun 20, 2017
11db817
moved AdditionalCredentials back to SubversionSCM
Jun 22, 2017
fa29350
undo changes in SubversionSCM and more efficient initializing in Subv…
Jun 23, 2017
3540bb6
made getter for additional credentials restricted and undid changes
Jul 7, 2017
4a9730b
returns unmodifiableList in getter for additionalCredentials
Jul 10, 2017
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
20 changes: 19 additions & 1 deletion src/main/java/jenkins/scm/impl/subversion/SubversionSCMSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@
import hudson.scm.FilterSVNAuthenticationManager;
import hudson.scm.SubversionRepositoryStatus;
import hudson.scm.SubversionSCM;
import hudson.scm.SubversionSCM.AdditionalCredentials;
import hudson.scm.subversion.SvnHelper;
import hudson.scm.subversion.UpdateUpdater;
import hudson.security.ACL;
import hudson.util.EditDistance;
import hudson.util.FormValidation;
Expand All @@ -68,6 +70,8 @@
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.tmatesoft.svn.core.SVNDirEntry;
import org.tmatesoft.svn.core.SVNException;
import org.tmatesoft.svn.core.SVNNodeKind;
Expand Down Expand Up @@ -121,6 +125,8 @@ public class SubversionSCMSource extends SCMSource {
private final String remoteBase;

private String credentialsId = ""; // TODO null would be a better default, but need to check null safety on usages

private List<AdditionalCredentials> additionalCredentials;
Copy link
Member

Choose a reason for hiding this comment

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

from what I see it still may be null when you read data from the disk


private String includes = DescriptorImpl.DEFAULT_INCLUDES;

Expand Down Expand Up @@ -159,6 +165,18 @@ public String getCredentialsId() {
public void setCredentialsId(String credentialsId) {
this.credentialsId = credentialsId;
}
@DataBoundSetter
public void setAdditionalCredentials(List<AdditionalCredentials> additionalCredentials) {
this.additionalCredentials = additionalCredentials;
}
//without getter jelly will crash
@Restricted(NoExternalUse.class)
public List<AdditionalCredentials> getAdditionalCredentials() {
if(additionalCredentials==null) {
return Collections.emptyList();
}
return(additionalCredentials);
Copy link
Member

Choose a reason for hiding this comment

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

FindBugs will grumble about exposing external implementation. Either make it @Restricted for Jelly only or wrap with unmodifyableList

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your comments. How do I make it restricted? Does adding "@restricted(NoExternalUse.class)" work?

Copy link
Member

Choose a reason for hiding this comment

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

yes

Copy link
Member

Choose a reason for hiding this comment

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

would be better to just return Collections.unmodifyableList(additionalCredentials)

}

/**
* Gets the comma separated list of exclusions.
Expand Down Expand Up @@ -689,7 +707,7 @@ public SubversionSCM build(@NonNull SCMHead head,
// name contains an @ so need to ensure there is an @ at the end of the name
remote.append('@');
}
return new SubversionSCM(remote.toString(), credentialsId, ".");
return new SubversionSCM(SubversionSCM.ModuleLocation.parse(new String[]{remote.toString()}, new String[]{credentialsId}, new String[]{"."}, null,null), new UpdateUpdater(), null, null, null, null, null, null, false, false, additionalCredentials);
}

/**
Expand Down
3 changes: 3 additions & 0 deletions src/main/resources/jenkins/scm/impl/subversion/SubversionSCMSource/config-detail.jelly
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,7 @@
<f:entry title="${%Exclude branches}" field="excludes">
<f:textbox default="${descriptor.DEFAULT_EXCLUDES}"/>
</f:entry>
<f:entry title="${%Additional Credentials}" field="additionalCredentials" help="/descriptor/hudson.scm.SubversionSCM/help/additionalCredentials">
<f:repeatableProperty field="additionalCredentials" add="${%Add additional credentials...}"/>
</f:entry>
</j:jelly>