Monkey-patching is Bad

Andy Byers
2 min readSep 6, 2017

Monkey-patching software is generally frowned upon, but there is a time and a place for everything, even monkey-patching.

Take Janeway for example, we were working nicely until we had a feature request from a developer who was thinking about using it: Could we handle multi-tenant journals without using multiple domains? It made sense, not every organisation who would be interested in using this software would have the in house capacity to manage domains and subdomains when they have tens of journals so we worked out how it could be done.

Currently we were using subdomains like:
ctp.localhost and we needed to move to a path based identifier eg. localhost/ctp/ so instead of using the domain itself as the identifier we needed to use the first path element, this required an updated of our SiteSettingsMiddleware:

def set_journal(request, site):
from journal import models as journal_models
if settings.URL_CONFIG == 'path':
journal_code = request.path.split('/')[1]
request.journal = journal_models.Journal.objects.get(code=journal_code)
else:
request.journal = journal_models.Journal.objects.get(domain=site.domain)

Basically, we now needed to insert the journal’s code into the URL structure and have it be passed around through reverse functions and {% url %} tags. This would require us to make changes to Django’s core. For the URL tag this is quite easy as we can grab the Request object from Context:

class URLNode(Node):
def __init__(self, view_name, args, kwargs, asvar):
self.view_name = view_name
self.args = args
self.kwargs = kwargs
self.asvar = asvar

def render(self, context):
from django.urls import reverse, NoReverseMatch
args = [arg.resolve(context) for arg in self.args]
kwargs = {
force_text(k, 'ascii'): v.resolve(context)
for k, v in self.kwargs.items()
}

if settings.URL_CONFIG == 'path':
request = context.get('request', None)
if request:
kwargs['journal_code'] = request.journal.code if request.journal else 'press'

view_name = self.view_name.resolve(context)
try:
current_app = context.request.current_app
except AttributeError:
try:
current_app = context.request.resolver_match.namespace
except AttributeError:
current_app = None
# Try to look up the URL. If it fails, raise NoReverseMatch unless the
# {% url ... as var %} construct is used, in which case return nothing.
url = ''
try:
url = reverse(view_name, args=args, kwargs=kwargs, current_app=current_app)
except NoReverseMatch:
if self.asvar is None:
raise

if self.asvar:
context[self.asvar] = url
return ''
else:
if context.autoescape:
url = conditional_escape(url)
return url

It’s not ridiculously difficult for the reverse function either:

def reverse(viewname, urlconf=None, args=None, kwargs=None, current_app=None):
"""
This monkey patch will add the journal_code to reverse kwargs if the URL_CONFIG setting is set to 'patch'
"""

if not viewname.startswith('djdt'):
local_request = GlobalRequestMiddleware.get_current_request()

if settings.URL_CONFIG == 'path':
code = local_request.journal.code if local_request.journal else 'press'
if kwargs and not args:
kwargs['journal_code'] = code
else:
kwargs = {'journal_code': code}

# Drop kwargs if user is accessing admin site.
if local_request.path.startswith('/admin/'):
kwargs.pop('journal_code')

# Drop kwargs if we have args (most likely from the template
if args:
kwargs = None
args = [code] + args

url = django_reverse(viewname, urlconf, args, kwargs, current_app)

# Ensure any unicode characters in the URL are escaped.
return iri_to_uri(url)

Both of these functions insert the request.journal.code either into args or kwargs as required and then carry on a redirect as expected, this is good as we want these functions to behave as a developer would expect.

--

--