From d399e097caf48927e6b036ad0833d6f7c8e42ce6 Mon Sep 17 00:00:00 2001 From: Doug Blank Date: Sat, 18 Apr 2026 10:05:45 -0400 Subject: [PATCH] Add sniff_function support to ImportPlugin for version-aware dispatch. Previously, Gramps mapped each file extension to a single importer. This change adds an optional sniff_function attribute to ImportPlugin and PluginData so that multiple importers can register for the same file extension and be distinguished by file content rather than extension alone. When the importer for a file is selected, plugins that declare a sniff_function are tested first. The first plugin whose sniff function returns True for the file wins. Plugins without a sniff function continue to work as before, acting as a fallback. This makes it possible for an addon to register a GEDCOM 7 importer alongside the built-in GEDCOM 5.5 importer: the addon's sniff function checks for the VERS 7.x header and claims the file when present, leaving all other GEDCOM files to the existing importer. The same dispatch logic is applied in both the GUI (dbloader.py) and the CLI (arghandler.py). --- gramps/cli/arghandler.py | 46 ++++- gramps/gen/plug/_import.py | 80 ++++++--- gramps/gen/plug/_manager.py | 4 + gramps/gen/plug/_pluginreg.py | 31 ++++ gramps/gen/plug/test/sniff_function_test.py | 179 ++++++++++++++++++++ gramps/gui/dbloader.py | 53 +++++- po/POTFILES.skip | 1 + 7 files changed, 363 insertions(+), 31 deletions(-) create mode 100644 gramps/gen/plug/test/sniff_function_test.py diff --git a/gramps/cli/arghandler.py b/gramps/cli/arghandler.py index 0c28dea0fe4..a38d11f9f0e 100644 --- a/gramps/cli/arghandler.py +++ b/gramps/cli/arghandler.py @@ -147,6 +147,43 @@ def _split_options(options_str): return options_str_dict +def _choose_import_plugin(plugins, extension, filename): + """ + Return the best import plugin for *filename* among *plugins*. + + Plugins that declare a ``sniff_function`` are tested first; the first one + whose sniff function returns ``True`` for *filename* is returned. If none + matches, the first plugin whose extension equals *extension* and that has + no sniff function is returned as a fallback. Returns ``None`` when no + suitable plugin is found. + + This allows multiple importers to share the same file extension while each + handling a distinct file version (e.g. GEDCOM 5.5 vs GEDCOM 7.0). + + :param plugins: list of :class:`~gramps.gen.plug.ImportPlugin` instances + :type plugins: list + :param extension: lower-case file extension without leading dot + :type extension: str + :param filename: full path of the file to import + :type filename: str + :returns: the chosen plugin, or ``None`` + :rtype: :class:`~gramps.gen.plug.ImportPlugin` | None + """ + candidates = [p for p in plugins if extension == p.get_extension()] + for plugin in candidates: + sniff = plugin.get_sniff_function() + if sniff is not None: + try: + if sniff(filename): + return plugin + except Exception: # pylint: disable=broad-except + pass + for plugin in candidates: + if plugin.get_sniff_function() is None: + return plugin + return None + + # ------------------------------------------------------------------------- # ArgHandler # ------------------------------------------------------------------------- @@ -595,10 +632,11 @@ def cl_import(self, filename, family_tree_format): Try to import filename using the family_tree_format. """ pmgr = BasePluginManager.get_instance() - for plugin in pmgr.get_import_plugins(): - if family_tree_format == plugin.get_extension(): - import_function = plugin.get_import_function() - import_function(self.dbstate.db, filename, self.user) + plugin = _choose_import_plugin( + pmgr.get_import_plugins(), family_tree_format, filename + ) + if plugin is not None: + plugin.get_import_function()(self.dbstate.db, filename, self.user) # ------------------------------------------------------------------------- # diff --git a/gramps/gen/plug/_import.py b/gramps/gen/plug/_import.py index 1bddbc9370d..19a515207a1 100644 --- a/gramps/gen/plug/_import.py +++ b/gramps/gen/plug/_import.py @@ -21,52 +21,92 @@ This module provides the :class:`.Plugin` class for import plugins. """ +# ------------------------ +# Python modules +# ------------------------ +from collections.abc import Callable + +# ------------------------ +# Gramps modules +# ------------------------ from . import Plugin +# ------------------------------------------------------------ +# +# ImportPlugin +# +# ------------------------------------------------------------ class ImportPlugin(Plugin): """ - This class represents a plugin for importing data into Gramps + This class represents a plugin for importing data into Gramps. """ - def __init__(self, name, description, import_function, extension): + def __init__( + self, + name: str, + description: str, + import_function: Callable, + extension: str, + sniff_function: Callable | None = None, + ) -> None: """ + Initialise the ImportPlugin. + :param name: A friendly name to call this plugin. Example: "GEDCOM Import" - :type name: string + :type name: str :param description: A short description of the plugin. Example: "This plugin will import a GEDCOM file into a database" - :type description: string + :type description: str :param import_function: A function to call to perform the import. - The function must take the form: - def import_function(db, filename, user): - where: - "db" is a Gramps database to import the data into - "filename" is the file that contains data to be imported - "user" is an instance of the User class implementing - GUI functions (callbacks, errors, warnings, etc) + The function must take the form ``import_function(db, filename, + user)`` where *db* is a Gramps database, *filename* is the file + to import, and *user* is a :class:`gramps.gen.user.User` instance. :type import_function: callable - :param extension: The extension for the files imported by this plugin. - Example: "ged" + :param extension: The file extension handled by this plugin (without + the leading dot). Example: ``"ged"`` :type extension: str - :return: nothing + :param sniff_function: Optional callable that accepts a filename and + returns ``True`` when this plugin should handle that file. Used to + disambiguate between importers that share the same extension (e.g. + GEDCOM 5.5 vs GEDCOM 7.0). When ``None`` (the default) the plugin + acts as a fallback for its extension. + :type sniff_function: callable | None """ Plugin.__init__(self, name, description, import_function.__module__) self.__import_func = import_function self.__extension = extension + self.__sniff_func = sniff_function - def get_import_function(self): + def get_import_function(self) -> Callable: """ - Get the import function for this plugins. + Return the import function for this plugin. - :return: the callable import_function passed into :meth:`__init__` + :returns: the callable import_function passed into :meth:`__init__` + :rtype: callable """ return self.__import_func - def get_extension(self): + def get_extension(self) -> str: """ - Get the extension for the files imported by this plugin. + Return the file extension handled by this plugin. - :return: str + :returns: file extension string (without leading dot) + :rtype: str """ return self.__extension + + def get_sniff_function(self) -> Callable | None: + """ + Return the sniff function for this plugin, or ``None`` if not set. + + The sniff function accepts a filename and returns ``True`` when this + plugin is the correct handler for the file. It is called when multiple + importers are registered for the same extension so that the right one + can be selected based on file content rather than extension alone. + + :returns: the sniff callable, or ``None`` + :rtype: callable | None + """ + return self.__sniff_func diff --git a/gramps/gen/plug/_manager.py b/gramps/gen/plug/_manager.py index e02427f84e3..3712ad91ac7 100644 --- a/gramps/gen/plug/_manager.py +++ b/gramps/gen/plug/_manager.py @@ -557,11 +557,15 @@ def get_import_plugins(self): continue mod = self.load_plugin(pdata) if mod: + sniff_func = None + if pdata.sniff_function: + sniff_func = getattr(mod, pdata.sniff_function, None) imp = ImportPlugin( name=pdata.name, description=pdata.description, import_function=getattr(mod, pdata.import_function), extension=pdata.extension, + sniff_function=sniff_func, ) self.__import_plugins.append(imp) diff --git a/gramps/gen/plug/_pluginreg.py b/gramps/gen/plug/_pluginreg.py index 7adafaf5d86..c10b479ba55 100644 --- a/gramps/gen/plug/_pluginreg.py +++ b/gramps/gen/plug/_pluginreg.py @@ -384,6 +384,11 @@ class PluginData: .. attribute:: import_function Function that starts an import + .. attribute:: sniff_function + Optional function that returns True if the plugin can handle the given + file, used to distinguish between multiple importers for the same + extension (e.g. GEDCOM 5.5 vs GEDCOM 7.0). + Attributes for GRAMPLET plugins .. attribute:: gramplet @@ -507,6 +512,7 @@ def __init__(self): self._export_options_title = "" # IMPORT attr self._import_function = None + self._sniff_function = None # GRAMPLET attr self._gramplet = None self._height = 200 @@ -1000,6 +1006,31 @@ def import_function(self, import_function): raise ValueError("import_function may only be set for IMPORT plugins") self._import_function = import_function + @property + def sniff_function(self): + """ + Return the name of the sniff function for this import plugin. + + :returns: The name of the sniff function, or ``None`` if not set. + :rtype: str | None + """ + return self._sniff_function + + @sniff_function.setter + def sniff_function(self, sniff_function: str) -> None: + """ + Set the name of the sniff function for this import plugin. + + :param sniff_function: The name of a callable in the plugin module + that accepts a filename and returns ``True`` if this plugin + should handle the file. Used when multiple importers share the + same extension (e.g. GEDCOM 5.5 vs GEDCOM 7.0). + :type sniff_function: str + """ + if self._ptype != IMPORT: + raise ValueError("sniff_function may only be set for IMPORT plugins") + self._sniff_function = sniff_function + # GRAMPLET attributes @property def gramplet(self): diff --git a/gramps/gen/plug/test/sniff_function_test.py b/gramps/gen/plug/test/sniff_function_test.py new file mode 100644 index 00000000000..23608353fd8 --- /dev/null +++ b/gramps/gen/plug/test/sniff_function_test.py @@ -0,0 +1,179 @@ +# +# Gramps - a GTK+/GNOME based genealogy program +# +# Copyright (C) 2026 Doug Blank +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +# + +""" +Tests for the sniff_function dispatch mechanism on ImportPlugin. +""" + +# ------------------------ +# Python modules +# ------------------------ +import unittest + +# ------------------------ +# Gramps modules +# ------------------------ +from gramps.gen.plug._import import ImportPlugin + + +def _make_plugin(name, extension, sniff_fn=None): + """Return a minimal ImportPlugin for testing.""" + + def _import(db, filename, user): + """Stub import function.""" + + _import.__module__ = "test" + return ImportPlugin( + name=name, + description="", + import_function=_import, + extension=extension, + sniff_function=sniff_fn, + ) + + +def _choose(plugins, extension, filename): + """Re-implementation of the dispatch logic under test.""" + candidates = [p for p in plugins if extension == p.get_extension()] + for plugin in candidates: + sniff = plugin.get_sniff_function() + if sniff is not None: + try: + if sniff(filename): + return plugin + except Exception: # pylint: disable=broad-except + pass + for plugin in candidates: + if plugin.get_sniff_function() is None: + return plugin + return None + + +class TestImportPluginSniffFunction(unittest.TestCase): + """Tests for ImportPlugin.get_sniff_function().""" + + def test_no_sniff_function_returns_none(self): + """ + A plugin created without a sniff_function returns None from + get_sniff_function(). + """ + plugin = _make_plugin("Legacy", "ged") + self.assertIsNone(plugin.get_sniff_function()) + + def test_sniff_function_stored_and_returned(self): + """ + A plugin created with a sniff_function returns that callable from + get_sniff_function(). + """ + sniff = lambda f: True + plugin = _make_plugin("Modern", "ged", sniff_fn=sniff) + self.assertIs(plugin.get_sniff_function(), sniff) + + +class TestChooseImportPlugin(unittest.TestCase): + """Tests for the sniff-aware plugin selection logic.""" + + def test_fallback_when_no_sniff_plugins(self): + """ + When no plugin has a sniff function the first extension match is + returned. + """ + p1 = _make_plugin("A", "ged") + p2 = _make_plugin("B", "ged") + result = _choose([p1, p2], "ged", "family.ged") + self.assertIs(result, p1) + + def test_sniff_winner_beats_fallback(self): + """ + A plugin whose sniff function returns True is preferred over a + fallback plugin that has no sniff function. + """ + fallback = _make_plugin("Legacy", "ged") + winner = _make_plugin("Modern", "ged", sniff_fn=lambda f: True) + result = _choose([fallback, winner], "ged", "family.ged") + self.assertIs(result, winner) + + def test_sniff_false_falls_through_to_fallback(self): + """ + A plugin whose sniff function returns False does not win; the + fallback (no sniff function) is returned instead. + """ + rejector = _make_plugin("Modern", "ged", sniff_fn=lambda f: False) + fallback = _make_plugin("Legacy", "ged") + result = _choose([rejector, fallback], "ged", "family.ged") + self.assertIs(result, fallback) + + def test_sniff_exception_falls_through(self): + """ + If the sniff function raises an exception the plugin is skipped and + the fallback is used. + """ + + def _bad_sniff(filename): + """Sniff function that always raises.""" + raise RuntimeError("sniff failed") + + rejector = _make_plugin("Broken", "ged", sniff_fn=_bad_sniff) + fallback = _make_plugin("Legacy", "ged") + result = _choose([rejector, fallback], "ged", "family.ged") + self.assertIs(result, fallback) + + def test_no_matching_extension_returns_none(self): + """When no plugin matches the extension, None is returned.""" + plugin = _make_plugin("CSV", "csv") + result = _choose([plugin], "ged", "family.ged") + self.assertIsNone(result) + + def test_all_sniff_false_no_fallback_returns_none(self): + """ + When every candidate has a sniff function that returns False and + there is no fallback plugin, None is returned. + """ + p1 = _make_plugin("A", "ged", sniff_fn=lambda f: False) + p2 = _make_plugin("B", "ged", sniff_fn=lambda f: False) + result = _choose([p1, p2], "ged", "family.ged") + self.assertIsNone(result) + + def test_first_sniff_true_wins_among_multiple_sniffers(self): + """ + When multiple plugins have sniff functions that return True, the + first one encountered wins. + """ + p1 = _make_plugin("First", "ged", sniff_fn=lambda f: True) + p2 = _make_plugin("Second", "ged", sniff_fn=lambda f: True) + result = _choose([p1, p2], "ged", "family.ged") + self.assertIs(result, p1) + + def test_sniff_receives_filename(self): + """The sniff function is called with the filename argument.""" + received = [] + + def _record_sniff(filename): + """Record the filename passed to the sniff function.""" + received.append(filename) + return True + + plugin = _make_plugin("Recorder", "ged", sniff_fn=_record_sniff) + _choose([plugin], "ged", "/path/to/family.ged") + self.assertEqual(received, ["/path/to/family.ged"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/gramps/gui/dbloader.py b/gramps/gui/dbloader.py index 29a83b462ea..cf0b3701be7 100644 --- a/gramps/gui/dbloader.py +++ b/gramps/gui/dbloader.py @@ -260,6 +260,43 @@ def read_file(self, filename, username=None, password=None): # FileChooser filters: what to show in the file chooser # # ------------------------------------------------------------------------- +def _choose_import_plugin(plugins, extension, filename): + """ + Return the best import plugin for *filename* among *plugins*. + + Plugins that declare a ``sniff_function`` are tested first; the first one + whose sniff function returns ``True`` for *filename* is returned. If none + matches, the first plugin whose extension equals *extension* and that has + no sniff function is returned as a fallback. Returns ``None`` when no + suitable plugin is found. + + This allows multiple importers to share the same file extension while + each handling a distinct file version (e.g. GEDCOM 5.5 vs GEDCOM 7.0). + + :param plugins: list of :class:`~gramps.gen.plug.ImportPlugin` instances + :type plugins: list + :param extension: lower-case file extension without leading dot + :type extension: str + :param filename: full path of the file to import + :type filename: str + :returns: the chosen plugin, or ``None`` + :rtype: :class:`~gramps.gen.plug.ImportPlugin` | None + """ + candidates = [p for p in plugins if extension == p.get_extension()] + for plugin in candidates: + sniff = plugin.get_sniff_function() + if sniff is not None: + try: + if sniff(filename): + return plugin + except Exception: # pylint: disable=broad-except + pass + for plugin in candidates: + if plugin.get_sniff_function() is None: + return plugin + return None + + def add_all_files_filter(chooser): """ Add an all-permitting filter to the file chooser dialog. @@ -447,13 +484,15 @@ def __init__(self, dbstate, uistate, callback=None): # or an empty string. extension = os.path.splitext(filename)[-1][1:].lower() - for plugin in pmgr.get_import_plugins(): - if extension == plugin.get_extension(): - self.close() - self.do_import(plugin.get_import_function(), filename) - if callback is not None: - callback(self.import_info) - return + plugin = _choose_import_plugin( + pmgr.get_import_plugins(), extension, filename + ) + if plugin is not None: + self.close() + self.do_import(plugin.get_import_function(), filename) + if callback is not None: + callback(self.import_info) + return # Finally, we give up and declare this an unknown format ErrorDialog( diff --git a/po/POTFILES.skip b/po/POTFILES.skip index b78b6b14dd5..c75a8694fee 100644 --- a/po/POTFILES.skip +++ b/po/POTFILES.skip @@ -211,6 +211,7 @@ gramps/gen/plug/_docgenplugin.py gramps/gen/plug/_export.py gramps/gen/plug/_import.py gramps/gen/plug/_plugin.py +gramps/gen/plug/test/sniff_function_test.py # # gen.plug.docbackend #