Handle issues as dict rather than Issue

bugwarrior/collect.py

@@ -38,7 +38,7 @@ def _aggregate_issues(conf, main_section, target, queue):
             issue_count += 1
     except SystemExit as e:
         log.critical(f"Worker for [{target}] exited: {e}")
-        queue.put((SERVICE_FINISHED_ERROR, (target, e)))
+        queue.put((SERVICE_FINISHED_ERROR, target))
     except BaseException as e:
         if hasattr(e, 'request') and e.request:
             # Exceptions raised by requests library have the HTTP request
@@ -47,10 +47,10 @@ def _aggregate_issues(conf, main_section, target, queue):
             # methods. There is no one left to call these hooks anyway.
             e.request.hooks = {}
         log.exception(f"Worker for [{target}] failed: {e}")
-        queue.put((SERVICE_FINISHED_ERROR, (target, e)))
+        queue.put((SERVICE_FINISHED_ERROR, target))
     else:
         log.debug(f"Worker for [{target}] finished ok.")
-        queue.put((SERVICE_FINISHED_OK, (target, issue_count, )))
+        queue.put((SERVICE_FINISHED_OK, target))
     finally:
         duration = time.time() - start
         log.info(f"Done with [{target}] in {duration}.")
@@ -87,14 +87,16 @@ def aggregate_issues(conf, main_section, debug):
     currently_running = len(targets)
     while currently_running > 0:
         issue = queue.get(True)
-        if isinstance(issue, tuple):
-            currently_running -= 1
-            completion_type, args = issue
-            if completion_type == SERVICE_FINISHED_ERROR:
-                target, e = args
-                log.error(f"Aborted [{target}] due to critical error.")
-                yield ('SERVICE FAILED', target)
-            continue
-        yield issue
+        try:
+            yield issue.get_taskwarrior_record()
+        except AttributeError:
+            if isinstance(issue, tuple):
+                currently_running -= 1
+                completion_type, target = issue
+                if completion_type == SERVICE_FINISHED_ERROR:
+                    log.error(f"Aborted [{target}] due to critical error.")
+                    yield ('SERVICE FAILED', target)
+                continue
+            yield issue
 
     log.info("Done aggregating remote issues.")

bugwarrior/db.py

@@ -98,19 +98,11 @@ def get_managed_task_uuids(tw, key_list):
     return expected_task_ids
 
 
-def make_unique_identifier(keys, issue):
+def make_unique_identifier(keys: dict, issue: dict) -> str:
     """ For a given issue, make an identifier from its unique keys.
 
     This is not the same as the taskwarrior uuid, which is assigned
     only once the task is created.
-
-    :params:
-    * `keys`: A list of lists of keys to use for uniquely identifying
-      an issue.
-    * `issue`: An instance of a subclass of `bugwarrior.services.Issue`.
-
-    :returns:
-    * A single string UUID.
     """
     for service, key_list in keys.items():
         if all([key in issue for key in key_list]):
@@ -326,54 +318,47 @@ def synchronize(issue_generator, conf, main_section, dry_run=False):
         'closed': [],
     }
 
-    issue_map = {}  # unique identifier -> issue dict
+    issue_map = {}  # unique identifier -> issue
     for issue in issue_generator:
-
-        try:
-            issue_dict = dict(issue)
-        except ValueError:
-            if isinstance(issue, tuple) and issue[0] == 'SERVICE FAILED':
-                targets.remove(issue[1])
-                continue
-            else:
-                raise
+        if isinstance(issue, tuple) and issue[0] == 'SERVICE FAILED':
+            targets.remove(issue[1])
+            continue
 
         # De-duplicate issues coming in
         unique_identifier = make_unique_identifier(key_list, issue)
         if unique_identifier in issue_map:
             log.debug(
                 f"Merging tags and skipping. Seen {unique_identifier} of {issue}")
             # Merge and deduplicate tags.
-            issue_map[unique_identifier]['tags'] += issue_dict['tags']
+            issue_map[unique_identifier]['tags'] += issue['tags']
             issue_map[unique_identifier]['tags'] = list(set(issue_map[unique_identifier]['tags']))
         else:
-            issue_map[unique_identifier] = issue_dict
+            issue_map[unique_identifier] = issue
 
     seen_uuids = set()
-    for issue_dict in issue_map.values():
-
+    for issue in issue_map.values():
         # We received this issue from The Internet, but we're not sure what
         # kind of encoding the service providers may have handed us. Let's try
         # and decode all byte strings from UTF8 off the bat.  If we encounter
         # other encodings in the wild in the future, we can revise the handling
         # here. https://github.com/ralphbean/bugwarrior/issues/350
-        for key in issue_dict.keys():
-            if isinstance(issue_dict[key], bytes):
+        for key in issue.keys():
+            if isinstance(issue[key], bytes):
                 try:
-                    issue_dict[key] = issue_dict[key].decode('utf-8')
+                    issue[key] = issue[key].decode('utf-8')
                 except UnicodeDecodeError:
                     log.warn("Failed to interpret %r as utf-8" % key)
 
         # Blank priority should mean *no* priority
-        if issue_dict['priority'] == '':
-            issue_dict['priority'] = None
+        if issue['priority'] == '':
+            issue['priority'] = None
 
         try:
-            existing_taskwarrior_uuid = find_taskwarrior_uuid(tw, key_list, issue_dict)
+            existing_taskwarrior_uuid = find_taskwarrior_uuid(tw, key_list, issue)
         except MultipleMatches as e:
             log.exception("Multiple matches: %s", str(e))
         except NotFound:  # Create new task
-            issue_updates['new'].append(issue_dict)
+            issue_updates['new'].append(issue)
         else:  # Update existing task.
             seen_uuids.add(existing_taskwarrior_uuid)
             _, task = tw.get_task(uuid=existing_taskwarrior_uuid)
@@ -386,23 +371,23 @@ def synchronize(issue_generator, conf, main_section, dry_run=False):
             # Drop static fields from the upstream issue.  We don't want to
             # overwrite local changes to fields we declare static.
             for field in main_config.static_fields:
-                if field in issue_dict:
-                    del issue_dict[field]
+                if field in issue:
+                    del issue[field]
 
             # Merge annotations & tags from online into our task object
             if main_config.merge_annotations:
-                merge_left('annotations', task, issue_dict, hamming=True)
+                merge_left('annotations', task, issue, hamming=True)
 
             if main_config.merge_tags:
                 if main_config.replace_tags:
-                    replace_left('tags', task, issue_dict, main_config.static_tags)
+                    replace_left('tags', task, issue, main_config.static_tags)
                 else:
-                    merge_left('tags', task, issue_dict)
+                    merge_left('tags', task, issue)
 
-            issue_dict.pop('annotations', None)
-            issue_dict.pop('tags', None)
+            issue.pop('annotations', None)
+            issue.pop('tags', None)
 
-            task.update(issue_dict)
+            task.update(issue)
 
             if task.get_changes(keep=True):
                 issue_updates['changed'].append(task)

bugwarrior/services/__init__.py

@@ -84,7 +84,7 @@ def get_owner(self, issue):
 
     @abc.abstractmethod
     def issues(self):
-        """ Returns a list of dicts representing issues from a remote service.
+        """ Returns a list of Issue instances representing issues from a remote service.
 
         Each item in the list should be a dict that looks something like this:
 
@@ -196,7 +196,7 @@ def get_added_tags(self):
 
         return added_tags
 
-    def get_taskwarrior_record(self, refined=True):
+    def get_taskwarrior_record(self, refined=True) -> dict:
         if not getattr(self, '_taskwarrior_record', None):
             self._taskwarrior_record = self.to_taskwarrior()
         record = copy.deepcopy(self._taskwarrior_record)
