Skip to content

Commit 2dac383

Browse files
lukaszgryglickilukaszgryglicki
committed
Final updates
Signed-off-by: Lukasz Gryglicki <[email protected]> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
1 parent 79449e0 commit 2dac383

File tree

2 files changed

+69
-40
lines changed

2 files changed

+69
-40
lines changed

‎cla-backend-go/github/github_repository.go‎

Lines changed: 54 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,12 @@ Please update your commit message(s) by doing |git commit --amend| and then |git
5959
`
6060

6161
const (
62-
help = "https://help.github.com/en/github/committing-changes-to-your-project/why-are-my-commits-linked-to-the-wrong-user"
63-
unknown = "Unknown"
64-
failureState = "failure"
65-
successState = "success"
66-
svgVersion = "?v=2"
62+
help = "https://help.github.com/en/github/committing-changes-to-your-project/why-are-my-commits-linked-to-the-wrong-user"
63+
unknown = "Unknown"
64+
failureState = "failure"
65+
successState = "success"
66+
svgVersion = "?v=2"
67+
QuickCacheTTL = 5 * time.Minute
6768
)
6869

6970
type cacheEntry struct {
@@ -159,6 +160,15 @@ func (c *UserCache) Set(key [3]string, value *models.User) {
159160
}
160161
}
161162

163+
func (c *UserCache) SetWithTTL(key [3]string, value *models.User, tl time.Duration) {
164+
c.mu.Lock()
165+
defer c.mu.Unlock()
166+
c.data[key] = userCacheEntry{
167+
value: value,
168+
expiresAt: time.Now().Add(tl),
169+
}
170+
}
171+
162172
func (c *UserCache) Cleanup() {
163173
c.mu.Lock()
164174
defer c.mu.Unlock()
@@ -205,6 +215,17 @@ func (c *ProjectUserCache) Get(key [4]string) (*models.User, bool, bool, bool) {
205215
return entry.value, entry.signed, entry.affiliated, true
206216
}
207217

218+
func (c *ProjectUserCache) SetWithTTL(key [4]string, value *models.User, signed, affiliated bool, tl time.Duration) {
219+
c.mu.Lock()
220+
defer c.mu.Unlock()
221+
c.data[key] = projectUserCacheEntry{
222+
value: value,
223+
signed: signed,
224+
affiliated: affiliated,
225+
expiresAt: time.Now().Add(tl),
226+
}
227+
}
228+
208229
func (c *ProjectUserCache) Set(key [4]string, value *models.User, signed, affiliated bool) {
209230
c.mu.Lock()
210231
defer c.mu.Unlock()
@@ -231,7 +252,7 @@ func (c *ProjectUserCache) Delete(key [4]string) { c.mu.Lock(); delete(c.data, k
231252

232253
var GithubUserCache = NewCache(24 * time.Hour)
233254
var ModelUserCache = NewUserCache(24 * time.Hour)
234-
var ModelProjectUserCache = NewProjectUserCache(24 * time.Hour)
255+
var ModelProjectUserCache = NewProjectUserCache(6 * time.Hour)
235256

236257
func init() {
237258
go func() {
@@ -739,14 +760,17 @@ func GetCommitAuthorSignedStatus(
739760
unsigned *[]*UserCommitSummary,
740761
mu *sync.Mutex,
741762
) {
763+
// here userSummary is NOT nil
742764
f := logrus.Fields{
743765
"functionName": "github.github_repository.GetCommitAuthorSignedStatus",
744766
"projectID": projectID,
745-
"userSummary": *userSummary,
746767
}
747768
commitAuthorID := userSummary.GetCommitAuthorID()
748769
commitAuthorUsername := userSummary.GetCommitAuthorUsername()
749770
commitAuthorEmail := userSummary.GetCommitAuthorEmail()
771+
f["authorID"] = commitAuthorID
772+
f["authorLogin"] = commitAuthorUsername
773+
f["authorEmail"] = commitAuthorEmail
750774

751775
log.WithFields(f).Debugf("checking user - sha: %s, user ID: %s, username: %s, email: %s",
752776
userSummary.SHA, commitAuthorID, commitAuthorUsername, commitAuthorEmail)
@@ -759,7 +783,7 @@ func GetCommitAuthorSignedStatus(
759783
if cachedUser != nil {
760784
log.WithFields(f).Debugf("per-project cache: %+v -> (%+v, %v, %v, %v)", projectCacheKey, *cachedUser, authorized, affiliated, ok)
761785
} else {
762-
log.WithFields(f).Debugf("per-project cache: %+v -> (%+v, nil, %v, %v)", projectCacheKey, authorized, affiliated, ok)
786+
log.WithFields(f).Debugf("per-project cache: %+v -> (nil, %v, %v, %v)", projectCacheKey, authorized, affiliated, ok)
763787
}
764788
if ok {
765789
if cachedUser == nil {
@@ -795,22 +819,22 @@ func GetCommitAuthorSignedStatus(
795819
}
796820
if ok {
797821
if cachedUser == nil {
798-
log.WithFields(f).Debugf("general cache: unsigned, user is null")
799822
mu.Lock()
800823
*unsigned = append(*unsigned, userSummary)
801824
mu.Unlock()
802-
ModelProjectUserCache.Set(projectCacheKey, nil, false, false)
825+
log.WithFields(f).Debugf("store per-project cache: unsigned, user is null (%+v)", projectCacheKey)
826+
ModelProjectUserCache.SetWithTTL(projectCacheKey, nil, false, false, QuickCacheTTL)
803827
return
804828
}
805829
user := cachedUser
806830
userSigned, companyAffiliation, signedErr := hasUserSigned(ctx, user, projectID)
807831
if signedErr != nil {
808832
log.WithFields(f).WithError(signedErr).Warnf("has user signed error - user: %+v, project: %s", user, projectID)
809-
log.WithFields(f).Debugf("general cache: unsigned, hasUserSigned error")
810833
mu.Lock()
811834
*unsigned = append(*unsigned, userSummary)
812835
mu.Unlock()
813-
ModelProjectUserCache.Set(projectCacheKey, user, false, false)
836+
log.WithFields(f).Debugf("store per-project cache: unsigned, hasUserSigned error (%+v)", projectCacheKey)
837+
ModelProjectUserCache.SetWithTTL(projectCacheKey, user, false, false, QuickCacheTTL)
814838
return
815839
}
816840

@@ -821,24 +845,24 @@ func GetCommitAuthorSignedStatus(
821845
if userSigned != nil {
822846
userSummary.Authorized = *userSigned
823847
if userSummary.Authorized {
824-
log.WithFields(f).Debugf("general cache: signed")
825848
mu.Lock()
826849
*signed = append(*signed, userSummary)
827850
mu.Unlock()
851+
log.WithFields(f).Debugf("store per-project cache: signed (%+v)", projectCacheKey)
828852
ModelProjectUserCache.Set(projectCacheKey, user, true, userSummary.Affiliated)
829853
} else {
830-
log.WithFields(f).Debugf("general cache: unsigned, authorized is false")
831854
mu.Lock()
832855
*unsigned = append(*unsigned, userSummary)
833856
mu.Unlock()
834-
ModelProjectUserCache.Set(projectCacheKey, user, false, userSummary.Affiliated)
857+
log.WithFields(f).Debugf("store per-project cache: unsigned, authorized is false (%+v)", projectCacheKey)
858+
ModelProjectUserCache.SetWithTTL(projectCacheKey, user, false, userSummary.Affiliated, QuickCacheTTL)
835859
}
836860
} else {
837-
log.WithFields(f).Debugf("general cache: unsigned, userSigned is null")
838861
mu.Lock()
839862
*unsigned = append(*unsigned, userSummary)
840863
mu.Unlock()
841-
ModelProjectUserCache.Set(projectCacheKey, user, false, userSummary.Affiliated)
864+
log.WithFields(f).Debugf("store per-project cache: unsigned, userSigned is null (%+v)", projectCacheKey)
865+
ModelProjectUserCache.SetWithTTL(projectCacheKey, user, false, userSummary.Affiliated, QuickCacheTTL)
842866
}
843867
return
844868
}
@@ -881,25 +905,25 @@ func GetCommitAuthorSignedStatus(
881905
if user == nil {
882906
log.WithFields(f).Debugf("unable to find user for commit author - sha: %s, user ID: %s, username: %s, email: %s",
883907
userSummary.SHA, commitAuthorID, commitAuthorUsername, commitAuthorEmail)
884-
log.WithFields(f).Debugf("store caches: unsigned, user is null")
908+
log.WithFields(f).Debugf("store caches: unsigned, user is null (%+v)", projectCacheKey)
885909
mu.Lock()
886910
*unsigned = append(*unsigned, userSummary)
887911
mu.Unlock()
888-
ModelProjectUserCache.Set(projectCacheKey, nil, false, false)
889-
ModelUserCache.Set(cacheKey, nil)
912+
ModelProjectUserCache.SetWithTTL(projectCacheKey, nil, false, false, QuickCacheTTL)
913+
ModelUserCache.SetWithTTL(cacheKey, nil, QuickCacheTTL)
890914
return
891915
}
892916

893917
log.WithFields(f).Debugf("checking to see if user has signed an ICLA or ECLA for project: %s", projectID)
894918
userSigned, companyAffiliation, signedErr := hasUserSigned(ctx, user, projectID)
895919
if signedErr != nil {
896920
log.WithFields(f).WithError(signedErr).Warnf("has user signed error - user: %+v, project: %s", user, projectID)
897-
log.WithFields(f).Debugf("store caches: unsigned, hasUserSigned error")
921+
log.WithFields(f).Debugf("store caches: unsigned, hasUserSigned error (%+v)", projectCacheKey)
898922
mu.Lock()
899923
*unsigned = append(*unsigned, userSummary)
900924
mu.Unlock()
901-
ModelProjectUserCache.Set(projectCacheKey, user, false, false)
902-
ModelUserCache.Set(cacheKey, user)
925+
ModelProjectUserCache.SetWithTTL(projectCacheKey, user, false, false, QuickCacheTTL)
926+
ModelUserCache.SetWithTTL(cacheKey, user, QuickCacheTTL)
903927
return
904928
}
905929

@@ -910,27 +934,27 @@ func GetCommitAuthorSignedStatus(
910934
if userSigned != nil {
911935
userSummary.Authorized = *userSigned
912936
if userSummary.Authorized {
913-
log.WithFields(f).Debugf("store caches: signed")
937+
log.WithFields(f).Debugf("store caches: signed (%+v)", projectCacheKey)
914938
mu.Lock()
915939
*signed = append(*signed, userSummary)
916940
mu.Unlock()
917941
ModelProjectUserCache.Set(projectCacheKey, user, true, userSummary.Affiliated)
918942
ModelUserCache.Set(cacheKey, user)
919943
} else {
920-
log.WithFields(f).Debugf("store caches: unsigned, authorized is false")
944+
log.WithFields(f).Debugf("store caches: unsigned, authorized is false (%+v)", projectCacheKey)
921945
mu.Lock()
922946
*unsigned = append(*unsigned, userSummary)
923947
mu.Unlock()
924-
ModelProjectUserCache.Set(projectCacheKey, user, false, userSummary.Affiliated)
925-
ModelUserCache.Set(cacheKey, user)
948+
ModelProjectUserCache.SetWithTTL(projectCacheKey, user, false, userSummary.Affiliated, QuickCacheTTL)
949+
ModelUserCache.SetWithTTL(cacheKey, user, QuickCacheTTL)
926950
}
927951
} else {
928-
log.WithFields(f).Debugf("store caches: unsigned, userSigned is null")
952+
log.WithFields(f).Debugf("store caches: unsigned, userSigned is null (%+v)", projectCacheKey)
929953
mu.Lock()
930954
*unsigned = append(*unsigned, userSummary)
931955
mu.Unlock()
932-
ModelProjectUserCache.Set(projectCacheKey, user, false, userSummary.Affiliated)
933-
ModelUserCache.Set(cacheKey, user)
956+
ModelProjectUserCache.SetWithTTL(projectCacheKey, user, false, userSummary.Affiliated, QuickCacheTTL)
957+
ModelUserCache.SetWithTTL(cacheKey, user, QuickCacheTTL)
934958
}
935959
}
936960

‎cla-backend/cla/models/github_models.py‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
# GitHub usernames must be 3-39 characters long, can only contain alphanumeric characters or hyphens,
4040
# cannot begin or end with a hyphen, and cannot contain consecutive hyphens.
4141
GITHUB_USERNAME_REGEX = re.compile(r'^(?!-)(?!.*--)[A-Za-z0-9-]{3,39}(?<!-)$')
42+
QUICK_CACHE_TTL = 300 # 5 minutes for quick cache
4243

4344
class TTLCache:
4445
def __init__(self, ttl_seconds=86400):
@@ -61,6 +62,10 @@ def set(self, key, value):
6162
with self.lock:
6263
self.data[key] = (value, time.time() + self.ttl)
6364

65+
def set_with_ttl(self, key, value, tl):
66+
with self.lock:
67+
self.data[key] = (value, time.time() + tl)
68+
6469
def cleanup(self):
6570
with self.lock:
6671
now = time.time()
@@ -1651,7 +1656,7 @@ def handle_commit_from_user(
16511656
if user is None:
16521657
missing.append(user_commit_summary)
16531658
cla.log.debug(f"{fn} - cache: negative case: aff mode: {check_aff}")
1654-
github_user_cache.set(project_cache_key, (None, False, False, False))
1659+
github_user_cache.set_with_ttl(project_cache_key, (None, False, False, False), QUICK_CACHE_TTL)
16551660
return
16561661
if check_aff:
16571662
cla.log.debug(f"{fn} - cache: aff mode, user: {user}")
@@ -1664,7 +1669,7 @@ def handle_commit_from_user(
16641669
if user.get_user_company_id() is None:
16651670
missing.append(user_commit_summary)
16661671
cla.log.debug(f"{fn} - cache: aff mode: no company_id, missing")
1667-
github_user_cache.set(project_cache_key, (user, True, False, False))
1672+
github_user_cache.set_with_ttl(project_cache_key, (user, True, False, False), QUICK_CACHE_TTL)
16681673
return
16691674
user_commit_summary.affiliated = True
16701675
cla.log.debug(f"{fn} - cache: aff mode: affiliated")
@@ -1696,7 +1701,7 @@ def handle_commit_from_user(
16961701
else:
16971702
cla.log.debug(f"{fn} - cache: aff mode: no authorized found, adding to missing")
16981703
missing.append(user_commit_summary)
1699-
github_user_cache.set(project_cache_key, (user, True, False, True))
1704+
github_user_cache.set_with_ttl(project_cache_key, (user, True, False, True), QUICK_CACHE_TTL)
17001705
else:
17011706
cla.log.debug(f"{fn} - cache: non-aff mode, user: {user}")
17021707
if cla.utils.user_signed_project_signature(user, project):
@@ -1707,7 +1712,7 @@ def handle_commit_from_user(
17071712
return
17081713
cla.log.debug(f"{fn} - cache: non-aff mode: no authorized, missing")
17091714
missing.append(user_commit_summary)
1710-
github_user_cache.set(project_cache_key, (user, False, False, False))
1715+
github_user_cache.set_with_ttl(project_cache_key, (user, False, False, False), QUICK_CACHE_TTL)
17111716
cla.log.debug(f"{fn} - cache: done, returning")
17121717
return
17131718
# LG: cache_authors - end
@@ -1798,8 +1803,8 @@ def handle_commit_from_user(
17981803
missing.append(user_commit_summary)
17991804
# set check_aff flag to false as in this case we didn't check affiliated flag, this can also store None (negative cache)
18001805
cla.log.debug(f"{fn} - store cache non-aff mode: missing: {project_cache_key}: {users}")
1801-
github_user_cache.set(project_cache_key, (None, False, False, False))
1802-
github_user_cache.set(cache_key, (None, False))
1806+
github_user_cache.set_with_ttl(project_cache_key, (None, False, False, False), QUICK_CACHE_TTL)
1807+
github_user_cache.set_with_ttl(cache_key, (None, False), QUICK_CACHE_TTL)
18031808
else:
18041809
cla.log.debug(
18051810
f"{fn} - Found {len(users)} GitHub user(s) matching "
@@ -1832,8 +1837,8 @@ def handle_commit_from_user(
18321837
missing.append(user_commit_summary)
18331838
# set check_aff flag to true in this case, as this code branch checks for affiliation, also store only 1st user as this branches considers only 1st user
18341839
cla.log.debug(f"{fn} - store cache aff mode: no company_id: {project_cache_key}: {user}")
1835-
github_user_cache.set(project_cache_key, (user, True, False, False))
1836-
github_user_cache.set(cache_key, (user, True))
1840+
github_user_cache.set_with_ttl(project_cache_key, (user, True, False, False), QUICK_CACHE_TTL)
1841+
github_user_cache.set_with_ttl(cache_key, (user, True), QUICK_CACHE_TTL)
18371842
return
18381843

18391844
# Mark the user as having a company affiliation
@@ -1878,8 +1883,8 @@ def handle_commit_from_user(
18781883
missing.append(user_commit_summary)
18791884
# set check_aff flag to true in this case, as this code branch checks for affiliation, also store only 1st user as this branches considers only 1st user
18801885
cla.log.debug(f"{fn} - store cache aff mode: missing: {project_cache_key}: {user}")
1881-
github_user_cache.set(project_cache_key, (user, True, False, True))
1882-
github_user_cache.set(cache_key, (user, True))
1886+
github_user_cache.set_with_ttl(project_cache_key, (user, True, False, True), QUICK_CACHE_TTL)
1887+
github_user_cache.set_with_ttl(cache_key, (user, True), QUICK_CACHE_TTL)
18831888

18841889

18851890
def get_merge_group_commit_authors(merge_group_sha, installation_id=None) -> List[UserCommitSummary]:

0 commit comments

Comments
 (0)