CR feedback

Author: Geoffrey Goh

windows/CodePush.csproj

@@ -109,9 +109,8 @@
   <ItemGroup>
     <Compile Include="CodePushReactPackage.cs" />
     <Compile Include="CodePushConstants.cs" />
-    <Compile Include="CodePushResumeListener.cs" />
+    <Compile Include="CodePushLifecycleEventListener.cs" />
     <Compile Include="InstallMode.cs" />
-    <Compile Include="CodePushInvalidUpdateException.cs" />
     <Compile Include="CodePushNativeModule.cs" />
     <Compile Include="UpdateManager.cs" />
     <Compile Include="UpdateState.cs" />

windows/CodePushInvalidUpdateException.cs

@@ -3,16 +3,16 @@
 
 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 @@ public void OnResume()
                 double durationInBackground = (new DateTime() - (DateTime)_lastSuspendDate).TotalSeconds;
                 if (durationInBackground >= MinimumBackgroundDuration)
                 {
-                    Action loadBundleAction = async () =>
-                    {
-                        await _codePushNativeModule.LoadBundle();
-                    };
-
-                    _codePushNativeModule.Context.RunOnNativeModulesQueueThread(loadBundleAction);
+                    _loadBundleAction.Invoke();
                 }
             }
         }

windows/CodePushResumeListener.cs renamed to windows/CodePushLifecycleEventListener.cs

@@ -4,6 +4,7 @@
 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 @@ public override IReadOnlyDictionary<string, object> Constants
             {
                 return new Dictionary<string, object>
                 {
-                    { "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 @@ await _codePush.UpdateManager.DownloadPackage(
                                     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<RCTDeviceEventEmitter>()
                                     .emit(CodePushConstants.DownloadProgressEventName, downloadProgress);
@@ -83,7 +87,7 @@ await _codePush.UpdateManager.DownloadPackage(
                     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 @@ public void getConfiguration(IPromise promise)
             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 @@ public void getUpdateMetadata(int updateState, IPromise promise)
                 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 @@ public void installUpdate(JObject updatePackage, int installMode, int minimumBac
         {
             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 @@ public void installUpdate(JObject updatePackage, int installMode, int minimumBac
         [ReactMethod]
         public void isFailedUpdate(string packageHash, IPromise promise)
         {
-            promise.Resolve(_codePush.IsFailedHash(packageHash));
+            promise.Resolve(SettingsManager.IsFailedHash(packageHash));
         }
 
         [ReactMethod]
@@ -245,7 +257,7 @@ public void restartApp(bool onlyIfUpdateIsPending)
             {
                 // 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();
                 }

windows/CodePushNativeModule.cs

@@ -48,7 +48,7 @@ public CodePushReactPackage(string deploymentKey, ReactPage mainPage)
             CurrentInstance = this;
         }
 
-        // Public methods
+        #region Public methods
         public IReadOnlyList<Type> CreateJavaScriptModulesConfig()
         {
             return new List<Type>();
@@ -131,8 +131,9 @@ public async Task<string> GetJavaScriptBundleFileAsync(string assetsBundleFileNa
                 return binaryJsBundleUrl;
             }
         }
+        #endregion
 
-        // Internal methods
+        #region Internal methods
         internal async Task<long> GetBinaryResourcesModifiedTime()
         {
             var assetJSBundleFile = await StorageFile.GetFileFromApplicationUriAsync(new Uri(CodePushConstants.AssetsBundlePrefix + AssetsBundleFileName));
@@ -169,40 +170,15 @@ internal void InitializeUpdateAfterRestart()
             }
         }
 
-        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 @@ private async Task RollbackPackage()
             await UpdateManager.RollbackPackage();
             SettingsManager.RemovePendingUpdate();
         }
+        #endregion
     }
 }

windows/CodePushReactPackage.cs

@@ -42,9 +42,9 @@ internal static string GetDeviceId()
         {
             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);

windows/CodePushUtils.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 @@ internal async static Task MergeDirectories(StorageFolder source, StorageFolder
             foreach (StorageFolder sourceDirectory in await source.GetFoldersAsync())
             {
                 StorageFolder nextTargetSubDir = await target.CreateFolderAsync(sourceDirectory.Name, CreationCollisionOption.OpenIfExists);
-                await MergeDirectories(sourceDirectory, nextTargetSubDir);
+                await MergeFolders(sourceDirectory, nextTargetSubDir);
             }
         }
     }

windows/FileUtils.cs

@@ -2,8 +2,8 @@
 {
     enum InstallMode
     {
-        IMMEDIATE = 0,
-        ON_NEXT_RESTART = 1,
-        ON_NEXT_RESUME = 2
+        Immediate,
+        OnNextRestart,
+        OnNextResume
     }
 }