diff --git a/pillar/api/custom_field_validation.py b/pillar/api/custom_field_validation.py index d53e1dfd..e86c7130 100644 --- a/pillar/api/custom_field_validation.py +++ b/pillar/api/custom_field_validation.py @@ -1,10 +1,13 @@ import logging from bson import ObjectId, tz_util -from datetime import datetime, tzinfo +from datetime import datetime +import cerberus.errors from eve.io.mongo import Validator from flask import current_app +import pillar.markdown + log = logging.getLogger(__name__) @@ -152,3 +155,52 @@ class ValidateCustomFields(Validator): if not isinstance(value, (bytes, bytearray)): self._error(field_name, f'wrong value type {type(value)}, expected bytes or bytearray') + + def _validate_coerce(self, coerce, field: str, value): + """Override Cerberus' _validate_coerce method for richer features. + + This now supports named coercion functions (available in Cerberus 1.0+) + and passes the field name to coercion functions as well. + """ + if isinstance(coerce, str): + coerce = getattr(self, f'_normalize_coerce_{coerce}') + + try: + return coerce(field, value) + except (TypeError, ValueError): + self._error(field, cerberus.errors.ERROR_COERCION_FAILED.format(field)) + + def _normalize_coerce_markdown(self, field: str, value): + """Render Markdown from this field into {field}_html. + + The field name MUST NOT end in `_html`. The Markdown is read from this + field and the rendered HTML is written to the field `{field}_html`. + """ + html = pillar.markdown.markdown(value) + field_name = pillar.markdown.cache_field_name(field) + self.current[field_name] = html + return value + + +if __name__ == '__main__': + from pprint import pprint + + v = ValidateCustomFields() + v.schema = { + 'foo': {'type': 'string', 'coerce': 'markdown'}, + 'foo_html': {'type': 'string'}, + 'nested': { + 'type': 'dict', + 'schema': { + 'bar': {'type': 'string', 'coerce': 'markdown'}, + 'bar_html': {'type': 'string'}, + } + } + } + print('Valid :', v.validate({ + 'foo': '# Title\n\nHeyyyy', + 'nested': {'bar': 'bhahaha'}, + })) + print('Document:') + pprint(v.document) + print('Errors :', v.errors) diff --git a/pillar/api/eve_settings.py b/pillar/api/eve_settings.py index 1774d70e..7f534c85 100644 --- a/pillar/api/eve_settings.py +++ b/pillar/api/eve_settings.py @@ -155,7 +155,9 @@ organizations_schema = { 'description': { 'type': 'string', 'maxlength': 256, + 'coerce': 'markdown', }, + '_description_html': {'type': 'string'}, 'website': { 'type': 'string', 'maxlength': 256, @@ -290,7 +292,9 @@ nodes_schema = { }, 'description': { 'type': 'string', + 'coerce': 'markdown', }, + '_description_html': {'type': 'string'}, 'picture': _file_embedded_schema, 'order': { 'type': 'integer', @@ -535,7 +539,9 @@ projects_schema = { }, 'description': { 'type': 'string', + 'coerce': 'markdown', }, + '_description_html': {'type': 'string'}, # Short summary for the project 'summary': { 'type': 'string', diff --git a/pillar/api/node_types/comment.py b/pillar/api/node_types/comment.py index 26a0f974..89866910 100644 --- a/pillar/api/node_types/comment.py +++ b/pillar/api/node_types/comment.py @@ -2,16 +2,14 @@ node_type_comment = { 'name': 'comment', 'description': 'Comments for asset nodes, pages, etc.', 'dyn_schema': { - # The actual comment content (initially Markdown format) + # The actual comment content 'content': { 'type': 'string', 'minlength': 5, 'required': True, + 'coerce': 'markdown', }, - # The converted-to-HTML content. - 'content_html': { - 'type': 'string', - }, + '_content_html': {'type': 'string'}, 'status': { 'type': 'string', 'allowed': [ diff --git a/pillar/api/node_types/post.py b/pillar/api/node_types/post.py index 91e59805..90fef0fa 100644 --- a/pillar/api/node_types/post.py +++ b/pillar/api/node_types/post.py @@ -4,13 +4,14 @@ node_type_post = { 'name': 'post', 'description': 'A blog post, for any project', 'dyn_schema': { - # The blogpost content (Markdown format) 'content': { 'type': 'string', 'minlength': 5, 'maxlength': 90000, - 'required': True + 'required': True, + 'coerce': 'markdown', }, + '_content_html': {'type': 'string'}, 'status': { 'type': 'string', 'allowed': [ diff --git a/pillar/api/nodes/__init__.py b/pillar/api/nodes/__init__.py index 75fc7b31..bc5707d6 100644 --- a/pillar/api/nodes/__init__.py +++ b/pillar/api/nodes/__init__.py @@ -378,30 +378,6 @@ def after_deleting_node(item): index.node_delete.delay(str(item['_id'])) -only_for_comments = only_for_node_type_decorator('comment') - - -@only_for_comments -def convert_markdown(node, original=None): - """Converts comments from Markdown to HTML. - - Always does this on save, even when the original Markdown hasn't changed, - because our Markdown -> HTML conversion rules might have. - """ - - try: - content = node['properties']['content'] - except KeyError: - node['properties']['content_html'] = '' - else: - node['properties']['content_html'] = pillar.markdown.markdown(content) - - -def nodes_convert_markdown(nodes): - for node in nodes: - convert_markdown(node) - - only_for_textures = only_for_node_type_decorator('texture') @@ -433,7 +409,6 @@ def setup_app(app, url_prefix): app.on_fetched_resource_nodes += before_returning_nodes app.on_replace_nodes += before_replacing_node - app.on_replace_nodes += convert_markdown app.on_replace_nodes += texture_sort_files app.on_replace_nodes += deduct_content_type app.on_replace_nodes += node_set_default_picture @@ -442,11 +417,9 @@ def setup_app(app, url_prefix): app.on_insert_nodes += before_inserting_nodes app.on_insert_nodes += nodes_deduct_content_type app.on_insert_nodes += nodes_set_default_picture - app.on_insert_nodes += nodes_convert_markdown app.on_insert_nodes += textures_sort_files app.on_inserted_nodes += after_inserting_nodes - app.on_update_nodes += convert_markdown app.on_update_nodes += texture_sort_files app.on_delete_item_nodes += before_deleting_node diff --git a/pillar/api/nodes/custom/comment.py b/pillar/api/nodes/custom/comment.py index 89da6c95..0795ac20 100644 --- a/pillar/api/nodes/custom/comment.py +++ b/pillar/api/nodes/custom/comment.py @@ -162,7 +162,7 @@ def edit_comment(user_id, node_id, patch): log.info('User %s edited comment %s', user_id, node_id) # Fetch the new content, so the client can show these without querying again. - node = nodes_coll.find_one(node_id, projection={'properties.content_html': 1}) + node = nodes_coll.find_one(node_id, projection={'properties._content_html': 1}) return status, node diff --git a/pillar/markdown.py b/pillar/markdown.py index 8c843f30..2a5e7f74 100644 --- a/pillar/markdown.py +++ b/pillar/markdown.py @@ -47,3 +47,11 @@ def markdown(s): attributes=ALLOWED_ATTRIBUTES, styles=ALLOWED_STYLES) return safe_html + + +def cache_field_name(field_name: str) -> str: + """Return the field name containing the cached HTML. + + See ValidateCustomFields._normalize_coerce_markdown(). + """ + return f'_{field_name}_html' diff --git a/pillar/web/jinja.py b/pillar/web/jinja.py index 57ddc446..f4c74074 100644 --- a/pillar/web/jinja.py +++ b/pillar/web/jinja.py @@ -10,6 +10,7 @@ import flask_login import jinja2.filters import jinja2.utils import werkzeug.exceptions as wz_exceptions +import pillarsdk import pillar.api.utils from pillar.web.utils import pretty_date @@ -95,6 +96,12 @@ def do_pluralize(value, arg='s'): def do_markdown(s: typing.Optional[str]): + """Convert Markdown. + + This filter is not preferred. Use {'coerce': 'markdown'} in the Eve schema + instead, to cache the HTML in the database, and use do_markdowned() to + fetch it. + """ if s is None: return None @@ -106,6 +113,35 @@ def do_markdown(s: typing.Optional[str]): return jinja2.utils.Markup(safe_html) +def do_markdowned(document: typing.Union[dict, pillarsdk.Resource], field_name: str) -> str: + """Fetch pre-converted Markdown or render on the fly. + + Use {'coerce': 'markdown'} in the Eve schema to cache the HTML in the + database and use do_markdowned() to fetch it in a safe way. + + Jinja example: {{ node.properties | markdowned:'content' }} + """ + if isinstance(document, pillarsdk.Resource): + document = document.to_dict() + + if not document: + return '' + + my_log = log.getChild('do_markdowned') + + cache_field_name = pillar.markdown.cache_field_name(field_name) + my_log.debug('Getting %r', cache_field_name) + + cached_html = document.get(cache_field_name) + if cached_html is not None: + my_log.debug('Cached HTML is %r', cached_html[:40]) + return jinja2.utils.Markup(cached_html) + + markdown_src = document.get(field_name) + my_log.debug('No cached HTML, rendering doc[%r]', field_name) + return do_markdown(markdown_src) + + def do_url_for_node(node_id=None, node=None): try: return url_for_node(node_id=node_id, node=node) @@ -156,6 +192,7 @@ def setup_jinja_env(jinja_env, app_config: dict): jinja_env.filters['pluralize'] = do_pluralize jinja_env.filters['gravatar'] = pillar.api.utils.gravatar jinja_env.filters['markdown'] = do_markdown + jinja_env.filters['markdowned'] = do_markdowned jinja_env.filters['yesno'] = do_yesno jinja_env.filters['repr'] = repr jinja_env.filters['urljoin'] = functools.partial(urllib.parse.urljoin, allow_fragments=True) diff --git a/pillar/web/nodes/custom/comments.py b/pillar/web/nodes/custom/comments.py index c39f90b8..7d0993a4 100644 --- a/pillar/web/nodes/custom/comments.py +++ b/pillar/web/nodes/custom/comments.py @@ -80,7 +80,7 @@ def comment_edit(comment_id): return jsonify({ 'status': 'success', 'data': { - 'content_html': result.properties.content_html, + 'content_html': result.properties['_content_html'], }}) diff --git a/requirements.txt b/requirements.txt index 02ce9cc4..33c16809 100644 --- a/requirements.txt +++ b/requirements.txt @@ -36,7 +36,7 @@ zencoder==0.6.5 amqp==2.1.4 billiard==3.5.0.2 Flask-PyMongo==0.4.1 -Cerberus==0.9.2 +-e git+git@github.com:armadillica/cerberus.git@sybren-0.9#egg=Cerberus Events==0.2.2 future==0.15.2 html5lib==0.9999999 diff --git a/src/templates/nodes/custom/asset/file/view_embed.pug b/src/templates/nodes/custom/asset/file/view_embed.pug index 1100ae57..b59178b3 100644 --- a/src/templates/nodes/custom/asset/file/view_embed.pug +++ b/src/templates/nodes/custom/asset/file/view_embed.pug @@ -18,7 +18,7 @@ | {% if node.description %} .node-details-description#node-description - | {{ node.description | markdown }} + | {{ node | markdowned('description') }} | {% endif %} include ../../_node_details diff --git a/src/templates/nodes/custom/asset/image/view_embed.pug b/src/templates/nodes/custom/asset/image/view_embed.pug index ccb6cb53..e805e8d7 100644 --- a/src/templates/nodes/custom/asset/image/view_embed.pug +++ b/src/templates/nodes/custom/asset/image/view_embed.pug @@ -18,7 +18,7 @@ | {% if node.description %} .node-details-description#node-description - | {{ node.description | markdown }} + | {{ node | markdowned('description') }} | {% endif %} include ../../_node_details @@ -46,4 +46,3 @@ script. }); | {% endblock %} - diff --git a/src/templates/nodes/custom/asset/video/view_embed.pug b/src/templates/nodes/custom/asset/video/view_embed.pug index ae1faf90..368b329c 100644 --- a/src/templates/nodes/custom/asset/video/view_embed.pug +++ b/src/templates/nodes/custom/asset/video/view_embed.pug @@ -46,7 +46,7 @@ | {% if node.description %} .node-details-description#node-description - | {{ node.description | markdown }} + | {{ node | markdowned('description') }} | {% endif %} include ../../_node_details diff --git a/src/templates/nodes/custom/blog/_macros.pug b/src/templates/nodes/custom/blog/_macros.pug index e583186d..834e6d0c 100644 --- a/src/templates/nodes/custom/blog/_macros.pug +++ b/src/templates/nodes/custom/blog/_macros.pug @@ -25,7 +25,7 @@ a.blog_index-header(href="{{ node.url }}") | {{ node.name }} .item-content - | {{ node.properties.content | markdown }} + | {{ node.properties | markdowned('content') }} | {% endmacro %} diff --git a/src/templates/nodes/custom/comment/_macros.pug b/src/templates/nodes/custom/comment/_macros.pug index 148b067d..c0dcd77d 100644 --- a/src/templates/nodes/custom/comment/_macros.pug +++ b/src/templates/nodes/custom/comment/_macros.pug @@ -11,7 +11,7 @@ .comment-body p.comment-author {{ comment._user.full_name }} - span {{comment.properties.content_html | safe }} + span {{comment.properties._content_html | safe }} // TODO: Markdown preview when editing diff --git a/src/templates/nodes/custom/group/view_embed.pug b/src/templates/nodes/custom/group/view_embed.pug index a39b62e1..2167293d 100644 --- a/src/templates/nodes/custom/group/view_embed.pug +++ b/src/templates/nodes/custom/group/view_embed.pug @@ -15,7 +15,7 @@ | {% if node.description %} .node-details-description - | {{ node.description | markdown }} + | {{ node | markdowned('description') }} | {% endif %} section.node-children.group diff --git a/src/templates/nodes/custom/group_hdri/view_embed.pug b/src/templates/nodes/custom/group_hdri/view_embed.pug index bf2f4239..a230f5dc 100644 --- a/src/templates/nodes/custom/group_hdri/view_embed.pug +++ b/src/templates/nodes/custom/group_hdri/view_embed.pug @@ -12,7 +12,7 @@ | {% if node.description %} section.node-row .node-details-description - | {{ node.description | markdown }} + | {{ node | markdowned('description') }} | {% endif %} | {% if children %} diff --git a/src/templates/nodes/custom/group_texture/view_embed.pug b/src/templates/nodes/custom/group_texture/view_embed.pug index 01696a28..9254c5a0 100644 --- a/src/templates/nodes/custom/group_texture/view_embed.pug +++ b/src/templates/nodes/custom/group_texture/view_embed.pug @@ -12,7 +12,7 @@ | {% if node.description %} section.node-row .node-details-description - | {{ node.description | markdown }} + | {{ node | markdowned('description') }} | {% endif %} | {% if children %} diff --git a/src/templates/nodes/custom/hdri/view_embed.pug b/src/templates/nodes/custom/hdri/view_embed.pug index 9a6f02ff..1adc9b0c 100644 --- a/src/templates/nodes/custom/hdri/view_embed.pug +++ b/src/templates/nodes/custom/hdri/view_embed.pug @@ -93,7 +93,7 @@ | {% if node.description %} .node-details-description#node-description - | {{ node.description | markdown }} + | {{ node | markdowned('description') }} | {% endif %} | {% if node.properties.license_notes %} diff --git a/src/templates/nodes/custom/page/view_embed.pug b/src/templates/nodes/custom/page/view_embed.pug index 1f11571b..a8059131 100644 --- a/src/templates/nodes/custom/page/view_embed.pug +++ b/src/templates/nodes/custom/page/view_embed.pug @@ -17,7 +17,7 @@ | {% if node.description %} .node-details-description#node-description - | {{ node.description | markdown }} + | {{ node | markdowned('description') }} | {% endif %} .node-details-meta.footer diff --git a/src/templates/projects/view_embed.pug b/src/templates/projects/view_embed.pug index 6ba828f5..fddd8e5d 100644 --- a/src/templates/projects/view_embed.pug +++ b/src/templates/projects/view_embed.pug @@ -39,7 +39,7 @@ | {% if project.description %} .node-details-description - | {{ project.description | markdown }} + | {{ project | markdowned('description') }} | {% endif %} | {# Until we implement urls for pages @@ -84,9 +84,9 @@ a.title(href="{{ url_for_node(node=n) }}") {{ n.name }} p.description(href="{{ url_for_node(node=n) }}") | {% if n.node_type == 'post' %} - | {{ n.properties.content | markdown | striptags | truncate(140, end="... read more") | safe | hide_none }} + | {{ n.properties | markdowned('content') | striptags | truncate(140, end="... read more") | safe | hide_none }} | {% else %} - | {{ n.description | markdown | striptags | truncate(140, end="... read more") | safe | hide_none }} + | {{ n | markdowned('description') | striptags | truncate(140, end="... read more") | safe | hide_none }} | {% endif %} span.details span.what {% if n.properties.content_type %}{{ n.properties.content_type | undertitle }}{% else %}{{ n.node_type | undertitle }}{% endif %} ยท diff --git a/tests/test_api/test_markdown.py b/tests/test_api/test_markdown.py new file mode 100644 index 00000000..aab4225c --- /dev/null +++ b/tests/test_api/test_markdown.py @@ -0,0 +1,53 @@ +import copy + +from pillar.tests import AbstractPillarTest +from pillar.tests import common_test_data as ctd + + +class CoerceMarkdownTest(AbstractPillarTest): + def test_node_description(self): + from pillar.markdown import markdown + pid, uid = self.create_project_with_admin(24 * 'a') + self.create_valid_auth_token(uid, 'token-a') + node = { + 'node_type': 'group', + 'name': 'Test group', + 'description': '# Title\n\nThis is content.', + 'properties': {}, + 'project': pid, + 'user': uid, + } + + created_data = self.post('/api/nodes', json=node, expected_status=201, + auth_token='token-a').json() + node_id = created_data['_id'] + + json_node = self.get(f'/api/nodes/{node_id}', auth_token='token-a').json() + self.assertEqual(markdown(node['description']), json_node['_description_html']) + + def test_project_description(self): + from pillar.markdown import markdown + from pillar.api.utils import remove_private_keys + + uid = self.create_user(24 * 'a', token='token-a') + + # Go through Eve to create the project. + proj = { + **ctd.EXAMPLE_PROJECT, + 'description': '# Title\n\nThis is content.', + 'user': uid, + } + proj.pop('picture_header') + proj.pop('picture_square') + proj.pop('permissions') + + r, _, _, status = self.app.post_internal('projects', remove_private_keys(proj)) + self.assertEqual(201, status, f'failed because {r}') + + pid = r['_id'] + + json_proj = self.get(f'/api/projects/{pid}', auth_token='token-a').json() + json_proj.pop('node_types', None) # just to make it easier to print + import pprint + pprint.pprint(json_proj) + self.assertEqual(markdown(proj['description']), json_proj['_description_html'])