From 2e9a846cde8724739eb00faadf16ea5a5abcb927 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Thu, 2 Oct 2014 22:01:18 -0400 Subject: [PATCH 1/6] Access website obj directly under resources.get I'm trying to get rid of request.website, so that instead we just pass in the website object where ever we need it. In simplate contexts, website is available as a global. It's unpythonic ("one right way") and overcomplicated to have it also available at request.website. --- aspen/algorithms/website.py | 6 +++--- aspen/resources/__init__.py | 13 +++++++------ tests/test_negotiated_resource.py | 30 +++++++++++++++--------------- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/aspen/algorithms/website.py b/aspen/algorithms/website.py index 17da57958..13f2f599e 100644 --- a/aspen/algorithms/website.py +++ b/aspen/algorithms/website.py @@ -73,8 +73,8 @@ def apply_typecasters_to_path(website, request): typecasting.apply_typecasters(website.typecasters, request.line.uri.path) -def get_resource_for_request(request): - return {'resource': resources.get(request)} +def get_resource_for_request(website, request): + return {'resource': resources.get(website, request)} def get_response_for_resource(request, resource=None): @@ -116,7 +116,7 @@ def delegate_error_to_simplate(website, request, response, resource=None): if resource is not None: # Try to return an error that matches the type of the original resource. request.headers['Accept'] = resource.media_type + ', text/plain; q=0.1' - resource = resources.get(request) + resource = resources.get(website, request) try: response = resource.respond(request, response) except Response as response: diff --git a/aspen/resources/__init__.py b/aspen/resources/__init__.py index 7f7e2b16b..4be3aecad 100644 --- a/aspen/resources/__init__.py +++ b/aspen/resources/__init__.py @@ -113,7 +113,7 @@ def get_declaration(line): # Core loaders # ============ -def load(request, mtime): +def load(website, request, mtime): """Given a Request and a mtime, return a Resource object (w/o caching). """ @@ -139,7 +139,7 @@ def load(request, mtime): guess_with = guess_with[:-4] fs_media_type = mimetypes.guess_type(guess_with, strict=False)[0] if fs_media_type is None: - media_type = request.website.media_type_default + media_type = website.media_type_default else: media_type = fs_media_type @@ -154,14 +154,15 @@ def load(request, mtime): else: # negotiated Class = NegotiatedResource - resource = Class(request.website, request.fs, raw, media_type, mtime) + resource = Class(website, request.fs, raw, media_type, mtime) return resource -def get(request): +def get(website, request): """Given a Request, return a Resource object (with caching). - We need the request because it carries media_type_default. + We need the request to pass through to the Resource constructor. That's + where it's placed into the simplate execution context. """ @@ -188,7 +189,7 @@ def get(request): raise entry.exc else: # cache miss try: - entry.resource = load(request, mtime) + entry.resource = load(website, request, mtime) except: # capture any Exception entry.exc = (LoadError(traceback.format_exc()) , sys.exc_info()[2] diff --git a/tests/test_negotiated_resource.py b/tests/test_negotiated_resource.py index f87912683..88c9e742e 100644 --- a/tests/test_negotiated_resource.py +++ b/tests/test_negotiated_resource.py @@ -110,11 +110,11 @@ def test_get_renderer_factory_can_raise_syntax_error(get): # get_response -def get_response(request, response): +def get_response(website, request, response): context = { 'request': request , 'response': response } - resource = resources.load(request, 0) + resource = resources.load(website, request, 0) return resource.get_response(context) NEGOTIATED_RESOURCE = """\ @@ -129,19 +129,19 @@ def test_get_response_gets_response(harness): harness.fs.www.mk(('index.spt', NEGOTIATED_RESOURCE)) response = Response() request = harness.make_request(filepath='index.spt', contents=NEGOTIATED_RESOURCE) - actual = get_response(request, response) + actual = get_response(harness.client.website, request, response) assert actual is response def test_get_response_is_happy_not_to_negotiate(harness): harness.fs.www.mk(('index.spt', NEGOTIATED_RESOURCE)) request = harness.make_request(filepath='index.spt', contents=NEGOTIATED_RESOURCE) - actual = get_response(request, Response()).body + actual = get_response(harness.client.website, request, Response()).body assert actual == "Greetings, program!\n" def test_get_response_sets_content_type_when_it_doesnt_negotiate(harness): harness.fs.www.mk(('index.spt', NEGOTIATED_RESOURCE)) request = harness.make_request(filepath='index.spt', contents=NEGOTIATED_RESOURCE) - actual = get_response(request, Response()).headers['Content-Type'] + actual = get_response(harness.client.website, request, Response()).headers['Content-Type'] assert actual == "text/plain; charset=UTF-8" def test_get_response_doesnt_reset_content_type_when_not_negotiating(harness): @@ -149,14 +149,14 @@ def test_get_response_doesnt_reset_content_type_when_not_negotiating(harness): request = harness.make_request(filepath='index.spt', contents=NEGOTIATED_RESOURCE) response = Response() response.headers['Content-Type'] = 'never/mind' - actual = get_response(request, response).headers['Content-Type'] + actual = get_response(harness.client.website, request, response).headers['Content-Type'] assert actual == "never/mind" def test_get_response_negotiates(harness): harness.fs.www.mk(('index.spt', NEGOTIATED_RESOURCE)) request = harness.make_request(filepath='index.spt', contents=NEGOTIATED_RESOURCE) request.headers['Accept'] = 'text/html' - actual = get_response(request, Response()).body + actual = get_response(harness.client.website, request, Response()).body assert actual == "

