archive/pillar
0
0
Fork 0
Code Issues 42 Pull Requests Projects Releases Wiki Activity

Disallow spaces in attachment slugs

Browse Source 
Slugs shouldn't have spaces. It also interferes with using slugs in
shortcodes.
This commit is contained in:
Sybren A. Stüvel 2018-04-03 13:59:18 +02:00
parent cbb5d546ef
commit 67e8e7c082
3 changed files with 32 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
pillar/api/node_types/__init__.py
View File

@ -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 = { attachments_embedded_schema = {
'type': 'dict', 'type': 'dict',

34
pillar/cli/maintenance.py
View File

@ -794,7 +794,6 @@ def upgrade_attachment_usage(proj_url=None, all_projects=False, go=False):
return 1 return 1
import html import html
from pillar.api.node_types import ATTACHMENT_SLUG_REGEX
from pillar.api.projects.utils import node_type_dict from pillar.api.projects.utils import node_type_dict
from pillar.api.utils import remove_private_keys from pillar.api.utils import remove_private_keys
from pillar.api.utils.authentication import force_cli_user 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') nodes_coll = current_app.db('nodes')
total_nodes = 0 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): for proj in _db_projects(proj_url, all_projects, go=go):
proj_id = proj['_id'] proj_id = proj['_id']
proj_url = proj.get('url', '-no-url-') 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: if node_count == 0:
log.debug('Skipping project %s (%s)', proj_url, proj_id) log.debug('Skipping project %s (%s)', proj_url, proj_id)
continue continue
total_nodes += node_count
proj_node_types = node_type_dict(proj) proj_node_types = node_type_dict(proj)
for node in nodes: for node in nodes:
attachments = node['properties']['attachments'] attachments = node['properties']['attachments']
replaced = False
# Inner functions because of access to the node's attachments. # Inner functions because of access to the node's attachments.
def replace(match): def replace(match):
nonlocal replaced
slug = match.group(1) slug = match.group(1)
log.debug(' - OLD STYLE attachment slug %r', slug) log.debug(' - OLD STYLE attachment slug %r', slug)
try: try:
@ -840,7 +844,8 @@ def upgrade_attachment_usage(proj_url=None, all_projects=False, go=False):
url = att.get('link_custom') url = att.get('link_custom')
if url: if url:
link = " link='%s'" % html.escape(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: def update_markdown(value: str) -> str:
return old_slug_re.sub(replace, value) 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) iter_markdown(proj_node_types, node, update_markdown)
# Remove no longer used properties from attachments # 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', None)
attachment.pop('link_custom', 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: if go:
# Use Eve to PUT, so we have schema checking. # 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']) r, _, _, status = current_app.put_internal('nodes', db_node, _id=node['_id'])
if status != 200: if status != 200:
log.error('Error %i storing altered node %s %s', status, node['_id'], r) 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.debug('Updated node %s: %s', node['_id'], r)
log.info('Project %s (%s) has %d nodes with attachments', log.info('Project %s (%s) has %d nodes with attachments',
proj_url, proj_id, node_count) proj_url, proj_id, node_count)
if not go: log.info('%s %d nodes', 'Updated' if go else 'Would update', total_nodes)
log.info('Would update %d nodes', 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) \ def _db_projects(proj_url: str, all_projects: bool, project_id='', *, go: bool) \

10
tests/test_cli/test_maintenance.py
View File

@ -134,13 +134,13 @@ class UpgradeAttachmentUsageTest(AbstractPillarTest):
'picture': self.fid, 'picture': self.fid,
'project': self.pid, 'project': self.pid,
'user': self.uid, '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': { 'properties': {
'status': 'published', 'status': 'published',
'content_type': 'image', 'content_type': 'image',
'file': self.fid, 'file': self.fid,
'attachments': { 'attachments': {
'slug0': { 'slug 0': {
'oid': self.fid, 'oid': self.fid,
'link': 'self', 'link': 'self',
}, },
@ -165,7 +165,7 @@ class UpgradeAttachmentUsageTest(AbstractPillarTest):
self.assertEqual( self.assertEqual(
"# Title\n\n" "# Title\n\n"
"{attachment 'slug0' link='self'}\n" "{attachment 'slug-0' link='self'}\n"
"{attachment 'slug1' link='https://cloud.blender.org/'}\n" "{attachment 'slug1' link='https://cloud.blender.org/'}\n"
"{attachment 'slug2'}\n" "{attachment 'slug2'}\n"
"Eitje van Fabergé.", "Eitje van Fabergé.",
@ -173,7 +173,7 @@ class UpgradeAttachmentUsageTest(AbstractPillarTest):
'The description should be updated') 'The description should be updated')
self.assertEqual( self.assertEqual(
"<h1>Title</h1>\n" "<h1>Title</h1>\n"
"<!-- {attachment 'slug0' link='self'} -->\n" "<!-- {attachment 'slug-0' link='self'} -->\n"
"<!-- {attachment 'slug1' link='https://cloud.blender.org/'} -->\n" "<!-- {attachment 'slug1' link='https://cloud.blender.org/'} -->\n"
"<!-- {attachment 'slug2'} -->\n" "<!-- {attachment 'slug2'} -->\n"
"<p>Eitje van Fabergé.</p>\n", "<p>Eitje van Fabergé.</p>\n",
@ -181,7 +181,7 @@ class UpgradeAttachmentUsageTest(AbstractPillarTest):
'The _description_html should be updated') 'The _description_html should be updated')
self.assertEqual( self.assertEqual(
{'slug0': {'oid': self.fid}, {'slug-0': {'oid': self.fid},
'slug1': {'oid': self.fid}, 'slug1': {'oid': self.fid},
'slug2': {'oid': self.fid}, 'slug2': {'oid': self.fid},
}, },