nebraska: Detect update/install operation.

An install request is distinguished from an update request based on
whether or not the platform app includes an update_check tag. Currently,
we don't have any reliable way of detecting whether an appid is for the
platform app, since update engine does not distinguish between DLCs and
the platform app when constructing a request. Since update requests are
never mixed with events or pings, we make the assumption that if the
update_check tag is omitted from any app request, it is the platform app
and the request is for an install.

BUG=chromium:892450
TEST=Unittests still pass, manual testing.

Change-Id: I14cfe82395fdaf6e9f1e7853e077facf73d48dcf
Reviewed-on: https://chromium-review.googlesource.com/1336774
Commit-Ready: Colin Howes <chowes@google.com>
Tested-by: Colin Howes <chowes@google.com>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
diff --git a/nebraska/nebraska.py b/nebraska/nebraska.py
index 54dfeeb..24ed526 100755
--- a/nebraska/nebraska.py
+++ b/nebraska/nebraska.py
@@ -152,18 +152,26 @@
       logging.error("Request string is not valid XML (%s)", str(err))
       raise ValueError
 
+    # TODO(chowes): It would be better to specifically check the platform app.
     # An install is signalled by omitting the update check for the platform
     # app, which can be found based on the presense of a hardware_class tag,
-    # which is absent on DLC update and install requests.
-    platform_app = next(iter([x for x in request_root.findall(self.APP_TAG) if
-                              x.get(self.HW_CLASS_ATTR) is not None]), None)
-    if platform_app is not None:
-      is_install = platform_app.find(self.UPDATE_CHECK_TAG) is None
-    else:
-      is_install = False
+    # which is absent on DLC update and install requests. UE does not currently
+    # omit hardware_class for DLCs, so we assume that if we have one appid for
+    # which the update_check tag is omitted, it is the platform app and this is
+    # an install request. This assumption should be fine since we never mix
+    # updates with requests that do not include an update_check tag.
+    app_elements = request_root.findall(self.APP_TAG)
+    noop_count = len(
+        [x for x in app_elements if x.find(self.UPDATE_CHECK_TAG) is None])
+
+    if noop_count > 1 and noop_count < len(app_elements):
+      raise ValueError("Client request omits update_check tag for more than "
+                       "one, but not all app requests.")
+
+    is_install = noop_count == 1
 
     app_requests = []
-    for app in request_root.findall(self.APP_TAG):
+    for app in app_elements:
       appid = app.get(self.APPID_ATTR)
       version = app.get(self.VERSION_ATTR)
       delta_okay = app.get(self.DELTA_OKAY_ATTR) == "true"
diff --git a/nebraska/request_unittest.py b/nebraska/request_unittest.py
index 22d270d..5bb0b4e 100755
--- a/nebraska/request_unittest.py
+++ b/nebraska/request_unittest.py
@@ -51,6 +51,22 @@
 </request>
 """
 
+  INVALID_NOOP_REQUEST = """<?xml version="1.0" encoding="UTF-8"?>
+<request protocol="3.0">
+  <os version="Indy" platform="Chrome OS" sp="10323.52.0_x86_64"></os>
+  <app appid="platform" version="1.0.0" hardware_class="c" delta_okay="true">
+    <ping active="1" a="1" r="1"></ping>
+  </app>
+  <app appid="foo" version="1.0.0" delta_okay="true">
+    <ping active="1" a="1" r="1"></ping>
+  </app>
+  <app appid="bar" version="1.0.0" delta_okay="false">
+    <ping active="1" a="1" r="1"></ping>
+    <updatecheck targetversionprefix=""></updatecheck>
+  </app>
+</request>
+"""
+
   EVENT_REQUEST = """<?xml version="1.0" encoding="UTF-8"?>
 <request protocol="3.0">
   <os version="Indy" platform="Chrome OS" sp="10323.52.0_x86_64"></os>
@@ -115,6 +131,12 @@
       request = nebraska.Request(XMLStrings.INVALID_INSTALL_REQUEST)
       request.ParseRequest()
 
+  def testParseRequestInvalidNoop(self):
+    """Tests ParseRequest handling of invalid mixed no-op request."""
+    with self.assertRaises(ValueError):
+      request = nebraska.Request(XMLStrings.INVALID_NOOP_REQUEST)
+      request.ParseRequest()
+
   def testParseRequestInstall(self):
     """Tests ParseRequest handling of install requests."""
     request = nebraska.Request(XMLStrings.INSTALL_REQUEST)
@@ -135,7 +157,7 @@
     self.assertTrue(app_requests[2].version == "1.0.0")
 
   def testParseRequestUpdate(self):
-    """Tests ParseRequest handling of install requests."""
+    """Tests ParseRequest handling of update requests."""
     request = nebraska.Request(XMLStrings.UPDATE_REQUEST)
     app_requests = request.ParseRequest()