Skip to content
This repository has been archived by the owner on Oct 2, 2023. It is now read-only.

Commit

Permalink
scmAuthorName fix (#246)
Browse files Browse the repository at this point in the history
* scmAuthorname fix

* getLDAPDN fix

* scmAuthorname fix
  • Loading branch information
nireeshT authored Oct 18, 2021
1 parent c5653d0 commit 30fc661
Show file tree
Hide file tree
Showing 10 changed files with 78 additions and 15 deletions.
6 changes: 3 additions & 3 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<artifactId>api</artifactId>
<packaging>jar</packaging>
<name>${project.groupId}:${project.artifactId}</name>
<version>3.4.24</version>
<version>3.4.25</version>
<description>Hygieia Rest API Layer</description>
<url>https://github.com/Hygieia/api</url>

Expand Down Expand Up @@ -59,9 +59,9 @@

<properties>
<!-- Dependencies -->
<com.capitalone.dashboard.core.version>3.15.16</com.capitalone.dashboard.core.version>
<com.capitalone.dashboard.core.version>3.15.20</com.capitalone.dashboard.core.version>
<spring-security.version>4.2.18.RELEASE</spring-security.version>
<tomcat.version>8.5.57</tomcat.version>
<tomcat.version>8.5.70</tomcat.version>
<commons-beanutils.version>1.9.4</commons-beanutils.version>
<commons-codec.version>1.14</commons-codec.version>
<commons-collections4.version>4.1</commons-collections4.version>
Expand Down
11 changes: 11 additions & 0 deletions src/main/java/com/capitalone/dashboard/settings/ApiSettings.java
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ public class ApiSettings {
@Value("${encryptRemoteCreatePayload:true}")
private boolean encryptRemoteCreatePayload;

@Value("${optimizeUserCallsToGithub:true}")
private boolean optimizeUserCallsToGithub;

private String hygieia_ui_url="";

public Map<String, String> getFunctional() {
Expand Down Expand Up @@ -256,4 +259,12 @@ public String getContextSecurityAuthentication() {
public void setContextSecurityAuthentication(String contextSecurityAuthentication) {
this.contextSecurityAuthentication = contextSecurityAuthentication;
}

public boolean isOptimizeUserCallsToGithub() {
return optimizeUserCallsToGithub;
}

public void setOptimizeUserCallsToGithub(boolean optimizeUserCallsToGithub) {
this.optimizeUserCallsToGithub = optimizeUserCallsToGithub;
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package com.capitalone.dashboard.webhook.github;

import com.capitalone.dashboard.model.GitHubCollector;
import com.capitalone.dashboard.model.UserEntitlements;
import com.capitalone.dashboard.repository.BaseCollectorRepository;
import com.capitalone.dashboard.repository.CollectorItemRepository;
import com.capitalone.dashboard.repository.UserEntitlementsRepository;
import com.capitalone.dashboard.settings.ApiSettings;
import com.capitalone.dashboard.client.RestClient;
import com.capitalone.dashboard.model.webhook.github.GitHubParsed;
Expand Down Expand Up @@ -47,9 +49,10 @@ public GitHubCommitV3(CollectorService collectorService,
CommitRepository commitRepository,
GitRequestRepository gitRequestRepository,
CollectorItemRepository collectorItemRepository,
UserEntitlementsRepository userEntitlementsRepository,
ApiSettings apiSettings,
BaseCollectorRepository<GitHubCollector> collectorRepository) {
super(collectorService, restClient, apiSettings, collectorItemRepository, collectorRepository);
super(collectorService, restClient, apiSettings, collectorItemRepository, userEntitlementsRepository, collectorRepository);

this.commitRepository = commitRepository;
this.gitRequestRepository = gitRequestRepository;
Expand Down Expand Up @@ -174,6 +177,9 @@ protected List<Commit> getCommits(List<Map> commitListPayload, String repoUrl,
String authorLogin = (userObject == null) ? "unknown" : restClient.getString(userObject, "login");
commit.setScmAuthorLogin(authorLogin);

String scmAuthorName = userObject == null ? null : restClient.getString(userObject, "name");
commit.setScmAuthorName(scmAuthorName);

if (senderObj != null && authorLogin.equalsIgnoreCase(restClient.getString(senderObj, "login"))) {
String authorType = restClient.getString(senderObj, "type");
if (!StringUtils.isEmpty(authorType)) {
Expand All @@ -198,7 +204,10 @@ protected List<Commit> getCommits(List<Map> commitListPayload, String repoUrl,
long end = System.currentTimeMillis();
LOG.debug("Time to fetch LDAPDN = "+(end-start));
}

// if ldap dn is null set it from ldapMap
if(StringUtils.isEmpty(commit.getScmAuthorLDAPDN())){
commit.setScmAuthorLDAPDN(getLDAPDN(repoUrl, StringUtils.isEmpty(scmAuthorName) ? authorLogin : scmAuthorName, gitHubWebHookToken));
}
// Set the Committer details. This in the case of a merge commit is the user who merges the PR.
// In the case of a regular commit, it is usually set to a default "name": "GitHub Enterprise", and login is null
Object committerObject = restClient.getAsObject(node, "committer");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.capitalone.dashboard.model.GitHubCollector;
import com.capitalone.dashboard.repository.BaseCollectorRepository;
import com.capitalone.dashboard.repository.CollectorItemRepository;
import com.capitalone.dashboard.repository.UserEntitlementsRepository;
import com.capitalone.dashboard.settings.ApiSettings;
import com.capitalone.dashboard.client.RestClient;
import com.capitalone.dashboard.repository.CommitRepository;
Expand All @@ -28,6 +29,7 @@ public class GitHubHookServiceImpl implements GitHubHookService {
private final GitRequestRepository gitRequestRepository;
private final CollectorItemRepository collectorItemRepository;
private final BaseCollectorRepository<GitHubCollector> collectorRepository;
private final UserEntitlementsRepository userEntitlementsRepository;
private final CollectorService collectorService;
protected final ApiSettings apiSettings;
protected final RestClient restClient;
Expand All @@ -37,13 +39,15 @@ public GitHubHookServiceImpl(CommitRepository commitRepository,
GitRequestRepository gitRequestRepository,
CollectorService collectorService,
CollectorItemRepository collectorItemRepository,
UserEntitlementsRepository userEntitlementsRepository,
ApiSettings apiSettings,
RestClient restClient,
BaseCollectorRepository<GitHubCollector> collectorRepository) {
this.commitRepository = commitRepository;
this.gitRequestRepository = gitRequestRepository;
this.collectorItemRepository = collectorItemRepository;
this.collectorRepository = collectorRepository;
this.userEntitlementsRepository = userEntitlementsRepository;
this.collectorService = collectorService;
this.apiSettings = apiSettings;
this.restClient = restClient;
Expand All @@ -67,15 +71,15 @@ public String createFromGitHubv3(JSONObject request) throws ParseException, Hygi

switch (payloadType) {
case Push:
gitHubv3 = new GitHubCommitV3(collectorService, restClient, commitRepository, gitRequestRepository, collectorItemRepository, apiSettings, collectorRepository);
gitHubv3 = new GitHubCommitV3(collectorService, restClient, commitRepository, gitRequestRepository, collectorItemRepository, userEntitlementsRepository, apiSettings, collectorRepository);
break;

case PullRequest:
gitHubv3 = new GitHubPullRequestV3(collectorService, restClient, gitRequestRepository, commitRepository, collectorItemRepository, apiSettings, collectorRepository);
gitHubv3 = new GitHubPullRequestV3(collectorService, restClient, gitRequestRepository, commitRepository, collectorItemRepository, userEntitlementsRepository, apiSettings, collectorRepository);
break;

case Issues:
gitHubv3 = new GitHubIssueV3(collectorService, restClient, gitRequestRepository, collectorItemRepository, apiSettings, collectorRepository);
gitHubv3 = new GitHubIssueV3(collectorService, restClient, gitRequestRepository, collectorItemRepository, userEntitlementsRepository , apiSettings, collectorRepository);
break;

default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.capitalone.dashboard.model.GitHubCollector;
import com.capitalone.dashboard.repository.BaseCollectorRepository;
import com.capitalone.dashboard.repository.CollectorItemRepository;
import com.capitalone.dashboard.repository.UserEntitlementsRepository;
import com.capitalone.dashboard.settings.ApiSettings;
import com.capitalone.dashboard.client.RestClient;
import com.capitalone.dashboard.model.webhook.github.GitHubParsed;
Expand All @@ -20,14 +21,17 @@
public class GitHubIssueV3 extends GitHubV3 {
private final GitRequestRepository gitRequestRepository;


public GitHubIssueV3(CollectorService collectorService,
RestClient restClient,
GitRequestRepository gitRequestRepository,
CollectorItemRepository collectorItemRepository,
UserEntitlementsRepository userEntitlementsRepository,
ApiSettings apiSettings,
BaseCollectorRepository<GitHubCollector> collectorRepository) {
super(collectorService, restClient, apiSettings, collectorItemRepository, collectorRepository);
super(collectorService, restClient, apiSettings, collectorItemRepository, userEntitlementsRepository, collectorRepository);
this.gitRequestRepository = gitRequestRepository;

}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.capitalone.dashboard.model.PullRequestEvent;
import com.capitalone.dashboard.repository.BaseCollectorRepository;
import com.capitalone.dashboard.repository.CollectorItemRepository;
import com.capitalone.dashboard.repository.UserEntitlementsRepository;
import com.capitalone.dashboard.settings.ApiSettings;
import com.capitalone.dashboard.client.RestClient;
import com.capitalone.dashboard.model.webhook.github.GitHubParsed;
Expand Down Expand Up @@ -49,9 +50,10 @@ public GitHubPullRequestV3(CollectorService collectorService,
GitRequestRepository gitRequestRepository,
CommitRepository commitRepository,
CollectorItemRepository collectorItemRepository,
UserEntitlementsRepository userEntitlementsRepository,
ApiSettings apiSettings,
BaseCollectorRepository<GitHubCollector> collectorRepository) {
super(collectorService, restClient, apiSettings, collectorItemRepository, collectorRepository);
super(collectorService, restClient, apiSettings, collectorItemRepository, userEntitlementsRepository, collectorRepository);

this.gitRequestRepository = gitRequestRepository;
this.commitRepository = commitRepository;
Expand Down Expand Up @@ -422,11 +424,14 @@ protected List<Commit> getPRCommits(String repoUrl, Object commitsObject, GitReq
JSONObject authorUserJSON = (JSONObject) author.get("user");
newCommit.setScmAuthor(restClient.getString(author, "name"));
newCommit.setScmAuthorLogin((authorUserJSON == null) ? "unknown" : restClient.getString(authorUserJSON, "login"));
String scmAuthorName = authorUserJSON == null ? null : restClient.getString(authorUserJSON, "name");
newCommit.setScmAuthorName(scmAuthorName);

String authorType = getAuthorType(repoUrl, newCommit.getScmAuthorLogin(), token);
if (!StringUtils.isEmpty(authorType)) {
newCommit.setScmAuthorType(authorType);
}
String authorLDAPDN = getLDAPDN(repoUrl, newCommit.getScmAuthorLogin(), token);
String authorLDAPDN = getLDAPDN(repoUrl, StringUtils.isEmpty(scmAuthorName) ? newCommit.getScmAuthorLogin() : scmAuthorName, token);
if (!StringUtils.isEmpty(authorLDAPDN)) {
newCommit.setScmAuthorLDAPDN(authorLDAPDN);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package com.capitalone.dashboard.webhook.github;

import com.capitalone.dashboard.model.AuthType;
import com.capitalone.dashboard.model.GitHubCollector;
import com.capitalone.dashboard.model.UserEntitlements;
import com.capitalone.dashboard.repository.BaseCollectorRepository;
import com.capitalone.dashboard.repository.CollectorItemRepository;
import com.capitalone.dashboard.model.webhook.github.GitHubRepo;
import com.capitalone.dashboard.repository.UserEntitlementsRepository;
import com.capitalone.dashboard.settings.ApiSettings;
import com.capitalone.dashboard.client.RestClient;
import com.capitalone.dashboard.model.webhook.github.GitHubParsed;
Expand Down Expand Up @@ -44,14 +47,19 @@ public abstract class GitHubV3 {
protected final ApiSettings apiSettings;
protected final CollectorItemRepository collectorItemRepository;
private final BaseCollectorRepository<GitHubCollector> collectorRepository;
private UserEntitlementsRepository userEntitlementsRepository;
private static final String ENTITLEMENT_TYPE = "distinguishedName";
private Map<String, String> authorTypeMap;


private Map<String, String> ldapMap;
private Map<String, String> authorTypeMap;


public GitHubV3(CollectorService collectorService,
RestClient restClient,
ApiSettings apiSettings,
CollectorItemRepository collectorItemRepository,
UserEntitlementsRepository userEntitlementsRepository,
BaseCollectorRepository<GitHubCollector> collectorRepository
) {
this.collectorService = collectorService;
Expand All @@ -61,6 +69,7 @@ public GitHubV3(CollectorService collectorService,
this.collectorRepository = collectorRepository;
ldapMap = new HashMap<>();
authorTypeMap = new HashMap<>();
this.userEntitlementsRepository = userEntitlementsRepository;
}

public GitHubCollector getCollector() {
Expand Down Expand Up @@ -181,6 +190,16 @@ protected void getUser(String repoUrl, String user, String token) {
String formattedUser = user.replace("_", "-");
int retryCount = 0;
ResponseEntity<String> response;

if(apiSettings.isOptimizeUserCallsToGithub()) {
UserEntitlements entitlements = userEntitlementsRepository.findTopByAuthTypeAndEntitlementTypeAndUsername(AuthType.LDAP,
ENTITLEMENT_TYPE, StringUtils.lowerCase(user));
String ldapDN = (entitlements == null) ? "" : entitlements.getEntitlements();
ldapMap.put(user, ldapDN);
authorTypeMap.put(user, "User");
return;
}

while(true) {
try {
long start = System.currentTimeMillis();
Expand Down Expand Up @@ -218,6 +237,11 @@ protected void getUser(String repoUrl, String user, String token) {
protected String getLDAPDN(String repoUrl, String user, String token) {
if (StringUtils.isEmpty(user) || "unknown".equalsIgnoreCase(user)) return null;
if (ldapMap == null) { ldapMap = new HashMap<>(); }

if(apiSettings.isOptimizeUserCallsToGithub()) {
return ldapMap.get(user);
}

//This is weird. Github does replace the _ in commit author with - in the user api!!!
String formattedUser = user.replace("_", "-");
if(ldapMap.containsKey(formattedUser)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import com.capitalone.dashboard.repository.CollectorItemRepository;
import com.capitalone.dashboard.repository.CommitRepository;
import com.capitalone.dashboard.repository.GitRequestRepository;
import com.capitalone.dashboard.repository.UserEntitlementsRepository;
import com.capitalone.dashboard.service.CollectorService;
import com.capitalone.dashboard.settings.ApiSettings;
import com.capitalone.dashboard.webhook.settings.GitHubWebHookSettings;
Expand Down Expand Up @@ -61,6 +62,7 @@ public class GitHubCommitV3Test {
@Mock private CommitRepository commitRepository;
@Mock private GitRequestRepository gitRequestRepository;
@Mock private CollectorItemRepository collectorItemRepository;
@Mock private UserEntitlementsRepository userEntitlementsRepository;
@Mock private BaseCollectorRepository<GitHubCollector> collectorRepository;
@Mock private ApiSettings apiSettings;
@Mock private RestOperationsSupplier restOperationsSupplier;
Expand All @@ -72,7 +74,7 @@ public class GitHubCommitV3Test {
public void init() {
RestClient restClientTemp = new RestClient(restOperationsSupplier);
restClient = Mockito.spy(restClientTemp);
gitHubCommitV3 = new GitHubCommitV3 (collectorService, restClient, commitRepository, gitRequestRepository, collectorItemRepository, apiSettings, collectorRepository);
gitHubCommitV3 = new GitHubCommitV3 (collectorService, restClient, commitRepository, gitRequestRepository, collectorItemRepository, userEntitlementsRepository, apiSettings, collectorRepository);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.capitalone.dashboard.repository.BaseCollectorRepository;
import com.capitalone.dashboard.repository.CollectorItemRepository;
import com.capitalone.dashboard.repository.GitRequestRepository;
import com.capitalone.dashboard.repository.UserEntitlementsRepository;
import com.capitalone.dashboard.service.CollectorService;
import com.capitalone.dashboard.settings.ApiSettings;
import org.apache.commons.logging.Log;
Expand Down Expand Up @@ -43,6 +44,7 @@ public class GitHubIssueV3Test {
@Mock private GitRequestRepository gitRequestRepository;
@Mock private CollectorItemRepository collectorItemRepository;
@Mock private BaseCollectorRepository<GitHubCollector> collectorRepository;
@Mock private UserEntitlementsRepository userEntitlementsRepository;
@Mock private ApiSettings apiSettings;
@Mock private RestOperationsSupplier restOperationsSupplier;

Expand All @@ -51,7 +53,7 @@ public class GitHubIssueV3Test {
@Before
public void init() {
RestClient restClient = new RestClient(restOperationsSupplier);
gitHubIssueV3 = new GitHubIssueV3 (collectorService, restClient, gitRequestRepository, collectorItemRepository, apiSettings, collectorRepository);
gitHubIssueV3 = new GitHubIssueV3 (collectorService, restClient, gitRequestRepository, collectorItemRepository, userEntitlementsRepository, apiSettings, collectorRepository);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import com.capitalone.dashboard.repository.CollectorItemRepository;
import com.capitalone.dashboard.repository.CommitRepository;
import com.capitalone.dashboard.repository.GitRequestRepository;
import com.capitalone.dashboard.repository.UserEntitlementsRepository;
import com.capitalone.dashboard.service.CollectorService;
import com.capitalone.dashboard.settings.ApiSettings;
import org.apache.commons.logging.Log;
Expand Down Expand Up @@ -57,6 +58,7 @@ public class GitHubPullRequestV3Test {
@Mock private CollectorItemRepository collectorItemRepository;
@Mock private BaseCollectorRepository<GitHubCollector> collectorRepository;
@Mock private CommitRepository commitRepository;
@Mock private UserEntitlementsRepository userEntitlementsRepository;
@Mock private ApiSettings apiSettings;
@Mock private RestOperationsSupplier restOperationsSupplier;

Expand All @@ -67,7 +69,7 @@ public class GitHubPullRequestV3Test {
@Before
public void init() {
restClient = new RestClient(restOperationsSupplier);
gitHubPullRequestV3 = new GitHubPullRequestV3 (collectorService, restClient, gitRequestRepository, commitRepository, collectorItemRepository, apiSettings, collectorRepository);
gitHubPullRequestV3 = new GitHubPullRequestV3 (collectorService, restClient, gitRequestRepository, commitRepository, collectorItemRepository, userEntitlementsRepository, apiSettings, collectorRepository);
payLoadJsonObject = makePullRequestPayloadObject();
}

Expand Down

0 comments on commit 30fc661

Please sign in to comment.