[albatross-users] [patch] Another attempt at "fixing" base_url

[Dave Cole]

> Attached is another patch, against stock 1.10pre2, which attempts to
> make the base_url + httpdapp combination work without breaking
> everything else ;-).

The pain that goes into a few lines of code.  I am sorry that you have
suffered so much.  It really highlights a large shortcoming in the
current code - comments.

> This time, with the knowledge that a relative base_url is perfectly
> valid, I have taken the approach of "fixing up" the base_url where
> it is used to determine the page and build redirect urls. I hope
> this approach is a little more successful than the previous one!

I have been looking at the patch and think it can be simplified.  See
comments below.

> diff -ru albatross-1.10pre2/albatross/app.py albatross-1.10pre2-matt/albatross/app.py
> --- albatross-1.10pre2/albatross/app.py	2003-07-05 08:52:40.000000000 +0100
> +++ albatross-1.10pre2-matt/albatross/app.py	2003-07-09 15:11:13.000000000 +0100
> @@ -198,6 +198,9 @@
>      def redirect_url(self, loc):
>          this_path = urlparse.urlparse(self.request.get_uri())[2]
>          base_path = urlparse.urlparse(self.app.base_url())[2]
> +        # If no base_path then the application is mounted at the root
> +        if not base_path:
> +            base_path = '/'
>          pos = this_path.find(base_path)
>          new_base = this_path[:pos + len(base_path)]
>          if new_base[-1] != '/':
> @@ -221,11 +224,7 @@
>      def __init__(self, base_url):
>          ResourceMixin.__init__(self)
>          self.register_tagclasses(*tags.tags)
> -        # Use a base_url of '/' if not provided
> -        if base_url:
> -            self.__base_url = base_url
> -        else:
> -            self.__base_url = '/'
> +        self.__base_url = base_url
>  
>      def run(self, req):
>          '''Process a single browser request

If you leave the base_url code untouched in the Application
constructor then there is no need for the change in redirect_url().
This means that we can leave this file unpatched.

The effect of this non-change also changes the patch to randompage.py

> diff -ru albatross-1.10pre2/albatross/randompage.py albatross-1.10pre2-matt/albatross/randompage.py
> --- albatross-1.10pre2/albatross/randompage.py	2003-07-05 08:52:40.000000000 +0100
> +++ albatross-1.10pre2-matt/albatross/randompage.py	2003-07-09 13:14:32.000000000 +0100
> @@ -23,6 +23,12 @@
>          try:
>              base_path = urlparse.urlparse(self.base_url())[2]
>              uri_path = urlparse.urlparse(uri)[2]
> +            # Fixup base_path to end with a '/' so that it can be used as
> +            # a root path.
> +            if not base_path:
> +                base_path = '/'
> +            elif base_path[-1] != '/':
> +                base_path =  base_path + '/'
>              page = uri_path.split(base_path, 1)[1]
>          except IndexError:
>              pass

The first check is no longer necessary if we do not change base_url
logic in the Application constructor.  This reduces the patch to this:

         try:
             base_path = urlparse.urlparse(self.base_url())[2]
             uri_path = urlparse.urlparse(uri)[2]
+            # Fixup base_path to end with a '/' so that it can be used as
+            # a root path.
+            if not base_path.endswith('/'):
+                base_path =  base_path + '/'
             page = uri_path.split(base_path, 1)[1]
         except IndexError:
             pass

Can you see any reason why this is not equivalent to your patch?  Am I
missing something else?

- Dave

-- 
http://www.object-craft.com.au