@@ -290,49 +290,6 @@ def refine_record(self, record):
                 record[field] = getattr(self, 'get_default_%s' % field)()
         return record
 
-    def __iter__(self):
-        record = self.get_taskwarrior_record()
-        yield from record.keys()
-
-    def keys(self):
-        return list(self.__iter__())
-
-    def iterkeys(self):
-        return self.__iter__()
-
-    def items(self):
-        record = self.get_taskwarrior_record()
-        return list(record.items())
-
-    def iteritems(self):
-        record = self.get_taskwarrior_record()
-        yield from record.items()
-
-    def update(self, *args):
-        raise AttributeError(
-            "You cannot set attributes on issues."
-        )
-
-    def get(self, attribute, default=None):
-        try:
-            return self[attribute]
-        except KeyError:
-            return default
-
-    def __getitem__(self, attribute):
-        record = self.get_taskwarrior_record()
-        return record[attribute]
-
-    def __setitem__(self, attribute, value):
-        raise AttributeError(
-            "You cannot set attributes on issues."
-        )
-
-    def __delitem__(self, attribute):
-        raise AttributeError(
-            "You cannot delete attributes from issues."
-        )
-
     @property
     def record(self):
         return self._foreign_record

tests/test_command.py

@@ -17,18 +17,20 @@ def fake_github_issues(self):
 
 
 def fake_bz_issues(self):
-    yield from [{
-        'annotations': [],
-        'bugzillabugid': 1234567,
-        'bugzillastatus': 'NEW',
-        'bugzillasummary': 'This is the issue summary',
-        'bugzillaurl': 'https://http://one.com//show_bug.cgi?id=1234567',
-        'bugzillaproduct': 'Product',
-        'bugzillacomponent': 'Something',
-        'description': '(bw)Is#1234567 - This is the issue summary .. https://http://one.com//show_bug.cgi?id=1234567',  # noqa: E501
-        'priority': 'H',
-        'project': 'Something',
-        'tags': []}]
+    yield from [self.get_issue_for_record(
+        {
+            'id': 1234567,
+            'status': 'NEW',
+            'summary': 'This is the issue summary',
+            'product': 'Product',
+            'component': 'Something',
+            'description': '(bw)Is#1234567 - This is the issue summary .. https://http://one.com//show_bug.cgi?id=1234567',  # noqa: E501
+            'priority': 'H',
+            'project': 'Something',
+            'tags': []
+        }, {
+            'url': 'https://http://one.com//show_bug.cgi?id=1234567',
+        })]
 
 
 class TestPull(ConfigTest):

tests/test_db.py

@@ -121,7 +121,7 @@ def get_tasks(tw):
             # These should be de-duplicated in db.synchronize before
             # writing out to taskwarrior.
             # https://github.com/ralphbean/bugwarrior/issues/601
-            issue_generator = iter((issue, duplicate_issue,))
+            issue_generator = iter((copy.deepcopy(issue), duplicate_issue,))
             db.synchronize(issue_generator, bwconfig, 'general')
 
             self.assertEqual(get_tasks(tw), {
@@ -144,7 +144,7 @@ def get_tasks(tw):
         # Change static field
         issue['project'] = 'other_project'
 
-        db.synchronize(iter((issue,)), bwconfig, 'general')
+        db.synchronize(iter((copy.deepcopy(issue),)), bwconfig, 'general')
 
         self.assertEqual(get_tasks(tw), {
             'completed': [],
@@ -182,7 +182,7 @@ def get_tasks(tw):
             'pending': []})
 
         # TEST REOPENED ISSUE
-        db.synchronize(iter((issue,)), bwconfig, 'general')
+        db.synchronize(iter((copy.deepcopy(issue),)), bwconfig, 'general')
 
         tasks = tw.load_tasks()
         self.assertEqual(