From 1cef3b8196889916e74e7cda09c2eb951c962f7b Mon Sep 17 00:00:00 2001 From: "Brian S. Stephan" Date: Sat, 20 Feb 2021 21:47:39 -0600 Subject: [PATCH] rewrite generate_parent_navs to work on resource paths the old code was kind of impossible to understand by reading it, so this is hopefully considerably clearer --- incorporealcms/pages.py | 47 +++++++++++++++++++++++++---------------- tests/test_pages.py | 16 +++++--------- 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/incorporealcms/pages.py b/incorporealcms/pages.py index c870ca1..3c51176 100644 --- a/incorporealcms/pages.py +++ b/incorporealcms/pages.py @@ -32,17 +32,18 @@ def display_page(path): try: with app.open_instance_resource(resolved_path, 'r') as entry_file: - parent_navs = generate_parent_navs(path) mtime = datetime.datetime.fromtimestamp(os.path.getmtime(entry_file.name), get_localzone()) entry = entry_file.read() - except FileNotFoundError: - logger.warning("requested path '%s' (resolved path '%s') not found!", path, resolved_path) - abort(404) + except OSError: + logger.error("resolved path '%s' could not be opened!", resolved_path) + abort(500) else: md = init_md() content = Markup(md.convert(entry)) logger.debug("file metadata: %s", md.Meta) + parent_navs = generate_parent_navs(resolved_path) + return render('base.html', title=get_meta_str(md, 'title'), description=get_meta_str(md, 'description'), image=get_meta_str(md, 'image'), base_url=request.base_url, content=content, navs=parent_navs, mtime=mtime.strftime('%Y-%m-%d %H:%M:%S %Z')) @@ -135,22 +136,32 @@ def resolve_page_file(path): def generate_parent_navs(path): - """Create a series of paths/links to navigate up from the given path.""" - # derive additional path/location stuff based on path - resolved_path = resolve_page_file(path) - parent_dir = os.path.dirname(resolved_path) - parent_path = '/'.join(path[:-1].split('/')[:-1]) + '/' - - logger.debug("path: '%s'; parent path: '%s'; resolved path: '%s'; parent dir: '%s'", - path, parent_path, resolved_path, parent_dir) - - if path in ('index', '/'): + """Create a series of paths/links to navigate up from the given resource path.""" + if path == 'pages/index.md': + # bail and return the title suffix (generally the domain name) as a terminal case return [(app.config['TITLE_SUFFIX'], '/')] else: - with app.open_instance_resource(resolved_path, 'r') as entry_file: - entry = entry_file.read() + if path.endswith('index.md'): + # index case: one dirname for foo/bar/index.md -> foo/bar, one for foo/bar -> foo + parent_resource_dir = os.path.dirname(os.path.dirname(path)) + else: + # usual case: foo/buh.md -> foo + parent_resource_dir = os.path.dirname(path) + + # generate the request path (i.e. what the link will be) for this path, and + # also the resource path of this parent (which is always a dir, so always index.md) + request_path = f'/{instance_resource_path_to_request_path(path)}' + parent_resource_path = os.path.join(parent_resource_dir, 'index.md') + + logger.debug("resource path: '%s'; request path: '%s'; parent resource path: '%s'", path, + request_path, parent_resource_path) + # for issues regarding parser reuse (see lib.init_md) we reinitialize the parser here md = init_md() + + # read the resource + with app.open_instance_resource(path, 'r') as entry_file: + entry = entry_file.read() _ = Markup(md.convert(entry)) - page_name = " ".join(md.Meta.get('title')) if md.Meta.get('title') else f'/{path}' - return generate_parent_navs(parent_path) + [(page_name, f'/{path}')] + page_name = " ".join(md.Meta.get('title')) if md.Meta.get('title') else request_path + return generate_parent_navs(parent_resource_path) + [(page_name, request_path)] diff --git a/tests/test_pages.py b/tests/test_pages.py index 8bad6ec..bd58555 100644 --- a/tests/test_pages.py +++ b/tests/test_pages.py @@ -24,32 +24,26 @@ def test_resolve_page_file_other_requests_fine(): def test_generate_page_navs_index(app): """Test that the index page has navs to the root (itself).""" with app.app_context(): - assert generate_parent_navs('/') == [('incorporeal.org', '/')] - - -def test_generate_page_navs_alternate_index(app): - """Test that the index page (as a page, not a path) also has navs only to the root (by path).""" - with app.app_context(): - assert generate_parent_navs('index') == [('incorporeal.org', '/')] + assert generate_parent_navs('pages/index.md') == [('incorporeal.org', '/')] def test_generate_page_navs_subdir_index(app): """Test that dir pages have navs to the root and themselves.""" with app.app_context(): - assert generate_parent_navs('subdir/') == [('incorporeal.org', '/'), ('/subdir/', '/subdir/')] + assert generate_parent_navs('pages/subdir/index.md') == [('incorporeal.org', '/'), ('/subdir/', '/subdir/')] def test_generate_page_navs_subdir_real_page(app): """Test that real pages have navs to the root, their parent, and themselves.""" with app.app_context(): - assert generate_parent_navs('subdir/page') == [('incorporeal.org', '/'), ('/subdir/', '/subdir/'), - ('Page', '/subdir/page')] + assert generate_parent_navs('pages/subdir/page.md') == [('incorporeal.org', '/'), ('/subdir/', '/subdir/'), + ('Page', '/subdir/page')] def test_generate_page_navs_subdir_with_title_parsing_real_page(app): """Test that title metadata is used in the nav text.""" with app.app_context(): - assert generate_parent_navs('subdir-with-title/page') == [ + assert generate_parent_navs('pages/subdir-with-title/page.md') == [ ('incorporeal.org', '/'), ('SUB!', '/subdir-with-title/'), ('/subdir-with-title/page', '/subdir-with-title/page')