From e20054ec7bfd1d2acd9f00bff8b666c46873ac49 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Wed, 3 Oct 2018 13:53:27 -0400 Subject: [PATCH] Functionality to cancel cache invalidation Correctly handling errors with tasks Cleanup Adding an additional warning message Avoid creating a new lambda for every invalidation --- src/GitHub.Api/Cache/CacheInterfaces.cs | 3 +- src/GitHub.Api/Git/Repository.cs | 36 ++++++++++++++----- .../Editor/GitHub.Unity/ApplicationCache.cs | 22 ++++++------ src/tests/IntegrationTests/CachingClasses.cs | 22 ++++++++---- 4 files changed, 57 insertions(+), 26 deletions(-) diff --git a/src/GitHub.Api/Cache/CacheInterfaces.cs b/src/GitHub.Api/Cache/CacheInterfaces.cs index 35487451e..be6bab1f1 100644 --- a/src/GitHub.Api/Cache/CacheInterfaces.cs +++ b/src/GitHub.Api/Cache/CacheInterfaces.cs @@ -40,6 +40,7 @@ public interface IManagedCache bool ValidateData(); void InvalidateData(); + void ResetInvalidation(); DateTimeOffset LastUpdatedAt { get; } CacheType CacheType { get; } @@ -91,7 +92,7 @@ public interface IBranchCache : IManagedCache ILocalConfigBranchDictionary LocalConfigBranches { get; } IRemoteConfigBranchDictionary RemoteConfigBranches { get; } IConfigRemoteDictionary ConfigRemotes { get; } - + void SetRemotes(Dictionary remoteConfigs, Dictionary> configBranches, GitRemote[] gitRemotes, GitBranch[] gitBranches); void SetLocals(Dictionary configBranches, GitBranch[] gitBranches); } diff --git a/src/GitHub.Api/Git/Repository.cs b/src/GitHub.Api/Git/Repository.cs index 61935cccc..9cceea0bc 100644 --- a/src/GitHub.Api/Git/Repository.cs +++ b/src/GitHub.Api/Git/Repository.cs @@ -208,20 +208,22 @@ private void CacheHasBeenInvalidated(CacheType cacheType) switch (cacheType) { case CacheType.Branches: - repositoryManager?.UpdateBranches().Start(); + repositoryManager?.UpdateBranches().Catch(ex => InvalidationFailed(ex, cacheType)).Start(); break; case CacheType.GitLog: - repositoryManager?.UpdateGitLog().Start(); + repositoryManager?.UpdateGitLog().Catch(ex => InvalidationFailed(ex, cacheType)).Start(); break; case CacheType.GitAheadBehind: - repositoryManager?.UpdateGitAheadBehindStatus().Start(); + repositoryManager?.UpdateGitAheadBehindStatus().Catch(ex => InvalidationFailed(ex, cacheType)).Start(); break; case CacheType.GitLocks: if (CurrentRemote != null) - repositoryManager?.UpdateLocks().Start(); + { + repositoryManager?.UpdateLocks().Catch(ex => InvalidationFailed(ex, cacheType)).Start(); + } break; case CacheType.GitUser: @@ -229,11 +231,11 @@ private void CacheHasBeenInvalidated(CacheType cacheType) break; case CacheType.RepositoryInfo: - repositoryManager?.UpdateRepositoryInfo().Start(); + repositoryManager?.UpdateRepositoryInfo().Catch(ex => InvalidationFailed(ex, cacheType)).Start(); break; case CacheType.GitStatus: - repositoryManager?.UpdateGitStatus().Start(); + repositoryManager?.UpdateGitStatus().Catch(ex => InvalidationFailed(ex, cacheType)).Start(); break; default: @@ -241,6 +243,15 @@ private void CacheHasBeenInvalidated(CacheType cacheType) } } + private bool InvalidationFailed(Exception ex, CacheType cacheType) + { + Logger.Warning(ex, "Error invalidating {0}", cacheType); + var managedCache = cacheContainer.GetCache(cacheType); + managedCache.ResetInvalidation(); + return false; + } + + private void RepositoryManagerOnCurrentBranchUpdated(ConfigBranch? branch, ConfigRemote? remote, string head) { taskManager.RunInUI(() => @@ -254,7 +265,7 @@ private void RepositoryManagerOnCurrentBranchUpdated(ConfigBranch? branch, Confi name = null; cloneUrl = null; cacheContainer.RepositoryInfoCache.UpdateData(data); - + // force refresh of the Name and CloneUrl propertys var n = Name; }); @@ -485,6 +496,7 @@ private void UpdateUserAndEmail() } gitClient.GetConfigUserAndEmail() + .Catch(InvalidationFailed) .ThenInUI((success, value) => { if (success) @@ -494,7 +506,15 @@ private void UpdateUserAndEmail() } }).Start(); } - + + private bool InvalidationFailed(Exception ex) + { + Logger.Warning(ex, "Error invalidating user cache"); + var managedCache = cacheContainer.GetCache(CacheType.GitUser); + managedCache.ResetInvalidation(); + return false; + } + public string Name { get { return cacheContainer.GitUserCache.Name; } diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/ApplicationCache.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/ApplicationCache.cs index 0a2160be2..a1fca0391 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/ApplicationCache.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/ApplicationCache.cs @@ -213,12 +213,14 @@ public void InvalidateData() private void Invalidate() { - if (!isInvalidating) - { - isInvalidating = true; - LastUpdatedAt = DateTimeOffset.MinValue; - CacheInvalidated.SafeInvoke(CacheType); - } + isInvalidating = true; + LastUpdatedAt = DateTimeOffset.MinValue; + CacheInvalidated.SafeInvoke(CacheType); + } + + public void ResetInvalidation() + { + isInvalidating = false; } protected void SaveData(DateTimeOffset now, bool isChanged) @@ -463,25 +465,25 @@ public void UpdateData(IRepositoryInfoCacheData data) isUpdated = true; } - if (forcedInvalidation ||!Nullable.Equals(currentGitBranch, data.CurrentGitBranch)) + if (forcedInvalidation || !Nullable.Equals(currentGitBranch, data.CurrentGitBranch)) { currentGitBranch = data.CurrentGitBranch ?? GitBranch.Default; isUpdated = true; } - if (forcedInvalidation ||!Nullable.Equals(currentConfigRemote, data.CurrentConfigRemote)) + if (forcedInvalidation || !Nullable.Equals(currentConfigRemote, data.CurrentConfigRemote)) { currentConfigRemote = data.CurrentConfigRemote ?? ConfigRemote.Default; isUpdated = true; } - if (forcedInvalidation ||!Nullable.Equals(currentConfigBranch, data.CurrentConfigBranch)) + if (forcedInvalidation || !Nullable.Equals(currentConfigBranch, data.CurrentConfigBranch)) { currentConfigBranch = data.CurrentConfigBranch ?? ConfigBranch.Default; isUpdated = true; } - if (forcedInvalidation ||!String.Equals(currentHead, data.CurrentHead)) + if (forcedInvalidation || !String.Equals(currentHead, data.CurrentHead)) { currentHead = data.CurrentHead; isUpdated = true; diff --git a/src/tests/IntegrationTests/CachingClasses.cs b/src/tests/IntegrationTests/CachingClasses.cs index 944c584e1..3c25d210c 100644 --- a/src/tests/IntegrationTests/CachingClasses.cs +++ b/src/tests/IntegrationTests/CachingClasses.cs @@ -109,6 +109,7 @@ abstract class ManagedCacheBase : ScriptObjectSingleton where T : class, I private DateTimeOffset? initializedAtValue; private bool isInvalidating; + protected bool forcedInvalidation; public event Action CacheInvalidated; public event Action CacheUpdated; @@ -134,13 +135,20 @@ public bool ValidateData() public void InvalidateData() { - if (!isInvalidating) - { - Logger.Trace("Invalidate"); - isInvalidating = true; - LastUpdatedAt = DateTimeOffset.MinValue; - CacheInvalidated.SafeInvoke(CacheType); - } + forcedInvalidation = true; + Invalidate(); + } + + private void Invalidate() + { + isInvalidating = true; + LastUpdatedAt = DateTimeOffset.MinValue; + CacheInvalidated.SafeInvoke(CacheType); + } + + public void ResetInvalidation() + { + isInvalidating = false; } protected void SaveData(DateTimeOffset now, bool isChanged)