Greetings, program!

\n" def test_handles_busted_accept(harness): @@ -164,14 +164,14 @@ def test_handles_busted_accept(harness): request = harness.make_request(filepath='index.spt', contents=NEGOTIATED_RESOURCE) # Set an invalid Accept header so it will return default (text/plain) request.headers['Accept'] = 'text/html;' - actual = get_response(request, Response()).body + actual = get_response(harness.client.website, request, Response()).body assert actual == "Greetings, program!\n" def test_get_response_sets_content_type_when_it_negotiates(harness): harness.fs.www.mk(('index.spt', NEGOTIATED_RESOURCE)) request = harness.make_request(filepath='index.spt', contents=NEGOTIATED_RESOURCE) request.headers['Accept'] = 'text/html' - actual = get_response(request, Response()).headers['Content-Type'] + actual = get_response(harness.client.website, request, Response()).headers['Content-Type'] assert actual == "text/html; charset=UTF-8" def test_get_response_doesnt_reset_content_type_when_negotiating(harness): @@ -180,24 +180,24 @@ def test_get_response_doesnt_reset_content_type_when_negotiating(harness): request.headers['Accept'] = 'text/html' response = Response() response.headers['Content-Type'] = 'never/mind' - actual = get_response(request, response).headers['Content-Type'] + actual = get_response(harness.client.website, request, response).headers['Content-Type'] response = Response() response.headers['Content-Type'] = 'never/mind' - actual = get_response(request, response).headers['Content-Type'] + actual = get_response(harness.client.website, request, response).headers['Content-Type'] assert actual == "never/mind" def test_get_response_raises_406_if_need_be(harness): harness.fs.www.mk(('index.spt', NEGOTIATED_RESOURCE)) request = harness.make_request(filepath='index.spt', contents=NEGOTIATED_RESOURCE) request.headers['Accept'] = 'cheese/head' - actual = raises(Response, get_response, request, Response()).value.code + actual = raises(Response, get_response, harness.client.website, request, Response()).value.code assert actual == 406 def test_get_response_406_gives_list_of_acceptable_types(harness): harness.fs.www.mk(('index.spt', NEGOTIATED_RESOURCE)) request = harness.make_request(filepath='index.spt', contents=NEGOTIATED_RESOURCE) request.headers['Accept'] = 'cheese/head' - actual = raises(Response, get_response, request, Response()).value.body + actual = raises(Response, get_response, harness.client.website, request, Response()).value.body expected = "The following media types are available: text/plain, text/html." assert actual == expected @@ -223,14 +223,14 @@ def test_can_override_default_renderers_by_mimetype(harness): harness.fs.www.mk(('index.spt', NEGOTIATED_RESOURCE),) request = harness.make_request(filepath='index.spt', contents=NEGOTIATED_RESOURCE) request.headers['Accept'] = 'text/plain' - actual = get_response(request, Response()).body + actual = get_response(harness.client.website, request, Response()).body assert actual == "glubber" def test_can_override_default_renderer_entirely(harness): harness.fs.project.mk(('configure-aspen.py', OVERRIDE_SIMPLATE)) request = harness.make_request(filepath='index.spt', contents=NEGOTIATED_RESOURCE) request.headers['Accept'] = 'text/plain' - actual = get_response(request, Response()).body + actual = get_response(harness.client.website, request, Response()).body assert actual == "glubber" From afcbdb5f132171e09b6827d653863b5764f84acc Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Thu, 2 Oct 2014 22:09:39 -0400 Subject: [PATCH 2/6] Remove request.website from our bundled simplates We have website already in simplate contexts. --- aspen/www/autoindex.html.spt | 4 ++-- aspen/www/error.spt | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/aspen/www/autoindex.html.spt b/aspen/www/autoindex.html.spt index d99ee4d2a..232316a81 100644 --- a/aspen/www/autoindex.html.spt +++ b/aspen/www/autoindex.html.spt @@ -56,7 +56,7 @@ def _get_time(stats): fspath = request.headers.get('X-Aspen-AutoIndexDir', os.path.dirname(__file__)) assert os.path.isdir(fspath) # sanity check -urlpath = fspath[len(request.website.www_root):] + os.sep +urlpath = fspath[len(website.www_root):] + os.sep urlpath = '/'.join(urlpath.split(os.sep)) title = urlpath and urlpath or '/' @@ -90,7 +90,7 @@ dirs.sort() files.sort() others.sort() -updir = "" if fspath == request.website.www_root else """ +updir = "" if fspath == website.www_root else """ ../   diff --git a/aspen/www/error.spt b/aspen/www/error.spt index b0bc90cc1..f9507bc5e 100644 --- a/aspen/www/error.spt +++ b/aspen/www/error.spt @@ -23,7 +23,7 @@ except ImportError: style = '' msg = status_strings.get(response.code, 'Sorry') msg = msg[0].upper() + msg[1:].lower() -if request.website.show_tracebacks: +if website.show_tracebacks: err_ascii = response.body if pygmentize: # Pygments does HTML escaping From f133e06a9973ac6dc0e49e0b395e3d0e8af9e20c Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Thu, 2 Oct 2014 22:10:13 -0400 Subject: [PATCH 3/6] Remove request.website from aspen.io --- doc/.aspen/aspen_io.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/.aspen/aspen_io.py b/doc/.aspen/aspen_io.py index fa769e140..0c41de9aa 100644 --- a/doc/.aspen/aspen_io.py +++ b/doc/.aspen/aspen_io.py @@ -6,14 +6,14 @@ opts = {} # populate this in configure-aspen.py -def add_stuff_to_request_context(request): +def add_stuff_to_request_context(website, request): # Define some closures for generating image markup. # ================================================= def translate(src): if src[0] != '/': - rel = dirname(request.fs)[len(request.website.www_root):] + rel = dirname(request.fs)[len(website.www_root):] src = '/'.join([rel, src]) src = opts['base'] + src return src From febd9305a7f2c69d630ba9a67fadcbac26d2e1d1 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Thu, 2 Oct 2014 22:14:03 -0400 Subject: [PATCH 4/6] Remove request.website from dispatcher Pass in website object instead. --- aspen/algorithms/website.py | 4 ++-- aspen/dispatcher.py | 24 ++++++++++++------------ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/aspen/algorithms/website.py b/aspen/algorithms/website.py index 13f2f599e..4e442352c 100644 --- a/aspen/algorithms/website.py +++ b/aspen/algorithms/website.py @@ -65,8 +65,8 @@ def raise_200_for_OPTIONS(request): raise Response(200) -def dispatch_request_to_filesystem(request): - dispatcher.dispatch(request) +def dispatch_request_to_filesystem(website, request): + dispatcher.dispatch(website, request) def apply_typecasters_to_path(website, request): diff --git a/aspen/dispatcher.py b/aspen/dispatcher.py index ea547a23d..e57f89130 100644 --- a/aspen/dispatcher.py +++ b/aspen/dispatcher.py @@ -219,15 +219,15 @@ def is_first_index(indices, basedir, name): return False -def update_neg_type(request, filename): +def update_neg_type(website, request, filename): media_type = mimetypes.guess_type(filename, strict=False)[0] if media_type is None: - media_type = request.website.media_type_default + media_type = website.media_type_default request.headers['X-Aspen-Accept'] = media_type debug(lambda: "set x-aspen-accept to %r" % media_type) -def dispatch(request, pure_dispatch=False): +def dispatch(website, request, pure_dispatch=False): """Concretize dispatch_abstract. This is all side-effecty on the request object, setting, at the least, @@ -245,9 +245,9 @@ def dispatch(request, pure_dispatch=False): listnodes = os.listdir is_leaf = os.path.isfile traverse = os.path.join - find_index = lambda x: match_index(request.website.indices, x) - noext_matched = lambda x: update_neg_type(request, x) - startdir = request.website.www_root + find_index = lambda x: match_index(website.indices, x) + noext_matched = lambda x: update_neg_type(website, request, x) + startdir = website.www_root # Dispatch! # ========= @@ -266,10 +266,10 @@ def dispatch(request, pure_dispatch=False): if result.match: debug(lambda: "result.match is true" ) matchbase, matchname = result.match.rsplit(os.path.sep,1) - if pathparts[-1] != '' and matchname in request.website.indices and \ - is_first_index(request.website.indices, matchbase, matchname): + if pathparts[-1] != '' and matchname in website.indices and \ + is_first_index(website.indices, matchbase, matchname): # asked for something that maps to a default index file; redirect to / per issue #175 - debug(lambda: "found default index '%s' maps into %r" % (pathparts[-1], request.website.indices)) + debug(lambda: "found default index '%s' maps into %r" % (pathparts[-1], website.indices)) uri = request.line.uri location = uri.path.raw[:-len(pathparts[-1])] if uri.querystring.raw: @@ -285,7 +285,7 @@ def dispatch(request, pure_dispatch=False): if request.line.uri.path.raw == '/favicon.ico': if result.status != DispatchStatus.okay: path = request.line.uri.path.raw[1:] - request.fs = request.website.find_ours(path) + request.fs = website.find_ours(path) return @@ -305,9 +305,9 @@ def dispatch(request, pure_dispatch=False): if result.status == DispatchStatus.okay: if result.match.endswith('/'): # autoindex - if not request.website.list_directories: + if not website.list_directories: raise Response(404) - autoindex = request.website.ours_or_theirs('autoindex.html.spt') + autoindex = website.ours_or_theirs('autoindex.html.spt') assert autoindex is not None # sanity check request.headers['X-Aspen-AutoIndexDir'] = result.match request.fs = autoindex From db40393bc063ab6941c2ff58f7f6e59c8879c081 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Thu, 2 Oct 2014 22:14:26 -0400 Subject: [PATCH 5/6] Remove request.website entirely --- aspen/algorithms/website.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/aspen/algorithms/website.py b/aspen/algorithms/website.py index 4e442352c..9622e4d0d 100644 --- a/aspen/algorithms/website.py +++ b/aspen/algorithms/website.py @@ -54,11 +54,6 @@ def parse_body_into_request(request, website): ) -def tack_website_onto_request(request, website): - # XXX Why? - request.website = website - - def raise_200_for_OPTIONS(request): """A hook to return 200 to an 'OPTIONS *' request""" if request.line.method == "OPTIONS" and request.line.uri == "*": From 9738437bf56d6036b2da6936a40c56fc07ed3843 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Thu, 2 Oct 2014 23:00:22 -0400 Subject: [PATCH 6/6] Format dispatcher for 100 chars --- aspen/dispatcher.py | 53 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 42 insertions(+), 11 deletions(-) diff --git a/aspen/dispatcher.py b/aspen/dispatcher.py index e57f89130..c284d87a8 100644 --- a/aspen/dispatcher.py +++ b/aspen/dispatcher.py @@ -107,10 +107,15 @@ def get_wildleaf_fallback(): # check all the possibilities: # node.html, node.html.spt, node.spt, node.html/, %node.html/ %*.html.spt, %*.spt - subnodes = set([ n for n in listnodes(curnode) if not n.startswith('.') ]) # don't serve hidden files + + # don't serve hidden files + subnodes = set([ n for n in listnodes(curnode) if not n.startswith('.') ]) + node_noext, node_ext = splitext(node) - maybe_wild_nodes = [ n for n in sorted(subnodes) if n.startswith("%") ] # only maybe because non-spt files aren't wild + # only maybe because non-spt files aren't wild + maybe_wild_nodes = [ n for n in sorted(subnodes) if n.startswith("%") ] + wild_leaf_ns = [ n for n in maybe_wild_nodes if is_leaf_node(n) and is_spt(n) ] wild_nonleaf_ns = [ n for n in maybe_wild_nodes if not is_leaf_node(n) ] @@ -132,7 +137,8 @@ def get_wildleaf_fallback(): if node == '': # dir request debug(lambda: "...last node is empty") path_so_far = traverse(curnode, node) - # return either an index file or have the path end in '/' which means 404 or autoindex as appropriate + # return either an index file or have the path end in '/' which means 404 or + # autoindex as appropriate found_n = find_index(path_so_far) if found_n is None: found_n = "" @@ -145,13 +151,19 @@ def get_wildleaf_fallback(): elif node in subnodes and is_leaf_node(node): debug(lambda: "...found exact file, must be static") if is_spt(node): - return DispatchResult(DispatchStatus.missing, None, None, "Node %r Not Found" % node) + return DispatchResult( DispatchStatus.missing + , None + , None + , "Node %r Not Found" % node + ) else: found_n = node elif node + ".spt" in subnodes and is_leaf_node(node + ".spt"): debug(lambda: "...found exact spt") found_n = node + ".spt" - elif node_noext + ".spt" in subnodes and is_leaf_node(node_noext + ".spt") and node_ext: # node has an extension + elif node_noext + ".spt" in subnodes and is_leaf_node(node_noext + ".spt") \ + and node_ext: + # node has an extension debug(lambda: "...found indirect spt") # indirect match noext_matched(node) @@ -166,16 +178,28 @@ def get_wildleaf_fallback(): curnode = traverse(curnode, found_n) result = get_wildleaf_fallback() if not result: - return DispatchResult(DispatchStatus.non_leaf, curnode, None, "Tried to access non-leaf node as leaf.") + return DispatchResult( DispatchStatus.non_leaf + , curnode + , None + , "Tried to access non-leaf node as leaf." + ) return result elif node in subnodes: debug(lambda: "exact dirmatch") - return DispatchResult(DispatchStatus.non_leaf, curnode, None, "Tried to access non-leaf node as leaf.") + return DispatchResult( DispatchStatus.non_leaf + , curnode + , None + , "Tried to access non-leaf node as leaf." + ) else: debug(lambda: "fallthrough") result = get_wildleaf_fallback() if not result: - return DispatchResult(DispatchStatus.missing, None, None, "Node %r Not Found" % node) + return DispatchResult( DispatchStatus.missing + , None + , None + , "Node %r Not Found" % node + ) return result if not last_node: # not at last path seg in request @@ -185,7 +209,8 @@ def get_wildleaf_fallback(): debug(lambda: "Exact match " + repr(node)) curnode = traverse(curnode, found_n) elif wild_nonleaf_ns: - # need to match a wildnode, and we're not the last node, so we should match non-leaf first, then leaf + # need to match a wildnode, and we're not the last node, so we should match + # non-leaf first, then leaf found_n = wild_nonleaf_ns[0] wildvals[found_n[1:]] = node debug(lambda: "Wildcard match %r = %r " % (found_n, node)) @@ -194,7 +219,11 @@ def get_wildleaf_fallback(): debug(lambda: "No exact match for " + repr(node)) result = get_wildleaf_fallback() if not result: - return DispatchResult(DispatchStatus.missing, None, None, "Node %r Not Found" % node) + return DispatchResult( DispatchStatus.missing + , None + , None + , "Node %r Not Found" % node + ) return result return DispatchResult(DispatchStatus.okay, curnode, wildvals, "Found.") @@ -269,7 +298,9 @@ def dispatch(website, request, pure_dispatch=False): if pathparts[-1] != '' and matchname in website.indices and \ is_first_index(website.indices, matchbase, matchname): # asked for something that maps to a default index file; redirect to / per issue #175 - debug(lambda: "found default index '%s' maps into %r" % (pathparts[-1], website.indices)) + debug( lambda: "found default index '%s' maps into %r" + % (pathparts[-1], website.indices) + ) uri = request.line.uri location = uri.path.raw[:-len(pathparts[-1])] if uri.querystring.raw: