Improved diff and properties

svn/common.py

@@ -157,8 +157,13 @@ def properties(self, rel_path=None):
         # query the proper list of this path
         root = xml.etree.ElementTree.fromstring(result)
         target_elem = root.find('target')
-        property_names = [p.attrib["name"]
-                          for p in target_elem.findall('property')]
+        if target_elem is not None:
+            property_names = [p.attrib["name"]
+                              for p in target_elem.findall('property')]
+        # In the event that no properties are associated, an empty properties
+        # element is returned as the result. 
+        else:
+            property_names = []
 
         # now query the content of each propery
         property_dict = {}
@@ -449,13 +454,36 @@ def diff(self, old, new, rel_path=None):
              '--new', '{0}@{1}'.format(full_url_or_path, new)],
             do_combine=True)
         file_to_diff = {}
+
+        # A diff has this form, potentially repeating for multiple files.
+        #
+        #  Index: relative/filename.txt
+        # ===================================================================
+        # The diff content
+        #
+        # Here we split diffs into files by the index section, pick up the 
+        # file name, then split again to pick up the content. 
         for non_empty_diff in filter(None, diff_result.decode('utf8').split('Index: ')):
-            split_diff = non_empty_diff.split('==')
-            file_to_diff[split_diff[0].strip().strip('/')] = split_diff[-1].strip('=').strip()
+            split_diff = non_empty_diff.split('='*67)
+            index_filename = split_diff[0].strip().strip('/')
+            file_to_diff[index_filename] = split_diff[-1].strip()
         diff_summaries = self.diff_summary(old, new, rel_path)
+
+        # Allocate summary info to the text for each change. 
+        # Not all diffs necessarily affect file text (directory changes, for example).
         for diff_summary in diff_summaries:
-            diff_summary['diff'] = \
-                file_to_diff[diff_summary['path'].split(full_url_or_path)[-1].strip('/')]
+            diff_summary['diff'] = ''
+            if 'path' in diff_summary:
+                # Summary file paths are absolute, while diff file indexes are relative.
+                # We try to match them up, first checking the root of the diff.
+                summary_index = diff_summary['path'][len(full_url_or_path):].strip('/')
+                # If the diff was conducted directly on the file, not a directory, the 
+                # above will fail to find the relative file name, so we look for that directly.
+                if summary_index == '':
+                    summary_index = os.path.basename(full_url_or_path)
+                # If we can match a diff to a summary, we attach it. 
+                if summary_index in file_to_diff:
+                    diff_summary['diff'] = file_to_diff[summary_index]
         return diff_summaries
 
     @property

tests/test_admin.py

@@ -32,7 +32,12 @@ def test_create_repository(self):
             a.create(temp_path)
 
             # Do a read.
-            rc = svn.remote.RemoteClient('file://' + temp_path)
+            # Duck-type identification of path type (Windows, Linux, etc.) to ensure                          
+            # file uri canonicity is enforced. URI must be file:///.
+            if temp_path[0] == '/':
+                rc = svn.remote.RemoteClient('file://' + temp_path)
+            else:
+                rc = svn.remote.RemoteClient('file:///' + temp_path)
             info = rc.info()
             _LOGGER.debug("Info from new repository: [%s]", str(info))
         finally:

tests/test_common.py

@@ -54,7 +54,12 @@ def __setup_test_environment(self):
         self.__temp_co_path = self.__get_temp_path_to_use()
         print("CO_PATH: {}".format(self.__temp_co_path))
 
-        r = svn.remote.RemoteClient('file://' + self.__temp_repo_path)
+        # Duck-type identification of path type (Windows, Linux, etc.) to ensure 
+        # file uri canonicity is enforced. URI must be file:///.
+        if self.__temp_repo_path[0] == '/':
+            r = svn.remote.RemoteClient('file://' + self.__temp_repo_path)
+        else: 
+            r = svn.remote.RemoteClient('file:///' + self.__temp_repo_path)
         r.checkout(self.__temp_co_path)
 
         # Create a client for it.
@@ -222,6 +227,18 @@ def test_diff(self):
                         'http://svn.apache.org/repos/asf/sling/trunk/pom.xml' \
                             in individual_diff[diff_key])
 
+    def test_diff_separator(self):
+        """
+        Checking diff
+        :return:
+        """
+        cc = self.__get_cc()
+        change = '502054'
+        file_cluster_event = 'activemq/activecluster/trunk/src/main/java/org/apache/activecluster/ClusterEvent.java'
+        actual_answer = cc.diff('0', change, file_cluster_event)[0]
+        self.assertTrue('static final int UPDATE_NODE = 2' in actual_answer['diff'])
+        self.assertTrue('else if (type == FAILED_NODE)' in actual_answer['diff'])
+
     def test_list(self):
         """
         Checking list
@@ -311,5 +328,16 @@ def test_force_export(self):
         except svn.exception.SvnException:
             self.fail("SvnException raised with force export")
 
+    def test_properties(self):
+        """
+        Checking we can retrieve properties
+        :return:
+        """
+        cc = self.__get_cc()
+        props = cc.properties('activemq/activecluster/trunk/src/main/java/org/apache/activecluster@1808406')
+        self.assertFalse(bool(props))
+        props = cc.properties('activemq/activecluster/trunk/src/main/java/org/apache/activecluster/Cluster.java@1808406')
+        self.assertEqual(props['svn:eol-style'], 'native')
+
     def test_cleanup(self):
         self.__temp_lc.cleanup()