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

Commit

Permalink
Remove version checks from extensions library and simply deserialize …
Browse files Browse the repository at this point in the history 
…every time (#28)
  • Loading branch information
@JPWilli
JPWilli authored May 13, 2019
1 parent 5cce6e7 commit 113ecc7
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 284 deletions.
43 changes: 20 additions & 23 deletions src/Microsoft.Identity.Client.Extensions.Adal/AdalCache.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,38 +80,35 @@ internal void BeforeAccessNotification(TokenCacheNotificationArgs args)
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Acquiring lock for token cache");
_cacheLock = new CrossPlatLock(Path.Combine(_store.CreationProperties.CacheDirectory, _store.CreationProperties.CacheFileName) + ".lockfile");

if (_store.HasChanged)
{
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Before access, the store has changed");
byte[] fileData = _store.ReadData();
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Read '{fileData?.Length}' bytes from storage");
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Before access, the store has changed");
byte[] fileData = _store.ReadData();
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Read '{fileData?.Length}' bytes from storage");

if (fileData != null && fileData.Length > 0)
if (fileData != null && fileData.Length > 0)
{
try
{
try
{
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Deserializing the store");
DeserializeAdalV3(fileData);
}
catch (Exception e)
{
_logger.TraceEvent(TraceEventType.Error, /*id*/ 0, $"An exception was encountered while deserializing the {nameof(AdalCache)} : {e}");
_logger.TraceEvent(TraceEventType.Error, /*id*/ 0, $"No data found in the store, clearing the cache in memory.");

// Clear the memory cache
DeserializeAdalV3(null);
_store.Clear();
throw;
}
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Deserializing the store");
DeserializeAdalV3(fileData);
}
else
catch (Exception e)
{
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"No data found in the store, clearing the cache in memory.");
_logger.TraceEvent(TraceEventType.Error, /*id*/ 0, $"An exception was encountered while deserializing the {nameof(AdalCache)} : {e}");
_logger.TraceEvent(TraceEventType.Error, /*id*/ 0, $"No data found in the store, clearing the cache in memory.");

// Clear the memory cache
DeserializeAdalV3(null);
_store.Clear();
throw;
}
}
else
{
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"No data found in the store, clearing the cache in memory.");

// Clear the memory cache
DeserializeAdalV3(null);
}
}

// Triggered right after ADAL accessed the cache.
Expand Down
69 changes: 3 additions & 66 deletions src/Microsoft.Identity.Client.Extensions.Adal/AdalCacheStorage.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ public sealed class AdalCacheStorage

internal StorageCreationProperties CreationProperties { get; }

// This is set to empty string here. During a file read, if the token isn't available, we will create a new guid, so we will always read the first time.
private string _lastVersionToken = string.Empty;

private IntPtr _libsecretSchema = IntPtr.Zero;

/// <summary>
Expand Down Expand Up @@ -63,11 +60,6 @@ public static string UserRootDirectory
/// </summary>
public string CacheFilePath => CreationProperties.CacheFilePath;

/// <summary>
/// Gets the guid representing the last time the cache was changed on disk.
/// </summary>
internal string LastVersionToken => _lastVersionToken;

/// <summary>
/// Gets the file path containing the guid representing the last time the cache was changed on disk.
/// </summary>
Expand All @@ -80,52 +72,9 @@ public bool HasChanged
{
get
{
bool versionFileExists = File.Exists(VersionFilePath);
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Has the store changed");
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"VersionFileExists '{versionFileExists}'");

if (!versionFileExists)
{
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"The version file does not exist, treat as 'changed'.");
// Attempts to make this more refined have all resulted in some form of cache inconsistency. Just returning
// true here so we always load from disk.
return true;
}

string currentVersion = File.ReadAllText(VersionFilePath);
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Current version '{currentVersion}' Last version '{_lastVersionToken}'");

bool hasChanged = !currentVersion.Equals(_lastVersionToken, StringComparison.OrdinalIgnoreCase);

_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Cache has changed '{hasChanged}'");
return hasChanged;
}
}

/// <summary>
/// Writes a new guid to the file at <see cref="VersionFilePath"/>, and updates <see cref="LastVersionToken"/>.
/// </summary>
private void WriteVersionFile()
{
try
{
string newVersion = Guid.NewGuid().ToString();
File.WriteAllText(VersionFilePath, newVersion);
_lastVersionToken = newVersion;
}
catch (IOException ex)
{
_logger.TraceEvent(TraceEventType.Warning, /*id*/ 0, $"Unable to write version file due to exception: '{ex.Message}'");
}
}

private void CacheLastReadVersion()
{
try
{
_lastVersionToken = File.ReadAllText(VersionFilePath);
}
catch (IOException ex)
{
_logger.TraceEvent(TraceEventType.Warning, /*id*/ 0, $"Unable to read version file due to exception: '{ex.Message}'");
}
}

