Merge pull request #85 from Microsoft/semver_fix

Fixing semver check issue in checkForUpdate

Author: lostintangent

CodePush.js

@@ -1,45 +1,81 @@
 'use strict';
 
-var requestFetchAdapter = require("./request-fetch-adapter.js");
-var Sdk = require("code-push/script/acquisition-sdk").AcquisitionManager;
+var { Alert } = require("./AlertAdapter");
 var NativeCodePush = require("react-native").NativeModules.CodePush;
 var PackageMixins = require("./package-mixins")(NativeCodePush);
-var { Alert } = require("./AlertAdapter");
+var requestFetchAdapter = require("./request-fetch-adapter.js");
+var Sdk = require("code-push/script/acquisition-sdk").AcquisitionManager;
+var semver = require("semver");
 
 function checkForUpdate(deploymentKey = null) {
   var config, sdk;
 
+  /*
+   * Before we ask the server if an update exists, we
+   * need to retrieve three pieces of information from the 
+   * native side: deployment key, app version (e.g. 1.0.1)
+   * and the hash of the currently running update (if there is one).
+   * This allows the client to only receive updates which are targetted
+   * for their specific deployment and version and which are actually
+   * different from the CodePush update they have already installed.
+   */
   return getConfiguration()
           .then((configResult) => {            
-            // If a deployment key was explicitly provided,
-            // then let's override the one we retrieved
-            // from the native-side of the app.
+            /*
+             * If a deployment key was explicitly provided,
+             * then let's override the one we retrieved
+             * from the native-side of the app. This allows
+             * dynamically "redirecting" end-users at different
+             * deployments (e.g. an early access deployment for insiders).
+             */
             if (deploymentKey) {
               config = Object.assign({}, configResult, { deploymentKey });
             } else {
               config = configResult;
             }
 
             sdk = getSDK(config);
+            
             // Allow dynamic overwrite of function. This is only to be used for tests.
             return module.exports.getCurrentPackage();
           })
           .then((localPackage) => {
             var queryPackage = { appVersion: config.appVersion };
-            if (localPackage && localPackage.appVersion === config.appVersion) {
+            
+            /*
+             * If the app has a previously installed update, and that update
+             * was targetted at the same app version that is currently running,
+             * then we want to use its package hash to determine whether a new
+             * release has been made on the server. Otherwise, we only need
+             * to send the app version to the server, since we are interested
+             * in any updates for current app store version, regardless of hash.
+             */
+            if (localPackage && semver.compare(localPackage.appVersion, config.appVersion) === 0) {
               queryPackage = localPackage;
             }
+            
             return new Promise((resolve, reject) => {
               sdk.queryUpdateWithCurrentPackage(queryPackage, (err, update) => {
                 if (err) {
                   return reject(err);
                 }
 
-                // Ignore updates that require a newer app version,
-                // since the end-user couldn't reliably install it
-                if (!update || update.updateAppVersion) {
+                /*
+                 * There are three cases where checkForUpdate will resolve to null:
+                 * ----------------------------------------------------------------
+                 * 1) The server said there isn't an update. This is the most common case.
+                 * 2) The server said there is an update but it requires a newer binary version.
+                 *    This would occur when end-users are running an older app store version than
+                 *    is available, and CodePush is making sure they don't get an update that
+                 *    potentially wouldn't be compatible with what they are running.
+                 * 3) The server said there is an update, but the update's hash is the same as
+                 *    the currently running update. This should _never_ happen, unless there is a
+                 *    bug in the server, but we're adding this check just to double-check that the
+                 *    client app is resilient to a potential issue with the update check.
+                 */
+                if (!update || update.updateAppVersion || (update.packageHash === localPackage.packageHash)) {
                   return resolve(null);
-                }
+                } 
 
                 update = Object.assign(update, PackageMixins.remote);
 
@@ -297,4 +333,4 @@ var CodePush = {
   }
 };
 
-module.exports = CodePush;
+module.exports = CodePush;

package.json

@@ -16,9 +16,10 @@
     "url": "https://github.com/Microsoft/react-native-code-push"
   },
   "dependencies": {
-    "code-push": "^1.1.1-beta"
+    "code-push": "^1.1.1-beta",
+    "semver": "^5.1.0"
   },
   "devDependencies": {
     "react-native": "0.14.2"
   }
-}
+}