Repository: hasgeek/coaster

Issue:
MarkdownColumn should accept markdown configuration
A markdown column may specifically need to allow sanitized HTML, or be pure Markdown, or something else of a variety of configuration options allowed by the Markdown processor.

1. Class `MarkdownComposite` accepts an `options={}` parameter as the last item
2. Class `MarkdownColumn` also accepts `options={}` (position irrelevant, but last for consistency)
3. `MarkdownColumn` calls `composite` with `functools.partial(MarkdownComposite, options=options)`
4. `MarkdownComposite.__setattr__` calls `markdown(value, **self.options)` (pending change in Markdown processor in #182)

Discussion:

Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index 28341ca..ba6ca16 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -45,6 +45,7 @@
 * Added datasets for limited enumeration in role access, as a stopgap until
   migration to GraphQL
 * Removed ShortUUID in favour of the more stable Base58
+* MarkdownColumn now supports a custom markdown processor and options
 
 0.6.0
 -----
diff --git a/coaster/sqlalchemy/__init__.py b/coaster/sqlalchemy/__init__.py
index 40d3079..9b6a43f 100644
--- a/coaster/sqlalchemy/__init__.py
+++ b/coaster/sqlalchemy/__init__.py
@@ -18,6 +18,7 @@ from .columns import *  # NOQA
 from .comparators import *  # NOQA
 from .functions import *  # NOQA
 from .immutable_annotation import *  # NOQA
+from .markdown import *  # NOQA
 from .mixins import *  # NOQA
 from .registry import *  # NOQA
 from .roles import *  # NOQA
diff --git a/coaster/sqlalchemy/columns.py b/coaster/sqlalchemy/columns.py
index 570aa0a..0873fa1 100644
--- a/coaster/sqlalchemy/columns.py
+++ b/coaster/sqlalchemy/columns.py
@@ -8,28 +8,16 @@ SQLAlchemy column types
 from __future__ import absolute_import
 import six
 
-from sqlalchemy import Column, UnicodeText
-from sqlalchemy.ext.mutable import Mutable, MutableComposite
-from sqlalchemy.orm import composite
+from sqlalchemy import UnicodeText
+from sqlalchemy.ext.mutable import Mutable
 from sqlalchemy.types import TEXT, TypeDecorator, UserDefinedType
 from sqlalchemy_utils.types import URLType as UrlTypeBase
 from sqlalchemy_utils.types import UUIDType
 
-from flask import Markup
-
 from furl import furl
 import simplejson
 
-from ..utils import markdown
-
-__all__ = [
-    'JsonDict',
-    'MarkdownComposite',
-    'MarkdownColumn',
-    'UUIDType',
-    'UrlType',
-    'markdown_column',
-]
+__all__ = ['JsonDict', 'UUIDType', 'UrlType']
 
 
 class JsonType(UserDefinedType):
@@ -124,93 +112,6 @@ class MutableDict(Mutable, dict):
 MutableDict.associate_with(JsonDict)
 
 
-@six.python_2_unicode_compatible
-class MarkdownComposite(MutableComposite):
-    """
-    Represents GitHub-flavoured Markdown text and rendered HTML as a composite column.
-    """
-
-    def __init__(self, text, html=None):
-        if html is None:
-            self.text = text  # This will regenerate HTML
-        else:
-            object.__setattr__(self, 'text', text)
-            object.__setattr__(self, '_html', html)
-
-    # If the text value is set, regenerate HTML, then notify parents of the change
-    def __setattr__(self, key, value):
-        if key == 'text':
-            object.__setattr__(self, '_html', markdown(value))
-        object.__setattr__(self, key, value)
-        self.changed()
-
-    # Return column values for SQLAlchemy to insert into the database
-    def __composite_values__(self):
-        return (self.text, self._html)
-
-    # Return a string representation of the text (see class decorator)
-    def __str__(self):
-        return six.text_type(self.text)
-
-    # Return a HTML representation of the text
-    def __html__(self):
-        return self._html or u''
-
-    # Return a Markup string of the HTML
-    @property
-    def html(self):
-        return Markup(self._html or u'')
-
-    # Compare text value
-    def __eq__(self, other):
-        return (
-            (self.text == other.text)
-            if isinstance(other, MarkdownComposite)
-            else (self.text == other)
-        )
-
-    def __ne__(self, other):
-        return not self.__eq__(other)
-
-    # Return state for pickling
-    def __getstate__(self):
-        return (self.text, self._html)
-
-    # Set state from pickle
-    def __setstate__(self, state):
-        object.__setattr__(self, 'text', state[0])
-        object.__setattr__(self, '_html', state[1])
-        self.changed()
-
-    def __bool__(self):
-        return bool(self.text)
-
-    __nonzero__ = __bool__
-
-    # Allow a composite column to be assigned a string value
-    @classmethod  # NOQA: A003
-    def coerce(cls, key, value):  # NOQA: A003
-        return cls(value)
-
-
-def markdown_column(name, deferred=False, group=None, **kwargs):
-    """
-    Create a composite column that autogenerates HTML from Markdown text,
-    storing data in db columns named with ``_html`` and ``_text`` prefixes.
-    """
-    return composite(
-        MarkdownComposite,
-        Column(name + '_text', UnicodeText, **kwargs),
-        Column(name + '_html', UnicodeText, **kwargs),
-        deferred=deferred,
-        group=group or name,
-    )
-
-
-# Compatibility name
-MarkdownColumn = markdown_column
-
-
 class UrlType(UrlTypeBase):
     """
     Extension of URLType_ from SQLAlchemy-Utils that adds basic validation to
diff --git a/coaster/sqlalchemy/markdown.py b/coaster/sqlalchemy/markdown.py
new file mode 100644
index 0000000..7f089ff
--- /dev/null
+++ b/coaster/sqlalchemy/markdown.py
@@ -0,0 +1,134 @@
+# -*- coding: utf-8 -*-
+from __future__ import absolute_import, unicode_literals
+import six
+
+from sqlalchemy import Column, UnicodeText
+from sqlalchemy.ext.mutable import MutableComposite
+from sqlalchemy.orm import composite
+
+from flask import Markup
+
+from ..utils import markdown as markdown_processor
+
+__all__ = ['MarkdownComposite', 'MarkdownColumn', 'markdown_column']
+
+
+@six.python_2_unicode_compatible
+class MarkdownComposite(MutableComposite):
+    """
+    Represents GitHub-flavoured Markdown text and rendered HTML as a composite column.
+    """
+
+    #: Markdown processor. Subclasses can override this. This has to be a staticmethod
+    #: or the markdown processor will receive `self` as first parameter
+    markdown = staticmethod(markdown_processor)
+    #: Markdown options. Subclasses can override this
+    options = {}
+
+    def __init__(self, text, html=None):
+        if html is None:
+            self.text = text  # This will regenerate HTML
+        else:
+            self._text = text
+            self._html = html
+
+    # Return column values for SQLAlchemy to insert into the database
+    def __composite_values__(self):
+        return (self._text, self._html)
+
+    # Return a string representation of the text (see class decorator)
+    def __str__(self):
+        return six.text_type(self.text)
+
+    # Return a HTML representation of the text
+    def __html__(self):
+        return self._html or ''
+
+    # Return a Markup string of the HTML
+    @property
+    def html(self):
+        return Markup(self._html or '')
+
+    @property
+    def text(self):
+        return self._text
+
+    @text.setter
+    def text(self, value):
+        self._text = value
+        self._html = self.markdown(
+            value, **(self.options() if callable(self.options) else self.options)
+        )
+        self.changed()
+
+    # Compare text value
+    def __eq__(self, other):
+        return isinstance(other, MarkdownComposite) and (
+            self.__composite_values__() == other.__composite_values__()
+        )
+
+    def __ne__(self, other):
+        return not self.__eq__(other)
+
+    # Pickle support methods implemented as per SQLAlchemy documentation, but not
+    # tested here as we don't use them.
+    # https://docs.sqlalchemy.org/en/13/orm/extensions/mutable.html#id1
+
+    def __getstate__(self):  # pragma: no cover
+        # Return state for pickling
+        return (self._text, self._html)
+
+    def __setstate__(self, state):  # pragma: no cover
+        # Set state from pickle
+        self._text, self._html = state
+        self.changed()
+
+    def __bool__(self):
+        return bool(self._text)
+
+    __nonzero__ = __bool__
+
+    # Allow a composite column to be assigned a string value
+    @classmethod  # NOQA: A003
+    def coerce(cls, key, value):  # NOQA: A003
+        return cls(value)
+
+
+def markdown_column(
+    name, deferred=False, group=None, markdown=None, options=None, **kwargs
+):
+    """
+    Create a composite column that autogenerates HTML from Markdown text,
+    storing data in db columns named with ``_html`` and ``_text`` prefixes.
+
+    :param str name: Column name base
+    :param bool deferred: Whether the columns should be deferred by default
+    :param str group: Defer column group
+    :param markdown: Markdown processor function (default: Coaster's implementation)
+    :param options: Additional options for the Markdown processor
+    :param kwargs: Additional column options, passed to SQLAlchemy's column constructor
+    """
+
+    # Construct a custom subclass of MarkdownComposite and set the markdown processor
+    # and processor options on it. We'll pass this class to SQLAlchemy's composite
+    # constructor.
+    class CustomMarkdownComposite(MarkdownComposite):
+        pass
+
+    CustomMarkdownComposite.options = options if options is not None else {}
+    if markdown is not None:
+        CustomMarkdownComposite.markdown = staticmethod(markdown)
+
+    return composite(
+        CustomMarkdownComposite
+        if (markdown is not None or options is not None)
+        else MarkdownComposite,
+        Column(name + '_text', UnicodeText, **kwargs),
+        Column(name + '_html', UnicodeText, **kwargs),
+        deferred=deferred,
+        group=group or name,
+    )
+
+
+# Compatibility name
+MarkdownColumn = markdown_column


Test patch:
diff --git a/tests/test_sqlalchemy_markdowncolumn.py b/tests/test_sqlalchemy_markdowncolumn.py
index 88b871a..c5ac7e5 100644
--- a/tests/test_sqlalchemy_markdowncolumn.py
+++ b/tests/test_sqlalchemy_markdowncolumn.py
@@ -1,6 +1,6 @@
 # -*- coding: utf-8 -*-
 
-from __future__ import absolute_import
+from __future__ import absolute_import, unicode_literals
 
 import unittest
 
@@ -16,6 +16,20 @@ class MarkdownData(BaseMixin, db.Model):
     value = MarkdownColumn('value', nullable=False)
 
 
+class MarkdownHtmlData(BaseMixin, db.Model):
+    __tablename__ = 'md_html_data'
+    value = MarkdownColumn('value', nullable=False, options={'html': True})
+
+
+def fake_markdown(text):
+    return 'fake-markdown'
+
+
+class FakeMarkdownData(BaseMixin, db.Model):
+    __tablename__ = 'fake_md_data'
+    value = MarkdownColumn('value', nullable=False, markdown=fake_markdown)
+
+
 # -- Tests --------------------------------------------------------------------
 
 
@@ -34,7 +48,7 @@ class TestMarkdownColumn(unittest.TestCase):
         self.ctx.pop()
 
     def test_markdown_column(self):
-        text = u"""# this is going to be h1.\n- Now a list. \n- 1\n- 2\n- 3"""
+        text = """# this is going to be h1.\n- Now a list. \n- 1\n- 2\n- 3"""
         data = MarkdownData(value=text)
         self.session.add(data)
         self.session.commit()
@@ -44,9 +58,9 @@ class TestMarkdownColumn(unittest.TestCase):
         assert data.value.__html__() == markdown(text)
 
     def test_does_not_render_on_load(self):
-        text = u"This is the text"
+        text = "This is the text"
         real_html = markdown(text)
-        fake_html = u"This is not the text"
+        fake_html = "This is not the text"
         data = MarkdownData(value=text)
         self.session.add(data)
 
@@ -75,12 +89,35 @@ class TestMarkdownColumn(unittest.TestCase):
         assert data.value.__html__() == real_html
 
     def test_raw_value(self):
-        text = u"This is the text"
+        text = "This is the text"
         data = MarkdownData()
         self.session.add(data)
         data.value = text
         self.session.commit()
 
+    def test_empty_value(self):
+        doc = MarkdownData(value=None)
+        assert not doc.value
+        assert doc.value.text is None
+        assert doc.value.html == ''
+
+    def test_html_customization(self):
+        """Markdown columns may specify custom Markdown processor options."""
+        text = "Allow <b>some</b> HTML"
+        d1 = MarkdownData(value=text)
+        d2 = MarkdownHtmlData(value=text)
+
+        assert d1.value.text == d2.value.text
+        assert d1.value != d2.value
+        assert d1.value.html == '<p>Allow &lt;b&gt;some&lt;/b&gt; HTML</p>'
+        assert d2.value.html == '<p>Allow <b>some</b> HTML</p>'
+
+    def test_custom_markdown_processor(self):
+        """Markdown columns may specify their own markdown processor."""
+        doc = FakeMarkdownData(value="This is some text")
+        assert doc.value.text == "This is some text"
+        assert doc.value.html == 'fake-markdown'
+
 
 class TestMarkdownColumn2(TestMarkdownColumn):
     app = app2