From 36575071a10ad312b9af83ef462cfb0b7846b1b9 Mon Sep 17 00:00:00 2001 From: Michal Fluder Date: Fri, 3 Nov 2023 19:33:05 +0100 Subject: [PATCH 1/3] Use same algorithm for includes in configurationStore as in IncludeObserver.update method. --- src/z3c/insist/insist.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/z3c/insist/insist.py b/src/z3c/insist/insist.py index afe82aa..d704f84 100644 --- a/src/z3c/insist/insist.py +++ b/src/z3c/insist/insist.py @@ -15,6 +15,7 @@ import json import logging import os +import pathlib import re import zope.component import zope.schema @@ -358,10 +359,11 @@ def getConfigPath(self): def getConfigFilename(self): return self.section + '.ini' - def getIncludes(self, cfgstr): - basePath = self.getConfigPath() + def getIncludes(self, cfgstr, configPath): + configPath = pathlib.Path(configPath) + # use string representation for backward compatibility return [ - os.path.normpath(os.path.join(basePath, include)) + str(pathlib.Path(configPath.parent, include).resolve()) for include in re.findall(RE_INCLUDES, cfgstr, re.MULTILINE) ] @@ -394,7 +396,7 @@ def dump(self, config=None): def _readSubConfig(self, configPath): with self.openFile(configPath, 'r') as fle: cfgstr = fle.read() - includes = self.getIncludes(cfgstr) + includes = self.getIncludes(cfgstr, configPath) for include in includes: if not self.fileExists(include): raise ValueError( From 7b8f5552fc21678a9439cb3f0bda637da4f6182d Mon Sep 17 00:00:00 2001 From: Michal Fluder Date: Tue, 7 Nov 2023 15:24:13 +0100 Subject: [PATCH 2/3] Add tests to test if bugs were actualy fixed --- src/z3c/insist/insist.py | 2 +- src/z3c/insist/tests/test_enforce.py | 151 +++++++++++++++------------ src/z3c/insist/tests/test_insist.py | 51 +++++---- 3 files changed, 118 insertions(+), 86 deletions(-) diff --git a/src/z3c/insist/insist.py b/src/z3c/insist/insist.py index d704f84..3994ee1 100644 --- a/src/z3c/insist/insist.py +++ b/src/z3c/insist/insist.py @@ -361,7 +361,7 @@ def getConfigFilename(self): def getIncludes(self, cfgstr, configPath): configPath = pathlib.Path(configPath) - # use string representation for backward compatibility + # use string representation for compatibility with rest of the code return [ str(pathlib.Path(configPath.parent, include).resolve()) for include in re.findall(RE_INCLUDES, cfgstr, re.MULTILINE) diff --git a/src/z3c/insist/tests/test_enforce.py b/src/z3c/insist/tests/test_enforce.py index 84b9c46..39b1554 100644 --- a/src/z3c/insist/tests/test_enforce.py +++ b/src/z3c/insist/tests/test_enforce.py @@ -193,29 +193,40 @@ def test_locking(self): def test_included(self): - baseDir = tempfile.mkdtemp('-base') - - basePath = os.path.join(baseDir, 'simple-collection.ini') - with open(basePath, 'w') as file: - file.write( - '[simple:one]\n' - 'text = One\n' - '\n' - '[simple:two]\n' - 'text = Two\n' + baseDir = pathlib.Path(tempfile.mkdtemp('-base')) + basePath = baseDir.joinpath('simple-collection.ini') + basePath.write_text( + '[simple:one]\n' + 'text = One\n' + '\n' + '[simple:two]\n' + 'text = Two\n' ) - confDir = tempfile.mkdtemp('-conf') - - simplePath = os.path.join(confDir, 'simple-collection.ini') - with open(simplePath, 'w') as file: - file.write( - f'#include {basePath}\n' - '[simple:two]\n' - 'text = 2\n' - '\n' - '[simple:three]\n' - 'text = 3\n' + nestedConfDir = pathlib.Path(tempfile.mkdtemp(suffix='-nestedConf', dir=baseDir)) + # baseDir/tmpdir-nestedConf/simple-collection.ini + nestedPath = nestedConfDir.joinpath('simple-collection.ini') + nestedPath.write_text( + '[simple:five]\n' + 'text = 5\n' + '\n' + '[simple:six]\n' + 'text = 6\n' + ) + + baseConfDir = pathlib.Path(tempfile.mkdtemp('-baseConf', dir=baseDir)) + confDir = pathlib.Path(tempfile.mkdtemp('-conf', dir=baseConfDir)) + simplePath = confDir.joinpath('simple-collection.ini') + simplePath.write_text( + f'#include {basePath}\n' + # ../../tmpdir-nestedConf/simple-collection.ini + # baseDir/ + f'#include ../../{nestedPath.relative_to(baseDir)}\n' + '[simple:two]\n' + 'text = 2\n' + '\n' + '[simple:three]\n' + 'text = 3\n' ) coll = {} @@ -233,7 +244,10 @@ class SimpleCollectionStore( item_factory = Simple def getConfigPath(self): - return confDir + return baseConfDir + + def getConfigFilename(self): + return simplePath.relative_to(baseConfDir) @classmethod def fromRootAndFilename(cls, root, filename=None): @@ -248,55 +262,64 @@ class SimpleStore(insist.ConfigurationStore): schema = ISimple def getConfigPath(self): - return confDir + return baseConfDir zope.component.provideAdapter(SimpleStore) - enf = enforce.Enforcer(confDir, coll) + enf = enforce.Enforcer(baseConfDir, coll) enf.registerHandlers() enf.start() - inc = enforce.IncludeObserver(confDir) + inc = enforce.IncludeObserver(baseConfDir) inc.initialize() inc.start() # Load the configuration when the main config file is modified. - pathlib.Path(simplePath).touch() + simplePath.touch() time.sleep(0.01) - self.assertEqual(len(coll), 3) + self.assertEqual(len(coll), 5) self.assertEqual(coll['one'].text, 'One') self.assertEqual(coll['two'].text, '2') self.assertEqual(coll['three'].text, '3') + self.assertEqual(coll['five'].text, '5') + self.assertEqual(coll['six'].text, '6') # Configuration should also be loaded when the base config file is # updated. - with open(basePath, 'w') as file: - file.write( - '[simple:one]\n' - 'text = 1\n' - ) + basePath.write_text( + '[simple:one]\n' + 'text = 1\n' + ) time.sleep(0.01) - self.assertEqual(len(coll), 3) + self.assertEqual(len(coll), 5) self.assertEqual(coll['one'].text, '1') - # Add a new base file. - baseDir2 = tempfile.mkdtemp('-base2') - basePath2 = os.path.join(baseDir2, 'simple-collection.ini') - with open(basePath2, 'w') as file: - file.write( - '[simple:four]\n' - 'text = Four\n' - ) - with open(simplePath, 'w') as file: - file.write( - f'#include {basePath}\n' - f'#include {basePath2}\n' - '[simple:two]\n' - 'text = 2\n' - '\n' - '[simple:three]\n' - 'text = 3\n' - ) + # Configuration should also be loaded when base config included using + # relative path is updated. + nestedPath.write_text( + '[simple:five]\n' + 'text = five\n' + ) + time.sleep(0.01) + self.assertEqual(len(coll), 4) + self.assertEqual(coll['five'].text, 'five') + + # Add a new base file and remove relative one. + baseDir2 = pathlib.Path(tempfile.mkdtemp('-base2')) + basePath2 = baseDir2.joinpath('simple-collection.ini') + basePath2.write_text( + '[simple:four]\n' + 'text = Four\n' + ) + simplePath.write_text( + f'#include {basePath}\n' + f'#include {basePath2}\n' + '[simple:two]\n' + 'text = 2\n' + '\n' + '[simple:three]\n' + 'text = 3\n' + ) time.sleep(0.01) self.assertEqual(len(coll), 4) @@ -306,24 +329,22 @@ def getConfigPath(self): self.assertEqual(coll['four'].text, 'Four') # Modify the newly added base. - with open(basePath2, 'w') as file: - file.write( - '[simple:four]\n' - 'text = 4\n' - ) + basePath2.write_text( + '[simple:four]\n' + 'text = 4\n' + ) time.sleep(0.01) self.assertEqual(coll['four'].text, '4') # Remove second base. - with open(simplePath, 'w') as file: - file.write( - f'#include {basePath}\n' - '[simple:two]\n' - 'text = 2\n' - '\n' - '[simple:three]\n' - 'text = 3\n' - ) + simplePath.write_text( + f'#include {basePath}\n' + '[simple:two]\n' + 'text = 2\n' + '\n' + '[simple:three]\n' + 'text = 3\n' + ) os.remove(basePath2) time.sleep(0.01) diff --git a/src/z3c/insist/tests/test_insist.py b/src/z3c/insist/tests/test_insist.py index 67b327d..40e9ea3 100644 --- a/src/z3c/insist/tests/test_insist.py +++ b/src/z3c/insist/tests/test_insist.py @@ -11,6 +11,7 @@ import datetime import doctest import os +import pathlib import pprint import tempfile import textwrap @@ -646,25 +647,31 @@ def getConfigPath(self): self.assertEqual(orig_hash, new_hash) def test_load_withIncludes(self): - dir = tempfile.mkdtemp() - - with open(os.path.join(dir, 'base.ini'), 'w') as file: - file.write( - '[simple:one]\n' - 'text = One\n' - '\n' - '[simple:two]\n' - 'text = Two\n' + baseDir = tempfile.mkdtemp() + + nestedConfDir = pathlib.Path(tempfile.mkdtemp(suffix='-nestedConf', dir=baseDir)) + # baseDir/tmpdir-nestedConf/simple-collection.ini + nestedPath = nestedConfDir.joinpath('simple-collection.ini') + nestedPath.write_text( + '[simple:one]\n' + 'text = One\n' + '\n' + '[simple:four]\n' + 'text = Four\n' ) - with open(os.path.join(dir, 'simple-collection.ini'), 'w') as file: - file.write( - '#include base.ini\n' - '[simple:two]\n' - 'text = 2\n' - '\n' - '[simple:three]\n' - 'text = 3\n' + baseConfDir = pathlib.Path(tempfile.mkdtemp('-baseConf', dir=baseDir)) + confDir = pathlib.Path(tempfile.mkdtemp('-conf', dir=baseConfDir)) + simplePath = confDir.joinpath('simple-collection.ini') + simplePath.write_text( + # ../../tmpdir-nestedConf/simple-collection.ini + # baseDir/ + f'#include ../../{nestedPath.relative_to(baseDir)}\n' + '[simple:two]\n' + 'text = 2\n' + '\n' + '[simple:three]\n' + 'text = 3\n' ) class SimpleCollectionStore( @@ -676,7 +683,10 @@ class SimpleCollectionStore( item_factory = Simple def getConfigPath(self): - return dir + return baseConfDir + + def getConfigFilename(self): + return simplePath.relative_to(baseConfDir) @zope.component.adapter(ISimple) @zope.interface.implementer(interfaces.IConfigurationStore) @@ -685,17 +695,18 @@ class SimpleStore(insist.ConfigurationStore): schema = ISimple def getConfigPath(self): - return dir + return baseConfDir reg = zope.component.provideAdapter(SimpleStore) coll = {} store = SimpleCollectionStore(coll) store.loads('') - self.assertEqual(len(coll), 3) + self.assertEqual(len(coll), 4) self.assertEqual(coll['one'].text, 'One') self.assertEqual(coll['two'].text, '2') self.assertEqual(coll['three'].text, '3') + self.assertEqual(coll['four'].text, 'Four') def test_getSectionFromPath(self): dir = tempfile.mkdtemp() From 10daf7e366150f0fd05a34b0ca75c0232165252c Mon Sep 17 00:00:00 2001 From: Michal Fluder Date: Tue, 7 Nov 2023 15:24:36 +0100 Subject: [PATCH 3/3] update changelog --- CHANGES.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 4a0efc2..b3df93e 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -4,7 +4,8 @@ Changelog 1.5.3 (unreleased) ------------------ -- Nothing changed yet. +- Fix inconsistencies in calculating includes paths. Please note that signature of + SeparateFileConfigurationStoreMixIn.getIncludes has changed. 1.5.2 (2023-05-05)