Re: [PATCH 2/2] python/notmuch2: add bindings for the database config strings

[Floris Bruynooghe <flub@devork.be>]

Thanks for adding more of the API!

This mostly is fine as well, again I'd mainly ask to add tests however.
At least the things which are implemented directly I guess: setitem,
getitem, iter and len.


On Sat 09 May 2020 at 07:05 +0200, Anton Khirnov wrote:

> ---
>  bindings/python-cffi/notmuch2/_build.py    | 17 +++++
>  bindings/python-cffi/notmuch2/_config.py   | 84 ++++++++++++++++++++++
>  bindings/python-cffi/notmuch2/_database.py | 23 ++++++
>  3 files changed, 124 insertions(+)
>  create mode 100644 bindings/python-cffi/notmuch2/_config.py
>
> diff --git a/bindings/python-cffi/notmuch2/_build.py b/bindings/python-cffi/notmuch2/_build.py
> index 5e1fcac1..f269f2a1 100644
> --- a/bindings/python-cffi/notmuch2/_build.py
> +++ b/bindings/python-cffi/notmuch2/_build.py
> @@ -314,6 +314,23 @@ ffibuilder.cdef(
>      notmuch_indexopts_get_decrypt_policy (const notmuch_indexopts_t *indexopts);
>      void
>      notmuch_indexopts_destroy (notmuch_indexopts_t *options);
> +
> +    notmuch_status_t
> +    notmuch_database_set_config (notmuch_database_t *db, const char *key, const char *value);
> +    notmuch_status_t
> +    notmuch_database_get_config (notmuch_database_t *db, const char *key, char **value);
> +    notmuch_status_t
> +    notmuch_database_get_config_list (notmuch_database_t *db, const char *prefix, notmuch_config_list_t **out);
> +    notmuch_bool_t
> +    notmuch_config_list_valid (notmuch_config_list_t *config_list);
> +    const char *
> +    notmuch_config_list_key (notmuch_config_list_t *config_list);
> +    const char *
> +    notmuch_config_list_value (notmuch_config_list_t *config_list);
> +    void
> +    notmuch_config_list_move_to_next (notmuch_config_list_t *config_list);
> +    void
> +    notmuch_config_list_destroy (notmuch_config_list_t *config_list);
>      """
>  )
>  
> diff --git a/bindings/python-cffi/notmuch2/_config.py b/bindings/python-cffi/notmuch2/_config.py
> new file mode 100644
> index 00000000..58383c16
> --- /dev/null
> +++ b/bindings/python-cffi/notmuch2/_config.py
> @@ -0,0 +1,84 @@
> +import collections.abc
> +
> +import notmuch2._base as base
> +import notmuch2._capi as capi
> +import notmuch2._errors as errors
> +
> +__all__ = ['ConfigMapping']
> +

Same style thing about 2 blank lines

> +class ConfigIter(base.NotmuchIter):
> +    def __init__(self, parent, iter_p):
> +        super().__init__(
> +            parent, iter_p,
> +            fn_destroy=capi.lib.notmuch_config_list_destroy,
> +            fn_valid=capi.lib.notmuch_config_list_valid,
> +            fn_get=capi.lib.notmuch_config_list_key,
> +            fn_next=capi.lib.notmuch_config_list_move_to_next)
> +
> +    def __next__(self):
> +        item = super().__next__()
> +        return base.BinString.from_cffi(item)
> +
> +class ConfigMapping(base.NotmuchObject, collections.abc.MutableMapping):
> +    """The config key/value pairs stored in the database.
> +
> +    The entries are exposed as a :class:`collections.abc.MutableMapping` object.
> +    Note that setting a value to an empty string is the same as deleting it.
> +
> +    :param parent: the parent object
> +    :param ptr_name: the name of the attribute on the parent which will
> +       return the memory pointer.  This allows this object to
> +       access the pointer via the parent's descriptor and thus
> +       trigger :class:`MemoryPointer`'s memory safety.
> +    """
> +
> +    def __init__(self, parent, ptr_name):
> +        self._parent = parent
> +        self._ptr = lambda: getattr(parent, ptr_name)
> +
> +    @property
> +    def alive(self):
> +        return self._parent.alive
> +
> +    def _destroy(self):
> +        pass
> +
> +    def __getitem__(self, key):
> +        if isinstance(key, str):
> +            key = key.encode('utf-8')
> +        val_pp = capi.ffi.new('char**')
> +        ret = capi.lib.notmuch_database_get_config(self._ptr(), key, val_pp)
> +        if ret != capi.lib.NOTMUCH_STATUS_SUCCESS:
> +            raise errors.NotmuchError(ret)
> +        if val_pp[0] == "":
> +            capi.lib.free(val_pp[0])
> +            raise KeyError
> +        val = base.BinString.from_cffi(val_pp[0])
> +        capi.lib.free(val_pp[0])
> +        return val
> +
> +    def __setitem__(self, key, val):
> +        if isinstance(key, str):
> +            key = key.encode('utf-8')
> +        if isinstance(val, str):
> +            val = val.encode('utf-8')
> +        ret = capi.lib.notmuch_database_set_config(self._ptr(), key, val)
> +        if ret != capi.lib.NOTMUCH_STATUS_SUCCESS:
> +            raise errors.NotmuchError(ret)
> +
> +    def __delitem__(self, key):
> +        self[key] = ""
> +
> +    def __iter__(self):
> +        """Return an iterator over the config items.
> +
> +        :raises NullPointerError: If the iterator can not be created.
> +        """
> +        configlist_pp = capi.ffi.new('notmuch_config_list_t**')
> +        ret = capi.lib.notmuch_database_get_config_list(self._ptr(), b'', configlist_pp)
> +        if ret != capi.lib.NOTMUCH_STATUS_SUCCESS:
> +            raise errors.NotmuchError(ret)
> +        return ConfigIter(self._parent, configlist_pp[0])
> +
> +    def __len__(self):
> +        return sum(1 for t in self)
> diff --git a/bindings/python-cffi/notmuch2/_database.py b/bindings/python-cffi/notmuch2/_database.py
> index f14eac78..fc55fea8 100644
> --- a/bindings/python-cffi/notmuch2/_database.py
> +++ b/bindings/python-cffi/notmuch2/_database.py
> @@ -7,6 +7,7 @@ import pathlib
>  import weakref
>  
>  import notmuch2._base as base
> +import notmuch2._config as config
>  import notmuch2._capi as capi
>  import notmuch2._errors as errors
>  import notmuch2._message as message
> @@ -536,6 +537,28 @@ class Database(base.NotmuchObject):
>              self._cached_tagset = weakref.ref(tagset)
>          return tagset
>  
> +    @property
> +    def config(self):
> +        """Return a mutable mapping with the settings stored in this database.
> +
> +        This returns an mutable dict-like object implementing the
> +        collections.abc.MutableMapping Abstract Base Class.
> +
> +        :rtype: Config
> +
> +        :raises ObjectDestroyedError: if used after destroyed.
> +        """
> +        try:
> +            ref = self._cached_config
> +        except AttributeError:
> +            config_mapping = None
> +        else:
> +            config_mapping = ref()
> +        if config_mapping is None:
> +            config_mapping = config.ConfigMapping(self, '_db_p')
> +            self._cached_config = weakref.ref(config_mapping)
> +        return config_mapping
> +
>      def _create_query(self, query, *,
>                        omit_excluded=EXCLUDE.TRUE,
>                        sort=SORT.UNSORTED,  # Check this default