diff --git a/pillar/api/node_types/__init__.py b/pillar/api/node_types/__init__.py index d6bd585d..53f4e1cf 100644 --- a/pillar/api/node_types/__init__.py +++ b/pillar/api/node_types/__init__.py @@ -7,7 +7,7 @@ _file_embedded_schema = { } } -ATTACHMENT_SLUG_REGEX = '[a-zA-Z0-9_]+' +ATTACHMENT_SLUG_REGEX = r'[a-zA-Z0-9_\-]+' attachments_embedded_schema = { 'type': 'dict', diff --git a/pillar/cli/maintenance.py b/pillar/cli/maintenance.py index 91ccecdc..22a5fdca 100644 --- a/pillar/cli/maintenance.py +++ b/pillar/cli/maintenance.py @@ -794,7 +794,6 @@ def upgrade_attachment_usage(proj_url=None, all_projects=False, go=False): return 1 import html - from pillar.api.node_types import ATTACHMENT_SLUG_REGEX from pillar.api.projects.utils import node_type_dict from pillar.api.utils import remove_private_keys from pillar.api.utils.authentication import force_cli_user @@ -803,7 +802,11 @@ def upgrade_attachment_usage(proj_url=None, all_projects=False, go=False): nodes_coll = current_app.db('nodes') total_nodes = 0 - old_slug_re = re.compile(r'@\[(%s)\]' % ATTACHMENT_SLUG_REGEX) + failed_node_ids = set() + + # Use a mixture of the old slug RE that still allowes spaces in the slug + # name and the new RE that allows dashes. + old_slug_re = re.compile(r'@\[([a-zA-Z0-9_\- ]+)\]') for proj in _db_projects(proj_url, all_projects, go=go): proj_id = proj['_id'] proj_url = proj.get('url', '-no-url-') @@ -816,15 +819,16 @@ def upgrade_attachment_usage(proj_url=None, all_projects=False, go=False): if node_count == 0: log.debug('Skipping project %s (%s)', proj_url, proj_id) continue - total_nodes += node_count proj_node_types = node_type_dict(proj) for node in nodes: attachments = node['properties']['attachments'] + replaced = False # Inner functions because of access to the node's attachments. def replace(match): + nonlocal replaced slug = match.group(1) log.debug(' - OLD STYLE attachment slug %r', slug) try: @@ -840,7 +844,8 @@ def upgrade_attachment_usage(proj_url=None, all_projects=False, go=False): url = att.get('link_custom') if url: link = " link='%s'" % html.escape(url) - return '{attachment %r%s}' % (slug, link) + replaced = True + return '{attachment %r%s}' % (slug.replace(' ', '-'), link) def update_markdown(value: str) -> str: return old_slug_re.sub(replace, value) @@ -848,9 +853,19 @@ def upgrade_attachment_usage(proj_url=None, all_projects=False, go=False): iter_markdown(proj_node_types, node, update_markdown) # Remove no longer used properties from attachments - for attachment in attachments.values(): + new_attachments = {} + for slug, attachment in attachments.items(): + replaced |= 'link' in attachment # link_custom implies link attachment.pop('link', None) attachment.pop('link_custom', None) + new_attachments[slug.replace(' ', '-')] = attachment + node['properties']['attachments'] = new_attachments + + if replaced: + total_nodes += 1 + else: + # Nothing got replaced, + continue if go: # Use Eve to PUT, so we have schema checking. @@ -858,13 +873,16 @@ def upgrade_attachment_usage(proj_url=None, all_projects=False, go=False): r, _, _, status = current_app.put_internal('nodes', db_node, _id=node['_id']) if status != 200: log.error('Error %i storing altered node %s %s', status, node['_id'], r) - raise SystemExit('Error storing node; see log.') + failed_node_ids.add(node['_id']) + # raise SystemExit('Error storing node; see log.') log.debug('Updated node %s: %s', node['_id'], r) log.info('Project %s (%s) has %d nodes with attachments', proj_url, proj_id, node_count) - if not go: - log.info('Would update %d nodes', total_nodes) + log.info('%s %d nodes', 'Updated' if go else 'Would update', total_nodes) + if failed_node_ids: + log.warning('Failed to update %d of %d nodes: %s', len(failed_node_ids), total_nodes, + ', '.join(str(nid) for nid in failed_node_ids)) def _db_projects(proj_url: str, all_projects: bool, project_id='', *, go: bool) \ diff --git a/tests/test_cli/test_maintenance.py b/tests/test_cli/test_maintenance.py index b0319b74..0e40c78c 100644 --- a/tests/test_cli/test_maintenance.py +++ b/tests/test_cli/test_maintenance.py @@ -134,13 +134,13 @@ class UpgradeAttachmentUsageTest(AbstractPillarTest): 'picture': self.fid, 'project': self.pid, 'user': self.uid, - 'description': "# Title\n\n@[slug0]\n@[slug1]\n@[slug2]\nEitje van Fabergé.", + 'description': "# Title\n\n@[slug 0]\n@[slug1]\n@[slug2]\nEitje van Fabergé.", 'properties': { 'status': 'published', 'content_type': 'image', 'file': self.fid, 'attachments': { - 'slug0': { + 'slug 0': { 'oid': self.fid, 'link': 'self', }, @@ -165,7 +165,7 @@ class UpgradeAttachmentUsageTest(AbstractPillarTest): self.assertEqual( "# Title\n\n" - "{attachment 'slug0' link='self'}\n" + "{attachment 'slug-0' link='self'}\n" "{attachment 'slug1' link='https://cloud.blender.org/'}\n" "{attachment 'slug2'}\n" "Eitje van Fabergé.", @@ -173,7 +173,7 @@ class UpgradeAttachmentUsageTest(AbstractPillarTest): 'The description should be updated') self.assertEqual( "
Eitje van Fabergé.
\n", @@ -181,7 +181,7 @@ class UpgradeAttachmentUsageTest(AbstractPillarTest): 'The _description_html should be updated') self.assertEqual( - {'slug0': {'oid': self.fid}, + {'slug-0': {'oid': self.fid}, 'slug1': {'oid': self.fid}, 'slug2': {'oid': self.fid}, },