Merge pull request #665 from octodns/fix-windows-filename-problems

Fix windows filename problems

.gitignore

@@ -5,8 +5,8 @@
 *.pyc
 .coverage
 .env
-/config/
 /build/
+/config/
 coverage.xml
 dist/
 env/
@@ -14,4 +14,5 @@ htmlcov/
 nosetests.xml
 octodns.egg-info/
 output/
+tests/zones/unit.tests.
 tmp/

octodns/provider/yaml.py

@@ -239,11 +239,13 @@ class SplitYamlProvider(YamlProvider):
     # instead of a file matching the record name.
     CATCHALL_RECORD_NAMES = ('*', '')
 
-    def __init__(self, id, directory, *args, **kwargs):
+    def __init__(self, id, directory, extension='.', *args, **kwargs):
         super(SplitYamlProvider, self).__init__(id, directory, *args, **kwargs)
+        self.extension = extension
 
     def _zone_directory(self, zone):
-        return join(self.directory, zone.name)
+        filename = '{}{}'.format(zone.name[:-1], self.extension)
+        return join(self.directory, filename)
 
     def populate(self, zone, target=False, lenient=False):
         self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name,

octodns/source/axfr.py

@@ -216,7 +216,7 @@ class ZoneFileSource(AxfrBaseSource):
         # (optional, default true)
         check_origin: false
     '''
-    def __init__(self, id, directory, file_extension=None, check_origin=True):
+    def __init__(self, id, directory, file_extension='.', check_origin=True):
         self.log = logging.getLogger('ZoneFileSource[{}]'.format(id))
         self.log.debug('__init__: id=%s, directory=%s, file_extension=%s, '
                        'check_origin=%s', id,
@@ -229,12 +229,7 @@ class ZoneFileSource(AxfrBaseSource):
         self._zone_records = {}
 
     def _load_zone_file(self, zone_name):
-
-        zone_filename = zone_name
-        if self.file_extension:
-            zone_filename = '{}{}'.format(zone_name,
-                                          self.file_extension.lstrip('.'))
-
+        zone_filename = '{}{}'.format(zone_name[:-1], self.file_extension)
         zonefiles = listdir(self.directory)
         if zone_filename in zonefiles:
             try:

tests/config/simple-split.yaml

@@ -4,14 +4,17 @@ providers:
   in:
     class: octodns.provider.yaml.SplitYamlProvider
     directory: tests/config/split
+    extension: .tst
   dump:
     class: octodns.provider.yaml.SplitYamlProvider
     directory: env/YAML_TMP_DIR
+    extension: .tst
   # This is sort of ugly, but it shouldn't hurt anything. It'll just write out
   # the target file twice where it and dump are both used
   dump2:
     class: octodns.provider.yaml.SplitYamlProvider
     directory: env/YAML_TMP_DIR
+    extension: .tst
   simple:
     class: helpers.SimpleProvider
   geo:

tests/test_octodns_provider_yaml.py

@@ -207,18 +207,20 @@ class TestSplitYamlProvider(TestCase):
 
     def test_zone_directory(self):
         source = SplitYamlProvider(
-            'test', join(dirname(__file__), 'config/split'))
+            'test', join(dirname(__file__), 'config/split'),
+            extension='.tst')
 
         zone = Zone('unit.tests.', [])
 
         self.assertEqual(
-            join(dirname(__file__), 'config/split/unit.tests.'),
+            join(dirname(__file__), 'config/split', 'unit.tests.tst'),
             source._zone_directory(zone))
 
     def test_apply_handles_existing_zone_directory(self):
         with TemporaryDirectory() as td:
-            provider = SplitYamlProvider('test', join(td.dirname, 'config'))
-            makedirs(join(td.dirname, 'config', 'does.exist.'))
+            provider = SplitYamlProvider('test', join(td.dirname, 'config'),
+                                         extension='.tst')
+            makedirs(join(td.dirname, 'config', 'does.exist.tst'))
 
             zone = Zone('does.exist.', [])
             self.assertTrue(isdir(provider._zone_directory(zone)))
@@ -227,7 +229,8 @@ class TestSplitYamlProvider(TestCase):
 
     def test_provider(self):
         source = SplitYamlProvider(
-            'test', join(dirname(__file__), 'config/split'))
+            'test', join(dirname(__file__), 'config/split'),
+            extension='.tst')
 
         zone = Zone('unit.tests.', [])
         dynamic_zone = Zone('dynamic.tests.', [])
@@ -246,9 +249,10 @@ class TestSplitYamlProvider(TestCase):
         with TemporaryDirectory() as td:
             # Add some subdirs to make sure that it can create them
             directory = join(td.dirname, 'sub', 'dir')
-            zone_dir = join(directory, 'unit.tests.')
-            dynamic_zone_dir = join(directory, 'dynamic.tests.')
-            target = SplitYamlProvider('test', directory)
+            zone_dir = join(directory, 'unit.tests.tst')
+            dynamic_zone_dir = join(directory, 'dynamic.tests.tst')
+            target = SplitYamlProvider('test', directory,
+                                       extension='.tst')
 
             # We add everything
             plan = target.plan(zone)
@@ -335,7 +339,8 @@ class TestSplitYamlProvider(TestCase):
 
     def test_empty(self):
         source = SplitYamlProvider(
-            'test', join(dirname(__file__), 'config/split'))
+            'test', join(dirname(__file__), 'config/split'),
+            extension='.tst')
 
         zone = Zone('empty.', [])
 
@@ -345,7 +350,8 @@ class TestSplitYamlProvider(TestCase):
 
     def test_unsorted(self):
         source = SplitYamlProvider(
-            'test', join(dirname(__file__), 'config/split'))
+            'test', join(dirname(__file__), 'config/split'),
+            extension='.tst')
 
         zone = Zone('unordered.', [])
 
@@ -356,14 +362,15 @@ class TestSplitYamlProvider(TestCase):
 
         source = SplitYamlProvider(
             'test', join(dirname(__file__), 'config/split'),
-            enforce_order=False)
+            extension='.tst', enforce_order=False)
         # no exception
         source.populate(zone)
         self.assertEqual(2, len(zone.records))
 
     def test_subzone_handling(self):
         source = SplitYamlProvider(
-            'test', join(dirname(__file__), 'config/split'))
+            'test', join(dirname(__file__), 'config/split'),
+            extension='.tst')
 
         # If we add `sub` as a sub-zone we'll reject `www.sub`
         zone = Zone('unit.tests.', ['sub'])

tests/test_octodns_source_axfr.py

@@ -9,6 +9,8 @@ import dns.zone
 from dns.exception import DNSException
 
 from mock import patch
+from os.path import exists
+from shutil import copyfile
 from six import text_type
 from unittest import TestCase
 
@@ -21,7 +23,7 @@ from octodns.record import ValidationError
 class TestAxfrSource(TestCase):
     source = AxfrSource('test', 'localhost')
 
-    forward_zonefile = dns.zone.from_file('./tests/zones/unit.tests.',
+    forward_zonefile = dns.zone.from_file('./tests/zones/unit.tests.tst',
                                           'unit.tests', relativize=False)
 
     @patch('dns.zone.from_xfr')
@@ -44,15 +46,35 @@ class TestAxfrSource(TestCase):
 
 
 class TestZoneFileSource(TestCase):
-    source = ZoneFileSource('test', './tests/zones')
+    source = ZoneFileSource('test', './tests/zones', file_extension='.tst')
 
     def test_zonefiles_with_extension(self):
-        source = ZoneFileSource('test', './tests/zones', 'extension')
+        source = ZoneFileSource('test', './tests/zones', '.extension')
         # Load zonefiles with a specified file extension
         valid = Zone('ext.unit.tests.', [])
         source.populate(valid)
         self.assertEquals(1, len(valid.records))
 
+    def test_zonefiles_without_extension(self):
+        # Windows doesn't let files end with a `.` so we add a .tst to them in
+        # the repo and then try and create the `.` version we need for the
+        # default case (no extension.)
+        copyfile('./tests/zones/unit.tests.tst', './tests/zones/unit.tests.')
+        # Unfortunately copyfile silently works and create the file without
+        # the `.` so we have to check to see if it did that
+        if exists('./tests/zones/unit.tests'):
+            # It did so we need to skip this test, that means windows won't
+            # have full code coverage, but skipping the test is going out of
+            # our way enough for a os-specific/oddball case.
+            self.skipTest('Unable to create unit.tests. (ending with .) so '
+                          'skipping default filename testing.')
+
+        source = ZoneFileSource('test', './tests/zones')
+        # Load zonefiles without a specified file extension
+        valid = Zone('unit.tests.', [])
+        source.populate(valid)
+        self.assertEquals(12, len(valid.records))
+
     def test_populate(self):
         # Valid zone file in directory
         valid = Zone('unit.tests.', [])