Skip to content

Commit 0b857c0

Browse files
chadlwilsonchadlwilson
committed
Tidy logging usage and minor code style fixes
1 parent 9c4fe55 commit 0b857c0

File tree

51 files changed

+518
-535
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

51 files changed

+518
-535
lines changed

‎build.gradle‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ dependencies {
7171
implementation 'com.google.code.gson:gson:2.13.2'
7272
implementation 'org.kohsuke:github-api:1.330'
7373
implementation 'com.squareup.okhttp3:okhttp:5.1.0'
74+
implementation platform('org.slf4j:slf4j-bom:2.0.17')
75+
implementation 'org.slf4j:jul-to-slf4j'
7476

7577
testImplementation project.deps.gocdPluginApi
7678
testImplementation platform('org.junit:junit-bom:6.0.0')
@@ -83,8 +85,6 @@ dependencies {
8385
testImplementation 'org.skyscreamer:jsonassert:1.5.3'
8486
testImplementation 'org.jsoup:jsoup:1.21.2'
8587
testImplementation 'com.squareup.okhttp3:mockwebserver3-junit5:5.1.0'
86-
87-
8888
}
8989

9090
test {

‎src/main/java/cd/go/authorization/github/GitHubAuthenticator.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,15 @@
2020
import cd.go.authorization.github.models.AuthConfig;
2121
import cd.go.authorization.github.models.LoggedInUserInfo;
2222
import cd.go.authorization.github.models.OAuthTokenInfo;
23+
import com.thoughtworks.go.plugin.api.logging.Logger;
2324
import org.kohsuke.github.GitHub;
2425

2526
import java.io.IOException;
2627
import java.util.List;
2728

28-
import static cd.go.authorization.github.GitHubPlugin.LOG;
29-
3029
public class GitHubAuthenticator {
30+
private static final Logger LOG = Logger.getLoggerFor(GitHubAuthenticator.class);
31+
3132
private final MembershipChecker membershipChecker;
3233
private final GitHubClientBuilder gitHubClientBuilder;
3334

@@ -46,10 +47,10 @@ public LoggedInUserInfo authenticate(OAuthTokenInfo tokenInfo, AuthConfig authCo
4647
final LoggedInUserInfo loggedInUserInfo = new LoggedInUserInfo(gitHub);
4748

4849
if (allowedOrganizations.isEmpty()) {
49-
LOG.info("[Authenticate] User `{}` authenticated successfully, organisation membership not required.", loggedInUserInfo.getUser().username());
50+
LOG.info("User `{}` authenticated successfully, organisation membership not required.", loggedInUserInfo.getUser().username());
5051
return loggedInUserInfo;
5152
} else if (membershipChecker.isAMemberOfAtLeastOneOrganization(loggedInUserInfo.getGitHubUser(), authConfig, allowedOrganizations)) {
52-
LOG.info("[Authenticate] User `{}` authenticated successfully as member of an allowed organisation.", loggedInUserInfo.getUser().username());
53+
LOG.info("User `{}` authenticated successfully as member of an allowed organisation.", loggedInUserInfo.getUser().username());
5354
return loggedInUserInfo;
5455
}
5556

‎src/main/java/cd/go/authorization/github/GitHubAuthorizer.java‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,17 @@
1818

1919
import cd.go.authorization.github.models.AuthConfig;
2020
import cd.go.authorization.github.models.Role;
21+
import com.thoughtworks.go.plugin.api.logging.Logger;
2122
import org.kohsuke.github.GHUser;
2223

2324
import java.io.IOException;
2425
import java.util.ArrayList;
2526
import java.util.Collections;
2627
import java.util.List;
2728

28-
import static cd.go.authorization.github.GitHubPlugin.LOG;
29-
3029
public class GitHubAuthorizer {
30+
private static final Logger LOG = Logger.getLoggerFor(GitHubAuthorizer.class);
31+
3132
private final MembershipChecker membershipChecker;
3233

3334
public GitHubAuthorizer() {
@@ -40,35 +41,35 @@ public GitHubAuthorizer(MembershipChecker membershipChecker) {
4041

4142
public List<String> authorize(GHUser user, AuthConfig authConfig, List<Role> roles) throws IOException {
4243
if (roles == null || roles.isEmpty()) {
43-
LOG.debug("[Authorize] User `{}` is authorized for no specific GoCD roles. No role configurations defined for plugin; so no authorizations can be inferred.", user.getLogin());
44+
LOG.debug("User `{}` is authorized for no specific GoCD roles. No role configurations defined for plugin; so no authorizations can be inferred.", user.getLogin());
4445
return Collections.emptyList();
4546
}
4647

4748
final List<String> assignedRoles = new ArrayList<>();
4849

49-
LOG.debug("[Authorize] Authorizing user `{}`", user.getLogin());
50+
LOG.debug("Authorizing user `{}`", user.getLogin());
5051

5152
for (Role role : roles) {
5253
final List<String> allowedUsers = role.roleConfiguration().users();
5354
if (!allowedUsers.isEmpty() && allowedUsers.contains(user.getLogin().toLowerCase())) {
54-
LOG.info("[Authorize] Assigning GoCD role `{}` to user `{}` as user belongs to allowed users list.", role.name(), user.getLogin());
55+
LOG.info("Assigning GoCD role `{}` to user `{}` as user belongs to allowed users list.", role.name(), user.getLogin());
5556
assignedRoles.add(role.name());
5657
continue;
5758
}
5859

5960
if (membershipChecker.isAMemberOfAtLeastOneOrganization(user, authConfig, role.roleConfiguration().organizations())) {
60-
LOG.info("[Authorize] Assigning GoCD role `{}` to user `{}` as user is a member of at least one allowed organization.", role.name(), user.getLogin());
61+
LOG.info("Assigning GoCD role `{}` to user `{}` as user is a member of at least one allowed organization.", role.name(), user.getLogin());
6162
assignedRoles.add(role.name());
6263
continue;
6364
}
6465

6566
if (membershipChecker.isAMemberOfAtLeastOneTeamOfOrganization(user, authConfig, role.roleConfiguration().teams())) {
66-
LOG.info("[Authorize] Assigning role GoCD `{}` to user `{}` as user is a member of at least one allowed team + organisation combination.", role.name(), user.getLogin());
67+
LOG.info("Assigning role GoCD `{}` to user `{}` as user is a member of at least one allowed team + organisation combination.", role.name(), user.getLogin());
6768
assignedRoles.add(role.name());
6869
}
6970
}
7071

71-
LOG.debug("[Authorize] User `{}` is authorized with `{}` GoCD role(s).", user.getLogin(), assignedRoles);
72+
LOG.debug("User `{}` is authorized with `{}` GoCD role(s).", user.getLogin(), assignedRoles);
7273

7374
return assignedRoles;
7475
}

‎src/main/java/cd/go/authorization/github/GitHubPlugin.java‎

Lines changed: 25 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -23,61 +23,46 @@
2323
import com.thoughtworks.go.plugin.api.GoPlugin;
2424
import com.thoughtworks.go.plugin.api.GoPluginIdentifier;
2525
import com.thoughtworks.go.plugin.api.annotation.Extension;
26-
import com.thoughtworks.go.plugin.api.exceptions.UnhandledRequestTypeException;
2726
import com.thoughtworks.go.plugin.api.logging.Logger;
2827
import com.thoughtworks.go.plugin.api.request.GoPluginApiRequest;
2928
import com.thoughtworks.go.plugin.api.response.GoPluginApiResponse;
29+
import org.slf4j.bridge.SLF4JBridgeHandler;
3030

3131
import static cd.go.authorization.github.Constants.PLUGIN_IDENTIFIER;
3232

3333
@Extension
3434
public class GitHubPlugin implements GoPlugin {
35-
public static final Logger LOG = Logger.getLoggerFor(GitHubPlugin.class);
35+
private static final Logger LOG = Logger.getLoggerFor(GitHubPlugin.class);
3636

37-
private GoApplicationAccessor accessor;
37+
static {
38+
// Redirect logging from the GitHub API and OKHttp to SLF4J and thus GoCD's logging system
39+
SLF4JBridgeHandler.removeHandlersForRootLogger();
40+
SLF4JBridgeHandler.install();
41+
}
3842

3943
@Override
40-
public void initializeGoApplicationAccessor(GoApplicationAccessor accessor) {
41-
this.accessor = accessor;
42-
}
44+
public void initializeGoApplicationAccessor(GoApplicationAccessor accessor) {}
4345

4446
@Override
4547
public GoPluginApiResponse handle(GoPluginApiRequest request) {
4648
try {
47-
switch (RequestFromServer.fromString(request.requestName())) {
48-
case REQUEST_GET_PLUGIN_ICON:
49-
return new GetPluginIconRequestExecutor().execute();
50-
case REQUEST_GET_CAPABILITIES:
51-
return new GetCapabilitiesRequestExecutor().execute();
52-
case REQUEST_GET_AUTH_CONFIG_METADATA:
53-
return new GetAuthConfigMetadataRequestExecutor().execute();
54-
case REQUEST_AUTH_CONFIG_VIEW:
55-
return new GetAuthConfigViewRequestExecutor().execute();
56-
case REQUEST_VALIDATE_AUTH_CONFIG:
57-
return AuthConfigValidateRequest.from(request).execute();
58-
case REQUEST_VERIFY_CONNECTION:
59-
return VerifyConnectionRequest.from(request).execute();
60-
case REQUEST_GET_ROLE_CONFIG_METADATA:
61-
return new GetRoleConfigMetadataRequestExecutor().execute();
62-
case REQUEST_ROLE_CONFIG_VIEW:
63-
return new GetRoleConfigViewRequestExecutor().execute();
64-
case REQUEST_VALIDATE_ROLE_CONFIG:
65-
return RoleConfigValidateRequest.from(request).execute();
66-
case REQUEST_AUTHORIZATION_SERVER_REDIRECT_URL:
67-
return GetAuthorizationServerUrlRequest.from(request).execute();
68-
case REQUEST_ACCESS_TOKEN:
69-
return FetchAccessTokenRequest.from(request).execute();
70-
case REQUEST_IS_VALID_USER:
71-
return ValidateUserRequest.from(request).execute();
72-
case REQUEST_AUTHENTICATE_USER:
73-
return UserAuthenticationRequest.from(request).execute();
74-
case REQUEST_SEARCH_USERS:
75-
return SearchUsersRequest.from(request).execute();
76-
case REQUEST_GET_USER_ROLES:
77-
return GetRolesRequest.from(request).execute();
78-
default:
79-
throw new UnhandledRequestTypeException(request.requestName());
80-
}
49+
return switch (RequestFromServer.fromString(request.requestName())) {
50+
case REQUEST_GET_PLUGIN_ICON -> new GetPluginIconRequestExecutor().execute();
51+
case REQUEST_GET_CAPABILITIES -> new GetCapabilitiesRequestExecutor().execute();
52+
case REQUEST_GET_AUTH_CONFIG_METADATA -> new GetAuthConfigMetadataRequestExecutor().execute();
53+
case REQUEST_AUTH_CONFIG_VIEW -> new GetAuthConfigViewRequestExecutor().execute();
54+
case REQUEST_VALIDATE_AUTH_CONFIG -> AuthConfigValidateRequest.from(request).execute();
55+
case REQUEST_VERIFY_CONNECTION -> VerifyConnectionRequest.from(request).execute();
56+
case REQUEST_GET_ROLE_CONFIG_METADATA -> new GetRoleConfigMetadataRequestExecutor().execute();
57+
case REQUEST_ROLE_CONFIG_VIEW -> new GetRoleConfigViewRequestExecutor().execute();
58+
case REQUEST_VALIDATE_ROLE_CONFIG -> RoleConfigValidateRequest.from(request).execute();
59+
case REQUEST_AUTHORIZATION_SERVER_REDIRECT_URL -> GetAuthorizationServerUrlRequest.from(request).execute();
60+
case REQUEST_ACCESS_TOKEN -> FetchAccessTokenRequest.from(request).execute();
61+
case REQUEST_IS_VALID_USER -> ValidateUserRequest.from(request).execute();
62+
case REQUEST_AUTHENTICATE_USER -> UserAuthenticationRequest.from(request).execute();
63+
case REQUEST_SEARCH_USERS -> SearchUsersRequest.from(request).execute();
64+
case REQUEST_GET_USER_ROLES -> GetRolesRequest.from(request).execute();
65+
};
8166
} catch (NoSuchRequestHandlerException e) {
8267
LOG.warn(e.getMessage());
8368
return null;

‎src/main/java/cd/go/authorization/github/MembershipChecker.java‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import cd.go.authorization.github.client.GitHubClientBuilder;
2020
import cd.go.authorization.github.models.AuthConfig;
21+
import com.thoughtworks.go.plugin.api.logging.Logger;
2122
import org.kohsuke.github.GHOrganization;
2223
import org.kohsuke.github.GHTeam;
2324
import org.kohsuke.github.GHUser;
@@ -27,9 +28,10 @@
2728
import java.util.List;
2829
import java.util.Map;
2930

30-
import static cd.go.authorization.github.GitHubPlugin.LOG;
3131

3232
public class MembershipChecker {
33+
public static final Logger LOG = Logger.getLoggerFor(MembershipChecker.class);
34+
3335
private final GitHubClientBuilder clientBuilder;
3436

3537
public MembershipChecker() {
@@ -43,7 +45,7 @@ public MembershipChecker() {
4345

4446
public boolean isAMemberOfAtLeastOneOrganization(GHUser ghUser, AuthConfig authConfig, List<String> organizationsAllowed) throws IOException {
4547
if (organizationsAllowed.isEmpty()) {
46-
LOG.debug("[MembershipChecker] No organizations provided - not allowed.");
48+
LOG.debug("No organizations provided - not allowed.");
4749
return false;
4850
}
4951

@@ -56,7 +58,7 @@ private boolean checkMembershipUsingPersonalAccessToken(GHUser ghUser, AuthConfi
5658
for (String organizationName : organizationsAllowed) {
5759
final GHOrganization organization = gitHubForPersonalAccessToken.getOrganization(organizationName);
5860
if (organization != null && organization.hasMember(ghUser)) {
59-
LOG.info("[MembershipChecker] User `{}` is a member of allowed `{}` organization.", ghUser.getLogin(), organizationName);
61+
LOG.info("User `{}` is a member of allowed `{}` organization.", ghUser.getLogin(), organizationName);
6062
return true;
6163
}
6264
}
@@ -66,7 +68,7 @@ private boolean checkMembershipUsingPersonalAccessToken(GHUser ghUser, AuthConfi
6668

6769
public boolean isAMemberOfAtLeastOneTeamOfOrganization(GHUser ghUser, AuthConfig authConfig, Map<String, List<String>> organizationAndTeamsAllowed) throws IOException {
6870
if (organizationAndTeamsAllowed.isEmpty()) {
69-
LOG.debug("[MembershipChecker] No teams provided - not allowed.");
71+
LOG.debug("No teams provided - not allowed.");
7072
return false;
7173
}
7274

@@ -85,7 +87,7 @@ private boolean checkTeamMembershipUsingPersonalAccessToken(GHUser ghUser, AuthC
8587

8688
for (GHTeam team : teamsFromGitHub.values()) {
8789
if (allowedTeamsFromRole.contains(team.getName().toLowerCase()) && team.hasMember(ghUser)) {
88-
LOG.info("[MembershipChecker] User `{}` is a member of allowed `{}` team.", ghUser.getLogin(), team.getName());
90+
LOG.info("User `{}` is a member of allowed `{}` team.", ghUser.getLogin(), team.getName());
8991
return true;
9092
}
9193
}

‎src/main/java/cd/go/authorization/github/annotation/MetadataHelper.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@
2323

2424
public class MetadataHelper {
2525

26-
public static List<ProfileMetadata> getMetadata(Class<?> clazz) {
26+
public static List<ProfileMetadata<?>> getMetadata(Class<?> clazz) {
2727
Field[] fields = clazz.getDeclaredFields();
28-
List<ProfileMetadata> metadata = new ArrayList<>();
28+
List<ProfileMetadata<?>> metadata = new ArrayList<>();
2929
for (Field field : fields) {
3030
ProfileField profileField = field.getAnnotation(ProfileField.class);
3131
if (profileField != null) {

‎src/main/java/cd/go/authorization/github/annotation/MetadataValidator.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,17 @@ public ValidationResult validate(Validatable configuration) {
2525
Map<String, String> properties = configuration.toProperties();
2626

2727
List<String> knownFields = new ArrayList<>();
28-
for (ProfileMetadata field : MetadataHelper.getMetadata(configuration.getClass())) {
28+
for (ProfileMetadata<?> field : MetadataHelper.getMetadata(configuration.getClass())) {
2929
knownFields.add(field.getKey());
30-
validationResult.addError(field.validate(properties.get(field.getKey())));
30+
field.validate(properties.get(field.getKey()))
31+
.ifPresent(validationResult::addError);
3132
}
3233

3334

34-
Set<String> set = new HashSet<>(properties.keySet());
35-
set.removeAll(knownFields);
35+
Set<String> knownKeys = new HashSet<>(properties.keySet());
36+
knownFields.forEach(knownKeys::remove);
3637

37-
for (String key : set) {
38+
for (String key : knownKeys) {
3839
validationResult.addError(key, "Is an unknown property");
3940
}
4041
return validationResult;

‎src/main/java/cd/go/authorization/github/annotation/ProfileMetadata.java‎

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,7 @@
1919
import com.google.gson.annotations.Expose;
2020
import com.google.gson.annotations.SerializedName;
2121

22-
import static cd.go.authorization.github.utils.Util.isBlank;
23-
import static cd.go.authorization.github.utils.Util.isNotBlank;
22+
import java.util.Optional;
2423

2524
public class ProfileMetadata<T extends Metadata> {
2625

@@ -37,24 +36,12 @@ public ProfileMetadata(String key, T metadata) {
3736
this.metadata = metadata;
3837
}
3938

40-
public ValidationError validate(String input) {
41-
String validationError = doValidate(input);
42-
if (isNotBlank(validationError)) {
43-
return new ValidationError(key, validationError);
44-
}
45-
return null;
39+
public Optional<ValidationError> validate(String input) {
40+
return isRequired() && (input == null || input.isBlank())
41+
? Optional.of(new ValidationError(key, this.key + " must not be blank."))
42+
: Optional.empty();
4643
}
4744

48-
protected String doValidate(String input) {
49-
if (isRequired()) {
50-
if (isBlank(input)) {
51-
return this.key + " must not be blank.";
52-
}
53-
}
54-
return null;
55-
}
56-
57-
5845
public String getKey() {
5946
return key;
6047
}

‎src/main/java/cd/go/authorization/github/annotation/ValidationResult.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package cd.go.authorization.github.annotation;
1818

19+
import org.jetbrains.annotations.NotNull;
20+
1921
import java.util.*;
2022

2123
import static cd.go.authorization.github.utils.Util.GSON;
@@ -34,10 +36,7 @@ public void addError(String key, String message) {
3436
errors.add(new ValidationError(key, message));
3537
}
3638

37-
public void addError(ValidationError validationError) {
38-
if (validationError == null) {
39-
return;
40-
}
39+
public void addError(@NotNull ValidationError validationError) {
4140
errors.add(validationError);
4241
}
4342

‎src/main/java/cd/go/authorization/github/client/GitHubClientBuilder.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import cd.go.authorization.github.models.AuthenticateWith;
2020
import cd.go.authorization.github.models.GitHubConfiguration;
21+
import com.thoughtworks.go.plugin.api.logging.Logger;
2122
import okhttp3.*;
2223
import org.kohsuke.github.GitHub;
2324
import org.kohsuke.github.GitHubBuilder;
@@ -27,9 +28,9 @@
2728

2829
import java.io.IOException;
2930

30-
import static cd.go.authorization.github.GitHubPlugin.LOG;
31-
3231
public class GitHubClientBuilder {
32+
private static final Logger LOG = Logger.getLoggerFor(GitHubClientBuilder.class);
33+
3334
public static final OkHttpClient HTTP_CLIENT = new OkHttpClient.Builder().build();
3435
private static final int OKHTTP_CACHE_MAX_AGE_SECONDS = 60;
3536
private static final GitHubConnector GITHUB_CONNECTOR = new OkHttpGitHubConnector(HTTP_CLIENT, OKHTTP_CACHE_MAX_AGE_SECONDS);

0 commit comments

Comments
 (0)