books.Loader: New loading strategy based on load_file. RT#11034.

Building a string and loading it means Beancount can never cache any
load. It only caches top-level file loads because options in the
top-level file can change the semantics of included entries.

Instead use load_file as much as possible, and filter entries as
needed.

conservancy_beancount/books.py

@@ -110,54 +110,39 @@ class Loader:
           load for a given date range.
         """
         self.books_root = books_root
-        self.opening_root = books_root / 'books'
         self.fiscal_year = fiscal_year
 
     def _iter_fy_books(self, fy_range: Iterable[int]) -> Iterator[Path]:
-        dir_path = self.opening_root
         for year in fy_range:
-            path = dir_path / f'{year}.beancount'
+            path = Path(self.books_root, 'books', f'{year}.beancount')
             if path.exists():
                 yield path
-                dir_path = self.books_root
-
-    def fy_range_string(self,
-                        from_fy: Year,
-                        to_fy: Optional[Year]=None,
-    ) -> str:
-        """Return a string to load books for a range of fiscal years
-
-        This method generates a range of fiscal years by calling
-        FiscalYear.range() with its first two arguments. It returns a string of
-        Beancount directives to load the books from the first available fiscal
-        year through the end of the range.
-
-        Pass the string to Loader.load_string() to actually load data from it.
-        """
-        paths = self._iter_fy_books(self.fiscal_year.range(from_fy, to_fy))
-        try:
-            with next(paths).open() as opening_books:
-                lines = [opening_books.read()]
-        except StopIteration:
-            return ''
-        for path in paths:
-            lines.append(f'include "../{path.name}"')
-        return '\n'.join(lines)
-
-    def load_string(self, source: str) -> LoadResult:
-        """Load a generated string of Beancount directives
-
-        This method takes a string generated by another Loader method, like
-        fy_range_string, and loads it through Beancount, setting up the
-        environment as necessary to do that.
-        """
-        with workdir(self.opening_root):
-            retval: LoadResult = bc_loader.load_string(source)
-        return retval
 
     def load_fy_range(self,
                       from_fy: Year,
                       to_fy: Optional[Year]=None,
     ) -> LoadResult:
-        """Load books for a range of fiscal years"""
+        """Load books for a range of fiscal years
-        return self.load_string(self.fy_range_string(from_fy, to_fy))
+
+        This method generates a range of fiscal years by calling
+        FiscalYear.range() with its first two arguments. It returns a string of
+        Beancount directives to load the books from the first available fiscal
+        year through the end of the range.
+        """
+        fy_range = self.fiscal_year.range(from_fy, to_fy)
+        fy_paths = self._iter_fy_books(fy_range)
+        try:
+            entries, errors, options_map = bc_loader.load_file(next(fy_paths))
+        except StopIteration:
+            entries, errors, options_map = [], [], {}
+        for load_path in fy_paths:
+            new_entries, new_errors, new_options = bc_loader.load_file(load_path)
+            # We only want transactions from the new fiscal year.
+            # We don't want the opening balance, duplicate definitions, etc.
+            fy_filename = str(load_path.parent.parent / load_path.name)
+            entries.extend(
+                entry for entry in new_entries
+                if entry.meta.get('filename') == fy_filename
+            )
+            errors.extend(new_errors)
+        return entries, errors, options_map

tests/books/books/2018.beancount

@@ -1,3 +1,3 @@
 option "title" "Books from 2018"
-plugin "beancount.plugins.auto"
+include "../definitions.beancount"
 include "../2018.beancount"

tests/books/books/2019.beancount

@@ -1,3 +1,3 @@
 option "title" "Books from 2019"
-plugin "beancount.plugins.auto"
+include "../definitions.beancount"
 include "../2019.beancount"

tests/books/books/2020.beancount

@@ -1,3 +1,3 @@
 option "title" "Books from 2020"
-plugin "beancount.plugins.auto"
+include "../definitions.beancount"
 include "../2020.beancount"

tests/books/definitions.beancount

@@ -0,0 +1,2 @@
+2018-03-01 open Assets:Checking
+2018-03-01 open Income:Donations

tests/test_books_loader.py

@@ -32,48 +32,42 @@ books_path = testutil.test_path('books')
 def conservancy_loader():
     return books.Loader(books_path, books.FiscalYear(3))
 
-def include_patterns(years, subdir='..'):
+@pytest.mark.parametrize('from_fy,to_fy,expect_years', [
-    for year in years:
+    (2019, 2019, range(2019, 2020)),
-        path = Path(subdir, f'{year}.beancount')
+    (0, 2019, range(2019, 2020)),
-        yield rf'^include "{re.escape(str(path))}"$'
+    (2018, 2019, range(2018, 2020)),
-
+    (1, 2018, range(2018, 2020)),
-@pytest.mark.parametrize('range_start,range_stop,expect_years', [
+    (-1, 2019, range(2018, 2020)),
-    (2019, 2020, [2019, 2020]),
+    (2019, 2020, range(2019, 2021)),
-    (-1, 2020, [2019, 2020]),
+    (1, 2019, range(2019, 2021)),
-    (10, 2019, [2019, 2020]),
+    (-1, 2020, range(2019, 2021)),
-    (-10, 2019, [2018, 2019]),
+    (2010, 2030, range(2018, 2021)),
-    (date(2019, 1, 1), date(2020, 6, 1), [2018, 2019, 2020]),
+    (20, 2010, range(2018, 2021)),
-    (-1, date(2020, 2, 1), [2018, 2019]),
+    (-20, 2030, range(2018, 2021)),
 ])
-def test_fy_range_string(conservancy_loader, range_start, range_stop, expect_years):
+def test_load_fy_range(conservancy_loader, from_fy, to_fy, expect_years):
-    actual = conservancy_loader.fy_range_string(range_start, range_stop)
+    entries, errors, options_map = conservancy_loader.load_fy_range(from_fy, to_fy)
-    testutil.check_lines_match(actual.splitlines(), [
-        rf'^option "title" "Books from {expect_years[0]}"$',
-        rf'^plugin "beancount\.plugins\.auto"$',
-        *include_patterns(expect_years),
-    ])
-
-@pytest.mark.parametrize('year_offset', range(-3, 1))
-def test_fy_range_string_with_offset(conservancy_loader, year_offset):
-    base_year = 2020
-    start_year = max(2018, base_year + year_offset)
-    expect_years = range(start_year, base_year + 1)
-    actual = conservancy_loader.fy_range_string(year_offset, base_year)
-    testutil.check_lines_match(actual.splitlines(), include_patterns(expect_years))
-
-def test_fy_range_string_empty_range(conservancy_loader):
-    assert conservancy_loader.fy_range_string(2020, 2019) == ''
-
-def test_load_fy_range(conservancy_loader):
-    entries, errors, options_map = conservancy_loader.load_fy_range(2018, 2019)
     assert not errors
     narrations = {getattr(entry, 'narration', None) for entry in entries}
-    assert '2018 donation' in narrations
+    assert ('2018 donation' in narrations) == (2018 in expect_years)
-    assert '2019 donation' in narrations
+    assert ('2019 donation' in narrations) == (2019 in expect_years)
-    assert '2020 donation' not in narrations
+    assert ('2020 donation' in narrations) == (2020 in expect_years)
+
+def test_load_fy_range_does_not_duplicate_openings(conservancy_loader):
+    entries, errors, options_map = conservancy_loader.load_fy_range(2010, 2030)
+    openings = []
+    open_accounts = set()
+    for entry in entries:
+        try:
+            open_accounts.add(entry.account)
+        except AttributeError:
+            pass
+        else:
+            openings.append(entry)
+    assert len(openings) == len(open_accounts)
 
 def test_load_fy_range_empty(conservancy_loader):
     entries, errors, options_map = conservancy_loader.load_fy_range(2020, 2019)
     assert not errors
     assert not entries
-    assert options_map.get('input_hash') == hashlib.md5().hexdigest()
+    assert not options_map

tests/test_config.py

@@ -398,7 +398,9 @@ def test_books_loader():
     config = config_mod.Config()
     config.load_string(f'[Beancount]\nbooks dir = {books_path}\n')
     loader = config.books_loader()
-    assert loader.fy_range_string(2020, 2020)
+    entries, errors, _ = loader.load_fy_range(2020, 2020)
+    assert entries
+    assert not errors
 
 def test_books_loader_without_books():
     assert config_mod.Config().books_loader() is None

tests/testutil.py

@@ -183,10 +183,8 @@ class TestBooksLoader(books.Loader):
     def __init__(self, source):
         self.source = source
 
-    def fy_range_string(self, from_fy=None, to_fy=None, plugins=None):
+    def load_fy_range(self, from_fy, to_fy=None):
-        return f'include "{self.source}"'
+        return bc_loader.load_file(self.source)
-
-    load_string = staticmethod(bc_loader.load_string)
 
 
 class TestConfig: