Skip to content

Commit 755fec8

Browse files
committed
Encapsulate access to repositoryAccessStrategy and defaultPermissionsStrategy for subclasses
c.f. 5a78598
1 parent 7a30947 commit 755fec8

File tree

1 file changed

+13
-16
lines changed

1 file changed

+13
-16
lines changed

src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubAppCredentials.java

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import java.security.GeneralSecurityException;
2626
import java.time.Duration;
2727
import java.time.Instant;
28-
import java.util.Collections;
2928
import java.util.List;
3029
import java.util.Locale;
3130
import java.util.Map;
@@ -135,8 +134,6 @@ public GitHubAppCredentials(
135134
super(scope, id, description);
136135
this.appID = appID;
137136
this.privateKey = privateKey;
138-
this.repositoryAccessStrategy = new AccessInferredOwner();
139-
this.defaultPermissionsStrategy = DefaultPermissionsStrategy.INHERIT_ALL;
140137
}
141138

142139
public String getApiUri() {
@@ -170,9 +167,9 @@ public String getOwner() {
170167
public void setOwner(String owner) {
171168
owner = Util.fixEmptyAndTrim(owner);
172169
if (owner != null) {
173-
this.repositoryAccessStrategy = new AccessSpecifiedRepositories(owner, List.of());
170+
setRepositoryAccessStrategy(new AccessSpecifiedRepositories(owner, List.of()));
174171
} else {
175-
this.repositoryAccessStrategy = new AccessInferredOwner();
172+
setRepositoryAccessStrategy(new AccessInferredOwner());
176173
}
177174
// We only expect this to be called by CasC and by a few plugins which implement variants of this class based on
178175
// external credential providers, so we still count it as a migration.
@@ -181,7 +178,7 @@ public void setOwner(String owner) {
181178

182179
@NonNull
183180
public RepositoryAccessStrategy getRepositoryAccessStrategy() {
184-
return repositoryAccessStrategy;
181+
return repositoryAccessStrategy == null ? new AccessInferredOwner() : repositoryAccessStrategy;
185182
}
186183

187184
@DataBoundSetter
@@ -191,7 +188,7 @@ public void setRepositoryAccessStrategy(@NonNull RepositoryAccessStrategy strate
191188

192189
@NonNull
193190
public DefaultPermissionsStrategy getDefaultPermissionsStrategy() {
194-
return defaultPermissionsStrategy;
191+
return defaultPermissionsStrategy == null ? DefaultPermissionsStrategy.INHERIT_ALL : defaultPermissionsStrategy;
195192
}
196193

197194
@DataBoundSetter
@@ -200,14 +197,14 @@ public void setDefaultPermissionsStrategy(@NonNull DefaultPermissionsStrategy st
200197
}
201198

202199
AccessibleRepositories getAccessibleRepositories() {
203-
return repositoryAccessStrategy.forContext(context);
200+
return getRepositoryAccessStrategy().forContext(context);
204201
}
205202

206203
Map<String, GHPermissionType> getPermissions() {
207204
if (context.getPermissions() != null) {
208205
return context.getPermissions();
209206
}
210-
return defaultPermissionsStrategy.getPermissions();
207+
return getDefaultPermissionsStrategy().getPermissions();
211208
}
212209

213210
@SuppressWarnings("deprecation")
@@ -446,8 +443,8 @@ public GitHubAppCredentials contextualize(final GitHubAppUsageContext context) {
446443
private GitHubAppCredentials clone(final GitHubAppUsageContext context) {
447444
final var clone = new GitHubAppCredentials(getScope(), getId(), getDescription(), getAppID(), getPrivateKey());
448445
clone.apiUri = getApiUri();
449-
clone.repositoryAccessStrategy = getRepositoryAccessStrategy();
450-
clone.defaultPermissionsStrategy = getDefaultPermissionsStrategy();
446+
clone.setRepositoryAccessStrategy(getRepositoryAccessStrategy());
447+
clone.setDefaultPermissionsStrategy(getDefaultPermissionsStrategy());
451448
clone.context = context;
452449
return clone;
453450
}
@@ -462,7 +459,7 @@ public Credentials forRun(Run<?, ?> context) {
462459
final var usageContext = GitHubAppUsageContext.builder()
463460
.inferredOwner(source.getRepoOwner())
464461
.inferredRepository(source.getRepository())
465-
.permissions(defaultPermissionsStrategy.getPermissions())
462+
.permissions(getDefaultPermissionsStrategy().getPermissions())
466463
.build();
467464
return contextualize(usageContext);
468465
}
@@ -473,7 +470,7 @@ public Credentials forRun(Run<?, ?> context) {
473470
final var usageContext = GitHubAppUsageContext.builder()
474471
.inferredOwner(ghrn.userName)
475472
.inferredRepository(ghrn.repositoryName)
476-
.permissions(defaultPermissionsStrategy.getPermissions())
473+
.permissions(getDefaultPermissionsStrategy().getPermissions())
477474
.build();
478475
return contextualize(usageContext);
479476
}
@@ -598,7 +595,7 @@ private Object readResolve() {
598595
if (repositoryAccessStrategy == null || defaultPermissionsStrategy == null) {
599596
if (owner != null) {
600597
// In this case, the migration should result in identical behavior.
601-
repositoryAccessStrategy = new AccessSpecifiedRepositories(owner, Collections.emptyList());
598+
setRepositoryAccessStrategy(new AccessSpecifiedRepositories(owner, List.of()));
602599
} else {
603600
// There is a choice here: We can either preserve compatibility for users who have
604601
// the app installed in multiple orgs and only use the credentials in contexts
@@ -607,9 +604,9 @@ private Object readResolve() {
607604
// use it in contexts where inference is not supported by using
608605
// AccessSpecifiedRepositories with a null owner.
609606
// None of the new strategies support these two use cases simultaneously.
610-
repositoryAccessStrategy = new AccessInferredOwner();
607+
setRepositoryAccessStrategy(new AccessInferredOwner());
611608
}
612-
defaultPermissionsStrategy = DefaultPermissionsStrategy.INHERIT_ALL;
609+
setDefaultPermissionsStrategy(DefaultPermissionsStrategy.INHERIT_ALL);
613610
MigrationAdminMonitor.addMigratedCredentialId(getId());
614611
}
615612
owner = null;

0 commit comments

Comments
 (0)