Edgewall Software

Ticket #7378: config-refact.patch

File config-refact.patch, 23.5 KB (added by cboos, 15 months ago)

config: get rid of ConfigParser (patch applies on r10593)

  • trac/config.py 

    # HG changeset patch
# Parent 49cb2d5c973c6b04e2146b94beeaded7e7ca53b0
config: get rid of !ConfigParser.

Instead, we use our own `Configuration._read()` which preserves the order of sections and of the entries within those sections, as found in the .ini file. The comments are also preserved, so that writing back a modified Configuration simply updates the entries in place.

The other behavior changes are:
 - full unicode support
 - case sensitive sections and keys
 - no `%(...)s` expansion

Closes #7378.

diff -r 49cb2d5c973c trac/config.py
    a b  
    1414 
    1515from __future__ import with_statement 
    1616 
    17 from ConfigParser import SafeConfigParser 
     17from StringIO import StringIO 
    1818from copy import deepcopy 
    1919import os.path 
    2020 
    … … _TRUE_VALUES = ('yes', 'true', 'enabled' 
    3434 
    3535_use_default = object() 
    3636 
    37 def _to_utf8(basestr): 
    38     return to_unicode(basestr).encode('utf-8') 
    39  
    40  
    4137class ConfigurationError(TracError): 
    4238    """Exception raised when a value in the configuration file is not valid.""" 
    4339    title = N_('Configuration Error') 
    4440 
    4541 
    4642class Configuration(object): 
    47     """Thin layer over `ConfigParser` from the Python standard library. 
     43    """Ini file reader and writer. 
    4844 
    49     In addition to providing some convenience methods, the class remembers 
    50     the last modification time of the configuration file, and reparses it 
    51     when the file has changed. 
     45    :param filename: location of the .ini file 
     46    :param params: global defaults that can be used in `%(name)s` 
     47                   style parameter substitution 
     48    In addition to providing some convenience methods, the class 
     49    remembers the last modification time of the configuration file, 
     50    and reparses it when the file has changed. 
    5251    """ 
    5352    def __init__(self, filename, params={}): 
    5453        self.filename = filename 
    55         self.parser = SafeConfigParser(params) 
     54        self.params = params 
     55        self.parsed_sections = [] 
     56        # parsed_sections stores sections and entries as read: 
     57        #   [(section, [(key, value), "#...", ...]), ...] 
     58        self.parsed_options = {} 
     59        # parsed_options stores actual values: 
     60        #   section -> {key: value} 
    5661        self._old_sections = {} 
    5762        self.parents = [] 
    5863        self._lastmtime = 0 
    59         self._sections = {} 
     64        self._sections = {}  
     65        # _sections stores the Section instances 
    6066        self.parse_if_needed(force=True) 
    6167 
     68    def has_section(self, section): 
     69        """Return `True` if `section` was defined in the .ini file.""" 
     70        return section in self.parsed_options 
     71 
    6272    def __contains__(self, name): 
    6373        """Return whether the configuration contains a section of the given 
    6474        name. 
    6575        """ 
    66         return name in self.sections() 
     76        return name and name in self.sections() 
    6777 
    6878    def __getitem__(self, name): 
    6979        """Return the configuration section with the specified name.""" 
    … … class Configuration(object): 
    183193        options declared in components that are enabled in the given 
    184194        `ComponentManager` are returned. 
    185195        """ 
    186         sections = set([to_unicode(s) for s in self.parser.sections()]) 
     196        sections = set(self.parsed_options.keys()) 
    187197        for parent in self.parents: 
    188198            sections.update(parent.sections(compmgr, defaults=False)) 
    189199        if defaults: 
    … … class Configuration(object): 
    197207         
    198208        (since Trac 0.11) 
    199209        """ 
    200         section_str = _to_utf8(section) 
    201         if self.parser.has_section(section_str): 
    202             if _to_utf8(option) in self.parser.options(section_str): 
    203                 return True 
     210        if option in self.parsed_options.get(section, ()): 
     211            return True 
    204212        for parent in self.parents: 
    205213            if parent.has_option(section, option, defaults=False): 
    206214                return True 
    … … class Configuration(object): 
    211219        if not self.filename: 
    212220            return 
    213221 
    214         # Only save options that differ from the defaults 
     222        # Only save options that differ from the defaults, return 
     223        # either a (key, value) pair if value differs from default,  
     224        # or a "# key = value (where it comes from)" comment 
     225        def make_entry(section, key, options): 
     226            default = _use_default 
     227            info = None 
     228            for parent in self.parents: 
     229                if parent.has_option(section, key, defaults=False): 
     230                    default = parent.get(section, key) 
     231                    info = 'inherited from ' + parent.filename 
     232                    break 
     233            if default is _use_default: 
     234                option = Option.registry.get((section, key)) 
     235                if option: 
     236                    default = option.default 
     237                    info = 'default' 
     238            if key in options: 
     239                current = options.pop(key) 
     240                if current != default: 
     241                    return key, current 
     242                return '# %s = %s ; (%s)' % (key, default, info) 
     243        # Collect only actual entries 
     244        def non_default_entries(section, options): 
     245            entries = [] 
     246            for key in sorted(options): 
     247                entry = make_entry(section, key, options) 
     248                if isinstance(entry, tuple): 
     249                    entries.append(entry) 
     250            return entries 
     251 
     252        parsed_options = deepcopy(self.parsed_options) 
    215253        sections = [] 
    216         for section in self.sections(): 
    217             section_str = _to_utf8(section) 
    218             options = [] 
    219             for option in self[section]: 
    220                 default_str = None 
    221                 for parent in self.parents: 
    222                     if parent.has_option(section, option, defaults=False): 
    223                         default_str = _to_utf8(parent.get(section, option)) 
    224                         break 
    225                 option_str = _to_utf8(option) 
    226                 current_str = False 
    227                 if self.parser.has_option(section_str, option_str): 
    228                     current_str = self.parser.get(section_str, option_str) 
    229                 if current_str is not False and current_str != default_str: 
    230                     options.append((option_str, current_str)) 
    231             if options: 
    232                 sections.append((section_str, sorted(options))) 
     254        for section, section_content in self.parsed_sections: 
     255            filtered_entries = [] 
     256            options = parsed_options.get(section, ()) 
     257            for entry in section_content: 
     258                if isinstance(entry, tuple): 
     259                    entry = make_entry(section, entry[0], options) 
     260                    if not entry: 
     261                        continue 
     262                # else try to see if this is a previously "commented out value" 
     263                # that needs to be updated in place (comment:12) 
     264                filtered_entries.append(entry) 
     265            sections.append((section, filtered_entries)) 
     266        if any(len(options) for options in parsed_options): 
     267            # add new entries to existing sections 
     268            for section, section_content in sections: 
     269                options = parsed_options.pop(section, ()) 
     270                new_entries = non_default_entries(section, options) 
     271                if new_entries: 
     272                    section_content.extend(new_entries + ['']) 
     273            # add remaining sections as new sections 
     274            if parsed_options: 
     275                for section in sorted(parsed_options.keys()): 
     276                    options = parsed_options[section] 
     277                    entries = non_default_entries(section, options) 
     278                    if entries: 
     279                        sections.append((section, entries + [''])) 
    233280 
    234         # At this point, all the strings in `sections` are UTF-8 encoded `str` 
    235281        try: 
     282            buf = StringIO() 
     283 
     284            encoding = '# -*- coding: utf-8 -*-' 
     285            if self.parsed_sections: 
     286                section, comments = self.parsed_sections[0] 
     287                if section is None and comments and comments[0] == encoding: 
     288                    encoding = None 
     289            if encoding: 
     290                buf.write(encoding + '\n\n') 
     291 
     292            for section, section_content in sections: 
     293                if section: 
     294                    buf.write('[%s]\n' % section) 
     295                for entry in section_content: 
     296                    if isinstance(entry, tuple): 
     297                        key, val = entry 
     298                        if key in self[section].overridden: 
     299                            buf.write('# %s = <inherited>\n' % key) 
     300                        else: 
     301                            val = val.replace(CRLF, '\n').replace('\n', '\n ') 
     302                            buf.write('%s = %s\n' % (key, val)) 
     303                    else: 
     304                        buf.write(entry + '\n') 
     305            utf8_content = buf.getvalue().encode('utf-8') 
    236306            with AtomicFile(self.filename, 'w') as fileobj: 
    237                 fileobj.write('# -*- coding: utf-8 -*-\n\n') 
    238                 for section, options in sections: 
    239                     fileobj.write('[%s]\n' % section) 
    240                     for key_str, val_str in options: 
    241                         if to_unicode(key_str) in self[section].overridden: 
    242                             fileobj.write('# %s = <inherited>\n' % key_str) 
    243                         else: 
    244                             val_str = val_str.replace(CRLF, '\n') \ 
    245                                              .replace('\n', '\n ') 
    246                             fileobj.write('%s = %s\n' % (key_str, val_str)) 
    247                     fileobj.write('\n') 
    248             self._old_sections = deepcopy(self.parser._sections) 
    249         except Exception: 
     307                fileobj.write(utf8_content) 
     308            ## self._old_sections = deepcopy(self.parser._sections) 
     309        except Exception, e: 
    250310            # Revert all changes to avoid inconsistencies 
    251             self.parser._sections = deepcopy(self._old_sections) 
     311            ## FIXME self.parser._sections = deepcopy(self._old_sections) 
    252312            raise 
    253313 
    254314    def parse_if_needed(self, force=False): 
    … … class Configuration(object): 
    259319        modtime = os.path.getmtime(self.filename) 
    260320        if force or modtime > self._lastmtime: 
    261321            self._sections = {} 
    262             self.parser._sections = {} 
    263             self.parser.read(self.filename) 
     322            ## self.parser._sections = {}  ### FIXME 
     323            self._read() 
    264324            self._lastmtime = modtime 
    265             self._old_sections = deepcopy(self.parser._sections) 
     325            ## self._old_sections = deepcopy(self.parser._sections) 
    266326            changed = True 
    267327         
    268328        if changed: 
    269329            self.parents = [] 
    270             if self.parser.has_option('inherit', 'file'): 
    271                 for filename in self.parser.get('inherit', 'file').split(','): 
    272                     filename = to_unicode(filename.strip()) 
     330            inherited = self.parsed_options.get('inherit', {}).get('file') 
     331            if inherited: 
     332                for filename in (f.strip() for f in inherited.split(',')): 
    273333                    if not os.path.isabs(filename): 
    274334                        filename = os.path.join(os.path.dirname(self.filename), 
    275335                                                filename) 
    … … class Configuration(object): 
    282342            self._cache = {} 
    283343        return changed 
    284344 
     345    def _read(self): 
     346        content = to_unicode(file(self.filename, 'rb').read()) 
     347        self.parsed_sections = [] 
     348        section, section_content = None, [] 
     349        for line in content.splitlines(): 
     350            lsline = line.lstrip() 
     351            sline = lsline.rstrip() 
     352            if not sline or sline[0] in '#;': 
     353                section_content.append(sline) 
     354            elif lsline != line: # multi-line value? 
     355                if section_content and isinstance(section_content[-1], tuple): 
     356                    key, value = section_content[-1] 
     357                    section_content[-1] = (key, value + '\n' + sline) 
     358            elif section and '=' in sline: # setting 
     359                key, value = sline.split('=', 1) 
     360                # TODO: split value at rightmost ';', then store (k,v,comment) 
     361                section_content.append((key.strip(), value.strip())) 
     362            elif sline[0] == '[' and sline[-1] == ']': 
     363                self.parsed_sections.append((section, section_content)) 
     364                section, section_content = sline[1:-1], [] 
     365        if section: 
     366            self.parsed_sections.append((section, section_content)) 
     367        self.parsed_options = {} 
     368        for section, section_content in self.parsed_sections: 
     369            if section: 
     370                options = self.parsed_options.setdefault(section, {}) 
     371                for key, value in (entry for entry in section_content 
     372                                   if isinstance(entry, tuple)): 
     373                    options[key] = value 
     374 
    285375    def touch(self): 
    286376        if self.filename and os.path.isfile(self.filename) \ 
    287377           and os.access(self.filename, os.W_OK): 
    … … class Configuration(object): 
    295385        """ 
    296386        for section, default_options in self.defaults(compmgr).items(): 
    297387            for name, value in default_options.items(): 
    298                 if not self.parser.has_option(_to_utf8(section), 
    299                                               _to_utf8(name)): 
     388                if not self.has_option(section, name): 
    300389                    if any(parent[section].contains(name, defaults=False) 
    301390                           for parent in self.parents): 
    302391                        value = None 
    … … class Section(object): 
    308397     
    309398    Objects of this class should not be instantiated directly. 
    310399    """ 
    311     __slots__ = ['config', 'name', 'overridden', '_cache'] 
     400    __slots__ = ('config', 'name', 'overridden', '_cache') 
    312401 
    313402    def __init__(self, config, name): 
    314403        self.config = config 
    … … class Section(object): 
    317406        self._cache = {} 
    318407 
    319408    def contains(self, key, defaults=True): 
    320         if self.config.parser.has_option(_to_utf8(self.name), _to_utf8(key)): 
     409        if self.config.has_option(self.name, key): 
    321410            return True 
    322411        for parent in self.config.parents: 
    323412            if parent[self.name].contains(key, defaults=False): 
    324413                return True 
    325         return defaults and Option.registry.has_key((self.name, key)) 
     414        return defaults and (self.name, key) in Option.registry 
    326415     
    327416    __contains__ = contains 
    328417 
    … … class Section(object): 
    333422        components that are enabled in the given `ComponentManager`. 
    334423        """ 
    335424        options = set() 
    336         name_str = _to_utf8(self.name) 
    337         if self.config.parser.has_section(name_str): 
    338             for option_str in self.config.parser.options(name_str): 
    339                 option = to_unicode(option_str) 
     425        poptions = self.config.parsed_options.get(self.name) 
     426        if poptions: 
     427            for option in poptions.keys(): 
    340428                options.add(option.lower()) 
    341429                yield option 
    342430        for parent in self.config.parents: 
    … … class Section(object): 
    363451        cached = self._cache.get(key, _use_default) 
    364452        if cached is not _use_default: 
    365453            return cached 
    366         name_str = _to_utf8(self.name) 
    367         key_str = _to_utf8(key) 
    368         if self.config.parser.has_option(name_str, key_str): 
    369             value = self.config.parser.get(name_str, key_str) 
     454        options = self.config.parsed_options.get(self.name, ()) 
     455        if key in options: 
     456            value = options[key] 
    370457        else: 
    371458            for parent in self.config.parents: 
    372459                value = parent[self.name].get(key, _use_default) 
    … … class Section(object): 
    382469            return default 
    383470        if not value: 
    384471            value = u'' 
    385         elif isinstance(value, basestring): 
    386             value = to_unicode(value) 
    387472        self._cache[key] = value 
    388473        return value 
    389474 
    … … class Section(object): 
    480565 
    481566    def set(self, key, value): 
    482567        """Change a configuration value. 
    483          
     568 
     569        :param key: setting key 
     570        :param value: setting value; if `None`, the value for this setting 
     571                      will be the inherited one 
     572        :return: the value, or `''` if `None` was given. 
     573 
    484574        These changes are not persistent unless saved with `save()`. 
    485575        """ 
    486576        self._cache.pop(key, None) 
    487         name_str = _to_utf8(self.name) 
    488         key_str = _to_utf8(key) 
    489         if not self.config.parser.has_section(name_str): 
    490             self.config.parser.add_section(name_str) 
    491577        if value is None: 
    492578            self.overridden[key] = True 
    493             value_str = '' 
    494         else: 
    495             value_str = _to_utf8(value) 
    496         return self.config.parser.set(name_str, key_str, value_str) 
     579            value = '' 
     580        self.config.parsed_options.setdefault(self.name, {})[key] = value 
     581        return value 
    497582 
    498583    def remove(self, key): 
    499584        """Delete a key from this section. 
    500585 
    501586        Like for `set()`, the changes won't persist until `save()` gets called. 
    502587        """ 
    503         name_str = _to_utf8(self.name) 
    504         if self.config.parser.has_section(name_str): 
     588        options = self.config.parsed_options.get(self.name) 
     589        if options: 
    505590            self._cache.pop(key, None) 
    506             self.config.parser.remove_option(_to_utf8(self.name), _to_utf8(key)) 
     591            options.pop(key, None) 
    507592 
    508593 
    509594def _get_registry(cls, compmgr=None): 
    … … class ChoiceOption(Option): 
    645730    """ 
    646731     
    647732    def __init__(self, section, name, choices, doc=''): 
    648         Option.__init__(self, section, name, _to_utf8(choices[0]), doc) 
    649         self.choices = set(_to_utf8(choice).strip() for choice in choices) 
     733        Option.__init__(self, section, name, to_unicode(choices[0]), doc) 
     734        self.choices = set(to_unicode(choice).strip() for choice in choices) 
    650735 
    651736    def accessor(self, section, name, default): 
    652737        value = section.get(name, default) 

  • trac/tests/config.py 

    diff -r 49cb2d5c973c trac/tests/config.py
    a b class ConfigurationTestCase(unittest.Tes 
    3535 
    3636    def tearDown(self): 
    3737        Option.registry = self._orig_registry 
    38         os.remove(self.filename) 
     38        if os.path.isfile(self.filename): 
     39            os.remove(self.filename) 
    3940 
    4041    def _read(self): 
    4142        return Configuration(self.filename) 
    … … class ConfigurationTestCase(unittest.Tes 
    218219    def test_set_and_save(self): 
    219220        config = self._read() 
    220221        config.set('b', u'öption0', 'y') 
    221         config.set(u'aä', 'öption0', 'x') 
    222         config.set('aä', 'option2', "Voilà l'été")  # UTF-8 
     222        config.set(u'aä', u'öption0', 'x') 
     223        # config.set('aä', 'option2', "Voilà l'été")  # UTF-8 no longer legit 
    223224        config.set(u'aä', 'option1', u"Voilà l'été") # unicode 
    224225        # Note: the following would depend on the locale.getpreferredencoding() 
    225226        # config.set('a', 'option3', "Voil\xe0 l'\xe9t\xe9") # latin-1 
    226227        self.assertEquals('x', config.get(u'aä', u'öption0')) 
    227228        self.assertEquals(u"Voilà l'été", config.get(u'aä', 'option1')) 
    228         self.assertEquals(u"Voilà l'été", config.get(u'aä', 'option2')) 
     229        #self.assertEquals(u"Voilà l'été", config.get(u'aä', 'option2')) 
    229230        config.save() 
    230231 
    231232        configfile = open(self.filename, 'r') 
    … … class ConfigurationTestCase(unittest.Tes 
    233234                           '\n', 
    234235                           '[aä]\n', 
    235236                           "option1 = Voilà l'été\n",  
    236                            "option2 = Voilà l'été\n",  
     237                           #"option2 = Voilà l'été\n",  
    237238                           'öption0 = x\n',  
    238239                           # "option3 = Voilà l'été\n",  
    239240                           '\n', 
    … … class ConfigurationTestCase(unittest.Tes 
    245246        config2 = Configuration(self.filename) 
    246247        self.assertEquals('x', config2.get(u'aä', u'öption0')) 
    247248        self.assertEquals(u"Voilà l'été", config2.get(u'aä', 'option1')) 
    248         self.assertEquals(u"Voilà l'été", config2.get(u'aä', 'option2')) 
     249        #self.assertEquals(u"Voilà l'été", config2.get(u'aä', 'option2')) 
    249250        # self.assertEquals(u"Voilà l'été", config2.get('a', 'option3')) 
    250251 
    251252    def test_set_and_save_inherit(self): 
    252253        def testcb(): 
    253254            config = self._read() 
    254             config.set('a', 'option2', "Voilà l'été")  # UTF-8 
     255            #config.set('a', 'option2', "Voilà l'été")  # UTF-8 
    255256            config.set('a', 'option1', u"Voilà l'été") # unicode 
    256257            self.assertEquals('x', config.get('a', 'option')) 
    257258            self.assertEquals(u"Voilà l'été", config.get('a', 'option1')) 
    258             self.assertEquals(u"Voilà l'été", config.get('a', 'option2')) 
     259            #self.assertEquals(u"Voilà l'été", config.get('a', 'option2')) 
    259260            config.save() 
    260261 
    261262            configfile = open(self.filename, 'r') 
    262263            self.assertEquals(['# -*- coding: utf-8 -*-\n', 
    263264                               '\n', 
     265                               '[inherit]\n', 
     266                               "file = trac-site.ini\n",  
    264267                               '[a]\n', 
    265268                               "option1 = Voilà l'été\n",  
    266                                "option2 = Voilà l'été\n",  
    267                                '\n', 
    268                                '[inherit]\n', 
    269                                "file = trac-site.ini\n",  
     269                               #"option2 = Voilà l'été\n",  
    270270                               '\n'], 
    271271                              configfile.readlines()) 
    272272            configfile.close() 
    273273            config2 = Configuration(self.filename) 
    274274            self.assertEquals('x', config2.get('a', 'option')) 
    275275            self.assertEquals(u"Voilà l'été", config2.get('a', 'option1')) 
    276             self.assertEquals(u"Voilà l'été", config2.get('a', 'option2')) 
     276            #self.assertEquals(u"Voilà l'été", config2.get('a', 'option2')) 
    277277        self._test_with_inherit(testcb) 
    278278 
    279279    def test_simple_remove(self): 
    … … class ConfigurationTestCase(unittest.Tes 
    335335        self.assertEquals((u'öption', 'x'), iter(config.options(u'ä')).next()) 
    336336        self.assertEquals(('option', 'y'), iter(config.options('b')).next()) 
    337337        self.assertRaises(StopIteration, iter(config.options('c')).next) 
    338         self.assertEquals(u'öption', iter(config['ä']).next()) 
     338        self.assertEquals(u'öption', iter(config[u'ä']).next()) 
    339339         
    340340        class Foo(object): 
    341341            option_a = Option(u'ä', u'öption2', 'c') 
    … … class ConfigurationTestCase(unittest.Tes 
    424424                self.assertEqual(os.path.join(base, 'site4'), 
    425425                                 config.getpath('c', 'path4', 'site4')) 
    426426            finally: 
    427                 os.remove(site2) 
    428                 os.rmdir(os.path.dirname(site2)) 
     427                if os.path.isfile(site2): 
     428                    os.remove(site2) 
     429                    os.rmdir(os.path.dirname(site2)) 
    429430        finally: 
    430             os.remove(site1) 
    431             os.rmdir(os.path.dirname(site1)) 
     431            if os.path.isfile(site1): 
     432                os.remove(site1) 
     433                os.rmdir(os.path.dirname(site1)) 
    432434 
    433435    def _test_with_inherit(self, testcb): 
    434436        sitename = os.path.join(tempfile.gettempdir(), 'trac-site.ini') 
    … … class ConfigurationTestCase(unittest.Tes 
    439441            self._write(['[inherit]', 'file = trac-site.ini']) 
    440442            testcb() 
    441443        finally: 
    442             os.remove(sitename) 
     444            if os.path.isfile(sitename): 
     445                os.remove(sitename) 
    443446 
    444447 
    445448def suite():