From 08283efe2d924436699a08c25a976974169e8fef Mon Sep 17 00:00:00 2001 From: Geoffrey Goh Date: Thu, 5 May 2016 10:37:38 -0700 Subject: [PATCH] CR feedback --- windows/CodePush.csproj | 3 +- windows/CodePushInvalidUpdateException.cs | 12 ---- ...r.cs => CodePushLifecycleEventListener.cs} | 15 ++--- windows/CodePushNativeModule.cs | 54 +++++++++------- windows/CodePushReactPackage.cs | 35 ++--------- windows/CodePushUtils.cs | 4 +- windows/FileUtils.cs | 4 +- windows/InstallMode.cs | 6 +- windows/SettingsManager.cs | 63 ++++++++++++------- windows/UpdateManager.cs | 12 ++-- windows/UpdateState.cs | 6 +- windows/UpdateUtils.cs | 2 +- 12 files changed, 104 insertions(+), 112 deletions(-) delete mode 100644 windows/CodePushInvalidUpdateException.cs rename windows/{CodePushResumeListener.cs => CodePushLifecycleEventListener.cs} (65%) diff --git a/windows/CodePush.csproj b/windows/CodePush.csproj index d614c07..ad3c48d 100644 --- a/windows/CodePush.csproj +++ b/windows/CodePush.csproj @@ -109,9 +109,8 @@ - + - diff --git a/windows/CodePushInvalidUpdateException.cs b/windows/CodePushInvalidUpdateException.cs deleted file mode 100644 index 3acf04a..0000000 --- a/windows/CodePushInvalidUpdateException.cs +++ /dev/null @@ -1,12 +0,0 @@ -using System; - -namespace CodePush.ReactNative -{ - internal class CodePushInvalidUpdateException : Exception - { - internal CodePushInvalidUpdateException(string message) - : base(message) - { - } - } -} diff --git a/windows/CodePushResumeListener.cs b/windows/CodePushLifecycleEventListener.cs similarity index 65% rename from windows/CodePushResumeListener.cs rename to windows/CodePushLifecycleEventListener.cs index 4b6ad12..5dc3552 100644 --- a/windows/CodePushResumeListener.cs +++ b/windows/CodePushLifecycleEventListener.cs @@ -3,16 +3,16 @@ using System; namespace CodePush.ReactNative { - internal class CodePushResumeListener : ILifecycleEventListener + internal class CodePushLifecycleEventListener : ILifecycleEventListener { private DateTime? _lastSuspendDate = null; - private CodePushNativeModule _codePushNativeModule; + private Action _loadBundleAction; internal int MinimumBackgroundDuration { get; set; } - internal CodePushResumeListener(CodePushNativeModule codePushNativeModule, int minimumBackgroundDuration) + internal CodePushLifecycleEventListener(Action loadBundleAction, int minimumBackgroundDuration) { - _codePushNativeModule = codePushNativeModule; + _loadBundleAction = loadBundleAction; MinimumBackgroundDuration = minimumBackgroundDuration; } @@ -29,12 +29,7 @@ namespace CodePush.ReactNative double durationInBackground = (new DateTime() - (DateTime)_lastSuspendDate).TotalSeconds; if (durationInBackground >= MinimumBackgroundDuration) { - Action loadBundleAction = async () => - { - await _codePushNativeModule.LoadBundle(); - }; - - _codePushNativeModule.Context.RunOnNativeModulesQueueThread(loadBundleAction); + _loadBundleAction.Invoke(); } } } diff --git a/windows/CodePushNativeModule.cs b/windows/CodePushNativeModule.cs index 3262032..bd0a4df 100644 --- a/windows/CodePushNativeModule.cs +++ b/windows/CodePushNativeModule.cs @@ -4,6 +4,7 @@ using ReactNative.Bridge; using ReactNative.Modules.Core; using System; using System.Collections.Generic; +using System.IO; using System.Reflection; using System.Threading.Tasks; using Windows.Web.Http; @@ -12,7 +13,7 @@ namespace CodePush.ReactNative { internal class CodePushNativeModule : ReactContextNativeModuleBase { - private CodePushResumeListener _codePushLifecycleEventListener = null; + private CodePushLifecycleEventListener _codePushLifecycleEventListener = null; private ReactContext _reactContext; private CodePushReactPackage _codePush; @@ -36,12 +37,12 @@ namespace CodePush.ReactNative { return new Dictionary { - { "codePushInstallModeImmediate", InstallMode.ON_NEXT_RESTART }, - { "codePushInstallModeOnNextResume", InstallMode.ON_NEXT_RESUME }, - { "codePushInstallModeOnNextRestart", InstallMode.ON_NEXT_RESTART }, - { "codePushUpdateStateRunning", UpdateState.RUNNING }, - { "codePushUpdateStatePending", UpdateState.PENDING }, - { "codePushUpdateStateLatest", UpdateState.LATEST }, + { "codePushInstallModeImmediate", InstallMode.Immediate }, + { "codePushInstallModeOnNextResume", InstallMode.OnNextResume }, + { "codePushInstallModeOnNextRestart", InstallMode.OnNextRestart }, + { "codePushUpdateStateRunning", UpdateState.Running }, + { "codePushUpdateStatePending", UpdateState.Pending }, + { "codePushUpdateStateLatest", UpdateState.Lastest }, }; } } @@ -70,9 +71,12 @@ namespace CodePush.ReactNative return; } - var downloadProgress = new JObject(); - downloadProgress["totalBytes"] = progress.TotalBytesToReceive; - downloadProgress["receivedBytes"] = progress.BytesReceived; + var downloadProgress = new JObject() + { + { "totalBytes", progress.TotalBytesToReceive }, + { "receivedBytes", progress.BytesReceived } + }; + _reactContext .GetJavaScriptModule() .emit(CodePushConstants.DownloadProgressEventName, downloadProgress); @@ -83,7 +87,7 @@ namespace CodePush.ReactNative JObject newPackage = await _codePush.UpdateManager.GetPackage((string)updatePackage[CodePushConstants.PackageHashKey]); promise.Resolve(newPackage); } - catch (CodePushInvalidUpdateException e) + catch (InvalidDataException e) { CodePushUtils.Log(e.ToString()); SettingsManager.SaveFailedUpdate(updatePackage); @@ -105,9 +109,9 @@ namespace CodePush.ReactNative var config = new JObject { { "appVersion", _codePush.AppVersion }, - { "deploymentKey", _codePush.DeploymentKey }, - { "serverUrl", CodePushConstants.CodePushServerUrl }, { "clientUniqueId", CodePushUtils.GetDeviceId() }, + { "deploymentKey", _codePush.DeploymentKey }, + { "serverUrl", CodePushConstants.CodePushServerUrl } }; // TODO generate binary hash @@ -136,16 +140,16 @@ namespace CodePush.ReactNative if (currentPackage[CodePushConstants.PackageHashKey] != null) { var currentHash = (string)currentPackage[CodePushConstants.PackageHashKey]; - currentUpdateIsPending = _codePush.IsPendingUpdate(currentHash); + currentUpdateIsPending = SettingsManager.IsPendingUpdate(currentHash); } - if (updateState == (int)UpdateState.PENDING && !currentUpdateIsPending) + if (updateState == (int)UpdateState.Pending && !currentUpdateIsPending) { // The caller wanted a pending update // but there isn't currently one. promise.Resolve(""); } - else if (updateState == (int)UpdateState.RUNNING && currentUpdateIsPending) + else if (updateState == (int)UpdateState.Running && currentUpdateIsPending) { // The caller wants the running update, but the current // one is pending, so we need to grab the previous. @@ -187,15 +191,23 @@ namespace CodePush.ReactNative { Action installUpdateAction = async () => { - await _codePush.UpdateManager.InstallPackage(updatePackage, _codePush.IsPendingUpdate(null)); + await _codePush.UpdateManager.InstallPackage(updatePackage, SettingsManager.IsPendingUpdate(null)); var pendingHash = (string)updatePackage[CodePushConstants.PackageHashKey]; SettingsManager.SavePendingUpdate(pendingHash, /* isLoading */false); - if (installMode == (int)InstallMode.ON_NEXT_RESUME) + if (installMode == (int)InstallMode.OnNextResume) { if (_codePushLifecycleEventListener == null) { // Ensure we do not add the listener twice. - _codePushLifecycleEventListener = new CodePushResumeListener(this, minimumBackgroundDuration); + Action loadBundleAction = () => + { + Context.RunOnNativeModulesQueueThread(async () => + { + await LoadBundle(); + }); + }; + + _codePushLifecycleEventListener = new CodePushLifecycleEventListener(loadBundleAction, minimumBackgroundDuration); _reactContext.AddLifecycleEventListener(_codePushLifecycleEventListener); } else @@ -213,7 +225,7 @@ namespace CodePush.ReactNative [ReactMethod] public void isFailedUpdate(string packageHash, IPromise promise) { - promise.Resolve(_codePush.IsFailedHash(packageHash)); + promise.Resolve(SettingsManager.IsFailedHash(packageHash)); } [ReactMethod] @@ -245,7 +257,7 @@ namespace CodePush.ReactNative { // If this is an unconditional restart request, or there // is current pending update, then reload the app. - if (!onlyIfUpdateIsPending || _codePush.IsPendingUpdate(null)) + if (!onlyIfUpdateIsPending || SettingsManager.IsPendingUpdate(null)) { await LoadBundle(); } diff --git a/windows/CodePushReactPackage.cs b/windows/CodePushReactPackage.cs index 1ae7f93..9a7f321 100644 --- a/windows/CodePushReactPackage.cs +++ b/windows/CodePushReactPackage.cs @@ -48,7 +48,7 @@ namespace CodePush.ReactNative CurrentInstance = this; } - // Public methods + #region Public methods public IReadOnlyList CreateJavaScriptModulesConfig() { return new List(); @@ -131,8 +131,9 @@ namespace CodePush.ReactNative return binaryJsBundleUrl; } } + #endregion - // Internal methods + #region Internal methods internal async Task GetBinaryResourcesModifiedTime() { var assetJSBundleFile = await StorageFile.GetFileFromApplicationUriAsync(new Uri(CodePushConstants.AssetsBundlePrefix + AssetsBundleFileName)); @@ -169,40 +170,15 @@ namespace CodePush.ReactNative } } - internal bool IsFailedHash(string packageHash) - { - JArray failedUpdates = SettingsManager.GetFailedUpdates(); - if (packageHash != null) - { - foreach (var failedPackage in failedUpdates) - { - var failedPackageHash = (string)failedPackage[CodePushConstants.PackageHashKey]; - if (packageHash.Equals(failedPackageHash)) - { - return true; - } - } - } - - return false; - } - - internal bool IsPendingUpdate(string packageHash) - { - JObject pendingUpdate = SettingsManager.GetPendingUpdate(); - return pendingUpdate != null && - !(bool)pendingUpdate[CodePushConstants.PendingUpdateIsLoadingKey] && - (packageHash == null || ((string)pendingUpdate[CodePushConstants.PendingUpdateHashKey]).Equals(packageHash)); - } - internal async Task ClearUpdates() { await UpdateManager.ClearUpdates(); SettingsManager.RemovePendingUpdate(); SettingsManager.RemoveFailedUpdates(); } + #endregion - // Private methods + #region Private methods private async Task ClearReactDevBundleCache() { var devBundleCacheFile = (StorageFile) await ApplicationData.Current.LocalFolder.TryGetItemAsync(CodePushConstants.ReactDevBundleCacheFileName); @@ -219,5 +195,6 @@ namespace CodePush.ReactNative await UpdateManager.RollbackPackage(); SettingsManager.RemovePendingUpdate(); } + #endregion } } \ No newline at end of file diff --git a/windows/CodePushUtils.cs b/windows/CodePushUtils.cs index 0f1744b..23b2e60 100644 --- a/windows/CodePushUtils.cs +++ b/windows/CodePushUtils.cs @@ -42,9 +42,9 @@ namespace CodePush.ReactNative { HardwareToken token = HardwareIdentification.GetPackageSpecificToken(null); IBuffer hardwareId = token.Id; - DataReader dataReader = DataReader.FromBuffer(hardwareId); + var dataReader = DataReader.FromBuffer(hardwareId); - byte[] bytes = new byte[hardwareId.Length]; + var bytes = new byte[hardwareId.Length]; dataReader.ReadBytes(bytes); return BitConverter.ToString(bytes); diff --git a/windows/FileUtils.cs b/windows/FileUtils.cs index d5db7ef..ec7313b 100644 --- a/windows/FileUtils.cs +++ b/windows/FileUtils.cs @@ -6,7 +6,7 @@ namespace CodePush.ReactNative { internal class FileUtils { - internal async static Task MergeDirectories(StorageFolder source, StorageFolder target) + internal async static Task MergeFolders(StorageFolder source, StorageFolder target) { foreach (StorageFile sourceFile in await source.GetFilesAsync()) { @@ -16,7 +16,7 @@ namespace CodePush.ReactNative foreach (StorageFolder sourceDirectory in await source.GetFoldersAsync()) { StorageFolder nextTargetSubDir = await target.CreateFolderAsync(sourceDirectory.Name, CreationCollisionOption.OpenIfExists); - await MergeDirectories(sourceDirectory, nextTargetSubDir); + await MergeFolders(sourceDirectory, nextTargetSubDir); } } } diff --git a/windows/InstallMode.cs b/windows/InstallMode.cs index 5ace374..c407127 100644 --- a/windows/InstallMode.cs +++ b/windows/InstallMode.cs @@ -2,8 +2,8 @@ { enum InstallMode { - IMMEDIATE = 0, - ON_NEXT_RESTART = 1, - ON_NEXT_RESUME = 2 + Immediate, + OnNextRestart, + OnNextResume } } \ No newline at end of file diff --git a/windows/SettingsManager.cs b/windows/SettingsManager.cs index ef44979..b8250f7 100644 --- a/windows/SettingsManager.cs +++ b/windows/SettingsManager.cs @@ -7,15 +7,11 @@ namespace CodePush.ReactNative { internal class SettingsManager { - private static ApplicationDataContainer GetCodePushSettings() - { - return ApplicationData.Current.LocalSettings.CreateContainer(CodePushConstants.CodePushPreferences, ApplicationDataCreateDisposition.Always); - } + private static ApplicationDataContainer Settings = ApplicationData.Current.LocalSettings.CreateContainer(CodePushConstants.CodePushPreferences, ApplicationDataCreateDisposition.Always); public static JArray GetFailedUpdates() { - ApplicationDataContainer settings = GetCodePushSettings(); - var failedUpdatesString = (string)settings.Values[CodePushConstants.FailedUpdatesKey]; + var failedUpdatesString = (string)Settings.Values[CodePushConstants.FailedUpdatesKey]; if (failedUpdatesString == null) { return new JArray(); @@ -28,15 +24,14 @@ namespace CodePush.ReactNative catch (Exception) { var emptyArray = new JArray(); - settings.Values[CodePushConstants.FailedUpdatesKey] = JsonConvert.SerializeObject(emptyArray); + Settings.Values[CodePushConstants.FailedUpdatesKey] = JsonConvert.SerializeObject(emptyArray); return emptyArray; } } internal static JObject GetPendingUpdate() { - ApplicationDataContainer settings = GetCodePushSettings(); - var pendingUpdateString = (string)settings.Values[CodePushConstants.PendingUpdateKey]; + var pendingUpdateString = (string)Settings.Values[CodePushConstants.PendingUpdateKey]; if (pendingUpdateString == null) { return null; @@ -55,22 +50,45 @@ namespace CodePush.ReactNative } } + internal static bool IsFailedHash(string packageHash) + { + JArray failedUpdates = SettingsManager.GetFailedUpdates(); + if (packageHash != null) + { + foreach (var failedPackage in failedUpdates) + { + var failedPackageHash = (string)failedPackage[CodePushConstants.PackageHashKey]; + if (packageHash.Equals(failedPackageHash)) + { + return true; + } + } + } + + return false; + } + + internal static bool IsPendingUpdate(string packageHash) + { + JObject pendingUpdate = SettingsManager.GetPendingUpdate(); + return pendingUpdate != null && + !(bool)pendingUpdate[CodePushConstants.PendingUpdateIsLoadingKey] && + (packageHash == null || ((string)pendingUpdate[CodePushConstants.PendingUpdateHashKey]).Equals(packageHash)); + } + internal static void RemoveFailedUpdates() { - ApplicationDataContainer settings = GetCodePushSettings(); - settings.Values.Remove(CodePushConstants.FailedUpdatesKey); + Settings.Values.Remove(CodePushConstants.FailedUpdatesKey); } internal static void RemovePendingUpdate() { - ApplicationDataContainer settings = GetCodePushSettings(); - settings.Values.Remove(CodePushConstants.PendingUpdateKey); + Settings.Values.Remove(CodePushConstants.PendingUpdateKey); } internal static void SaveFailedUpdate(JObject failedPackage) { - ApplicationDataContainer settings = GetCodePushSettings(); - var failedUpdatesString = (string)settings.Values[CodePushConstants.FailedUpdatesKey]; + var failedUpdatesString = (string)Settings.Values[CodePushConstants.FailedUpdatesKey]; JArray failedUpdates; if (failedUpdatesString == null) { @@ -82,17 +100,18 @@ namespace CodePush.ReactNative } failedUpdates.Add(failedPackage); - settings.Values[CodePushConstants.FailedUpdatesKey] = JsonConvert.SerializeObject(failedUpdates); + Settings.Values[CodePushConstants.FailedUpdatesKey] = JsonConvert.SerializeObject(failedUpdates); } internal static void SavePendingUpdate(string packageHash, bool isLoading) { - ApplicationDataContainer settings = GetCodePushSettings(); - var pendingUpdate = new JObject(); - pendingUpdate[CodePushConstants.PendingUpdateHashKey] = packageHash; - pendingUpdate[CodePushConstants.PendingUpdateIsLoadingKey] = isLoading; - settings.Values[CodePushConstants.PendingUpdateKey] = JsonConvert.SerializeObject(pendingUpdate); - } + var pendingUpdate = new JObject() + { + { CodePushConstants.PendingUpdateHashKey, packageHash }, + { CodePushConstants.PendingUpdateIsLoadingKey, isLoading } + }; + Settings.Values[CodePushConstants.PendingUpdateKey] = JsonConvert.SerializeObject(pendingUpdate); + } } } \ No newline at end of file diff --git a/windows/UpdateManager.cs b/windows/UpdateManager.cs index ec8ec34..313dba7 100644 --- a/windows/UpdateManager.cs +++ b/windows/UpdateManager.cs @@ -13,7 +13,7 @@ namespace CodePush.ReactNative { internal class UpdateManager { - // Internal methods + #region Internal methods internal async Task ClearUpdates() { await (await GetCodePushFolder()).DeleteAsync(); @@ -60,14 +60,14 @@ namespace CodePush.ReactNative StorageFolder currentPackageFolder = await GetCurrentPackageFolder(); if (currentPackageFolder == null) { - throw new CodePushInvalidUpdateException("Received a diff update, but there is no current version to diff against."); + throw new InvalidDataException("Received a diff update, but there is no current version to diff against."); } await UpdateUtils.CopyNecessaryFilesFromCurrentPackage(diffManifestFile, currentPackageFolder, newUpdateFolder); await diffManifestFile.DeleteAsync(); } - await FileUtils.MergeDirectories(unzippedFolder, newUpdateFolder); + await FileUtils.MergeFolders(unzippedFolder, newUpdateFolder); await unzippedFolder.DeleteAsync(); // For zip updates, we need to find the relative path to the jsBundle and save it in the @@ -75,7 +75,7 @@ namespace CodePush.ReactNative string relativeBundlePath = await UpdateUtils.FindJSBundleInUpdateContents(newUpdateFolder, expectedBundleFileName); if (relativeBundlePath == null) { - throw new CodePushInvalidUpdateException("Update is invalid - A JS bundle file named \"" + expectedBundleFileName + "\" could not be found within the downloaded contents. Please check that you are releasing your CodePush updates using the exact same JS bundle file name that was shipped with your app's binary."); + throw new InvalidDataException("Update is invalid - A JS bundle file named \"" + expectedBundleFileName + "\" could not be found within the downloaded contents. Please check that you are releasing your CodePush updates using the exact same JS bundle file name that was shipped with your app's binary."); } else { @@ -218,8 +218,9 @@ namespace CodePush.ReactNative info[CodePushConstants.PreviousPackageKey] = null; await UpdateCurrentPackageInfo(info); } + #endregion - // Private methods + #region Private methods private async Task GetCodePushFolder() { return await ApplicationData.Current.LocalFolder.CreateFolderAsync(CodePushConstants.CodePushFolderPrefix, CreationCollisionOption.OpenIfExists); @@ -265,5 +266,6 @@ namespace CodePush.ReactNative { await FileIO.WriteTextAsync(await GetStatusFile(), JsonConvert.SerializeObject(packageInfo)); } + #endregion } } diff --git a/windows/UpdateState.cs b/windows/UpdateState.cs index 8af2535..a480b0c 100644 --- a/windows/UpdateState.cs +++ b/windows/UpdateState.cs @@ -2,8 +2,8 @@ { enum UpdateState { - RUNNING = 0, - PENDING = 1, - LATEST = 2 + Running, + Pending, + Lastest } } \ No newline at end of file diff --git a/windows/UpdateUtils.cs b/windows/UpdateUtils.cs index bcb6156..026febf 100644 --- a/windows/UpdateUtils.cs +++ b/windows/UpdateUtils.cs @@ -10,7 +10,7 @@ namespace CodePush.ReactNative { internal async static Task CopyNecessaryFilesFromCurrentPackage(StorageFile diffManifestFile, StorageFolder currentPackageFolder, StorageFolder newPackageFolder) { - await FileUtils.MergeDirectories(currentPackageFolder, newPackageFolder); + await FileUtils.MergeFolders(currentPackageFolder, newPackageFolder); JObject diffManifest = await CodePushUtils.GetJObjectFromFile(diffManifestFile); var deletedFiles = (JArray)diffManifest["deletedFiles"]; foreach (string fileNameToDelete in deletedFiles)