Expand All @@ -143,16 +92,6 @@ public byte[] ReadData()
bool alreadyLoggedException = false;
try
{
// Guarantee that the version file exists so that we can know if it changes.
if (!File.Exists(VersionFilePath))
{
WriteVersionFile();
}
else
{
CacheLastReadVersion();
}

_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Reading Data");
byte[] fileData = ReadDataCore();

Expand Down Expand Up @@ -380,7 +319,6 @@ private void WriteDataCore(byte[] data)
TryProcessFile(() =>
{
File.WriteAllBytes(CacheFilePath, data);
WriteVersionFile();
});
}

Expand All @@ -402,8 +340,7 @@ private void ClearCore()
_logger.TraceEvent(TraceEventType.Error, /*id*/ 0, $"Problem deleting the cache file '{e}'");
}

WriteVersionFile();
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"After deleting the cache file. Last write time is '{_lastVersionToken}'");
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"After deleting the cache file.");
});

if (SharedUtilities.IsMacPlatform())
Expand Down
58 changes: 20 additions & 38 deletions src/Microsoft.Identity.Client.Extensions.Msal/MsalCacheHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public class MsalCacheHelper
/// Contains a reference to all caches currently registered to synchronize with this MsalCacheHelper, along with
/// timestamp of the cache file the last time they deserialized.
/// </summary>
internal readonly Dictionary<ITokenCache, string> _registeredCaches = new Dictionary<ITokenCache, string>();
internal readonly HashSet<ITokenCache> _registeredCaches = new HashSet<ITokenCache>();

/// <summary>
/// Creates a new instance of <see cref="MsalCacheHelper"/>.
Expand Down Expand Up @@ -213,7 +213,7 @@ public void RegisterCache(ITokenCache tokenCache)
lock (_lockObject)
{
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Registering token cache with on disk storage");
if (_registeredCaches.ContainsKey(tokenCache))
if (_registeredCaches.Contains(tokenCache))
{
_logger.TraceEvent(TraceEventType.Warning, /*id*/ 0, $"Redundant registration of {nameof(tokenCache)} in {nameof(MsalCacheHelper)}, skipping further registration.");
return;
Expand Down Expand Up @@ -248,7 +248,8 @@ public void RegisterCache(ITokenCache tokenCache)
}
}

_registeredCaches[tokenCache] = _store.LastVersionToken;
_registeredCaches.Add(tokenCache); // Ignore return value, since we already bail if _registeredCaches contains tokenCache earlier

_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Done initializing");
}
}
Expand All @@ -264,7 +265,7 @@ public void UnregisterCache(ITokenCache tokenCache)
{
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Unregistering token cache from on disk storage");

if (_registeredCaches.ContainsKey(tokenCache))
if (_registeredCaches.Contains(tokenCache))
{
_registeredCaches.Remove(tokenCache);
tokenCache.SetBeforeAccess(args => { });
Expand Down Expand Up @@ -308,33 +309,25 @@ internal void BeforeAccessNotification(TokenCacheNotificationArgs args)
// 2. Use _lockObject which is used in UnregisterCache, and is needed for all accesses of _registeredCaches.
CacheLock = CreateCrossPlatLock(_storageCreationProperties);

if (_store.HasChanged)
{
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Before access, the store has changed");
_cachedStoreData = _store.ReadData();
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Read '{_cachedStoreData?.Length}' bytes from storage");
}
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Before access, the store has changed");
_cachedStoreData = _store.ReadData();
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Read '{_cachedStoreData?.Length}' bytes from storage");

lock (_lockObject)
{
if ((!_registeredCaches.ContainsKey(args.TokenCache)) || !string.Equals(_registeredCaches[args.TokenCache], _store.LastVersionToken, StringComparison.OrdinalIgnoreCase))
try
{
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Deserializing the store");
args.TokenCache.DeserializeMsalV3(_cachedStoreData, shouldClearExistingCache: true);
}
catch (Exception e)
{
_logger.TraceEvent(TraceEventType.Error, /*id*/ 0, $"An exception was encountered while deserializing the {nameof(MsalCacheHelper)} : {e}");
_logger.TraceEvent(TraceEventType.Error, /*id*/ 0, $"No data found in the store, clearing the cache in memory.");

try
{
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Deserializing the store");
args.TokenCache.DeserializeMsalV3(_cachedStoreData, shouldClearExistingCache: true);
_registeredCaches[args.TokenCache] = _store.LastVersionToken;
}
catch (Exception e)
{
_logger.TraceEvent(TraceEventType.Error, /*id*/ 0, $"An exception was encountered while deserializing the {nameof(MsalCacheHelper)} : {e}");
_logger.TraceEvent(TraceEventType.Error, /*id*/ 0, $"No data found in the store, clearing the cache in memory.");

// Clear the memory cache
Clear();
throw;
}
// Clear the memory cache
Clear();
throw;
}
}
}
Expand All @@ -357,21 +350,10 @@ internal void AfterAccessNotification(TokenCacheNotificationArgs args)
{
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Before Write Store");
byte[] data = args.TokenCache.SerializeMsalV3();
_cachedStoreData = data;

_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Serializing '{data.Length}' bytes");
_store.WriteData(data);

try
{
// The _store.LastVersionToken is written when we call _store.ReadData, but we know that we're
// up-to-date here, so read the actual version from the file, so we can skip deserializing next time.
var versionToken = File.ReadAllText(_store.VersionFilePath);
_registeredCaches[args.TokenCache] = versionToken;
}
catch (IOException ex)
{
_logger.TraceEvent(TraceEventType.Warning, /*id*/ 0, $"MsalCacheHelper: Unable to read version file due to exception: '{ex.Message}'");
}
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"After write store");
}
catch (Exception e)
Expand Down
67 changes: 4 additions & 63 deletions src/Microsoft.Identity.Client.Extensions.Msal/MsalCacheStorage.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@ public sealed class MsalCacheStorage
internal readonly StorageCreationProperties _creationProperties;
private readonly TraceSource _logger;

// This is set to empty string here. During a file read, if the token isn't available, we will create a new guid, so we will always read the first time.
private string _lastVersionToken = string.Empty;

private IntPtr _libsecretSchema = IntPtr.Zero;

/// <summary>
Expand All @@ -42,60 +39,16 @@ public MsalCacheStorage(StorageCreationProperties creationProperties, TraceSourc
/// </summary>
public string CacheFilePath => _creationProperties.CacheFilePath;

internal string VersionFilePath => CacheFilePath + ".version";

internal string LastVersionToken => string.Copy(_lastVersionToken);

/// <summary>
/// Gets a value indicating whether the persisted file has changed since we last read it.
/// </summary>
public bool HasChanged
{
get
{
bool versionFileExists = File.Exists(VersionFilePath);
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Has the store changed");
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"VersionFileExists '{versionFileExists}'");

if (!versionFileExists)
{
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"The version file does not exist, treat as 'changed'.");
return true;
}

string currentVersion = File.ReadAllText(VersionFilePath);
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Current version '{currentVersion}' Last version '{_lastVersionToken}'");

bool hasChanged = !currentVersion.Equals(_lastVersionToken, StringComparison.OrdinalIgnoreCase);

_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Cache has changed '{hasChanged}'");
return hasChanged;
}
}

private void WriteVersionFile()
{
try
{
string newVersion = Guid.NewGuid().ToString();
File.WriteAllText(VersionFilePath, newVersion);
_lastVersionToken = newVersion;
}
catch (IOException ex)
{
_logger.TraceEvent(TraceEventType.Warning, /*id*/ 0, $"Unable to write version file due to exception: '{ex.Message}'");
}
}

private void CacheLastReadVersion()
{
try
{
_lastVersionToken = File.ReadAllText(VersionFilePath);
}
catch (IOException ex)
{
_logger.TraceEvent(TraceEventType.Warning, /*id*/ 0, $"Unable to read version file due to exception: '{ex.Message}'");
// Attempts to make this more refined have all resulted in some form of cache inconsistency. Just returning
// true here so we always load from disk.
return true;
}
}

Expand All @@ -113,16 +66,6 @@ public byte[] ReadData()
bool alreadyLoggedException = false;
try
{
// Guarantee that the version file exists so that we can know if it changes.
if (!File.Exists(VersionFilePath))
{
WriteVersionFile();
}
else
{
CacheLastReadVersion();
}

_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"Reading Data");
byte[] fileData = ReadDataCore();

Expand Down Expand Up @@ -350,7 +293,6 @@ private void WriteDataCore(byte[] data)
TryProcessFile(() =>
{
File.WriteAllBytes(CacheFilePath, data);
WriteVersionFile();
});
}

Expand All @@ -372,8 +314,7 @@ private void ClearCore()
_logger.TraceEvent(TraceEventType.Error, /*id*/ 0, $"Problem deleting the cache file '{e}'");
}

WriteVersionFile();
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"After deleting the cache file. Last version token is '{_lastVersionToken}'");
_logger.TraceEvent(TraceEventType.Information, /*id*/ 0, $"After deleting the cache file.");
});

if (SharedUtilities.IsMacPlatform())
Expand Down
Loading

0 comments on commit 113ecc7

Please sign in to comment.