Merge lp:~zyga/checkbox/fix-1412660 into lp:checkbox

[Zygmunt Krynicki]

7e7ea2b checkbox-ng:service: fix .via for jobs not in session

[lp:~zyga/checkbox/fix-1412660]

3536. By Zygmunt Krynicki on 2015-01-20

checkbox-ng:service: fix .via for jobs not in session

This patch fixes a crash when JobDefinitionWrapper.via was being
accessed after resuming a session, when the corresponding job was not
being a part of the session (but is still available on DBus via a
provider). For example:
 - create a provider with jobs [a, b]
 - create a session with job [a]
 - save the session before a runs
 - resume the session, the session now has [a] but not [b] in it
 - attempt to access b.via

In the current code, this would crash as there was an implicit,
incorrect assumption that each job visible on DBus is also a part of the
current session. The patch corrects that assumption by allowing no
JobState to exist for any JobDefinition.

(A copy of the relevant comment on the bug corresponding report)

1) Job definitions are either generated (and thus bound to a session) or
   not (and thus bound to a provider)
2) The .via property was previously a mutable state on a job. Now it is
   something one can access from session-bound JobState.
   2a) this means that .via cannot be accessed before a session exists
   2b) this means that .via cannot be accessed if a session doesn't
       contain a given job (despite that job existing).

In the past, via was always accessible, perhaps holding nothing or
perhaps holding stale data (from previous session run). Currently when
2b happens, we crash.

The fix is to simply return "" when there is no session or there
is no corresponding JobState in the session.

Fixes: https://bugs.launchpad.net/checkbox-ng/+bug/1412660
Signed-off-by: Zygmunt Krynicki <email address hidden>

[Sylvain Pineau (sylvain-pineau)]

That's indeed what the service must return if the job is not yet in the state map.
Thanks for the detailed investigation.

+1

review: Approve

=== modified file 'checkbox-ng/checkbox_ng/service.py'
--- checkbox-ng/checkbox_ng/service.py	2015-01-07 14:52:42 +0000
+++ checkbox-ng/checkbox_ng/service.py	2015-01-20 13:07:53 +0000
@@ -373,7 +373,12 @@
         session_obj = ServiceWrapper.get_session_obj()
         if session_obj is None:
             return ""
-        state = session_obj.job_state_map[self.native.id]
+        try:
+            state = session_obj.job_state_map[self.native.id]
+        except KeyError:
+            # See: LP: #1412660, session_obj can be None and event if it isn't,
+            # session_obj.job_state_map might *not* have self.native.id in it.
+            return ""
         if state.via_job is None:
             return ""
         return state.via_job.checksum