Merge lp:~leonardr/launchpad/multiversion-wadl into lp:launchpad/db-devel

[Leonard Richardson]

[Leonard Richardson (leonardr)]

This branch changes the build process so that it generates a static WADL file for every version of the web service, not just one file. There were a number of parts of this system that I didn't understand and might not be necessary anymore (mainly the fake 'development' WADL file, which only seems to get in the way), but I left them alone rather than make this branch more complex.

[Gary Poster (gary)]

Thank you, Leonard.

review: Approve

[Tim Penhey (thumper)]

review: Approve (release-critical)

[Leonard Richardson (leonardr)]

This needs another review. Two tests failed when I ran the ec2 suite. Both of them were fallout from when I changed the 'api' vhost's root url from "http://api.launchpad.dev/beta/" to "http://api.launchpad.dev/".

The fix to webapp-publication.txt was trivial. The fix to xx-branchmergeproposal.txt was not. canonical_url() had no idea how to incorporate the version of a web service request, and the test wasn't specifying which version of the web service was desired. I had to make canonical_url work when passed a versioned web service request, and then change the test to create a versioned request and pass it into canonical_url.

ec2 test is running again and will hopefully come up roses in the morning.

[Leonard Richardson (leonardr)]

Incremental diff: http://paste.ubuntu.com/386728/

[Leonard Richardson (leonardr)]

OK, one more. My earlier fix to xx-branchmergeproposal broke a whole lot of other tests because it changed the ways all URLs were generated, just for the sake of one test. This update reverts the change to canonical_url() and instead changes xx-branchmergeproposal to use the standard way of generating web service URLs from a browser-initiated request.

The problem here is that the web service URLs generated by a browser-initiated request look like "http://launchpad.dev/api/". Ordinarily this is fine, but xx-branchmergeproposal generates URLs from a browser-initiated request and then passes them into a web service client that expects URLs that look like "http://api.launchpad.dev/". Rather than try to come up with a standard way of doing this, I created a helper method in the test that does the translation.

This branch also upgrades the version of lazr.restful used. I'm not 100% sure that this new version of lazr.restful is necessary, and I'm going to do some preliminary tests with the old version before landing.

Incremental diff: http://pastebin.ubuntu.com/387107/

[Leonard Richardson (leonardr)]

I should mention that I'm still trying to track down a launchpadlib test failure.

[Gary Poster (gary)]

r10428 and r10429 approved. I'd like fix_url to be removed eventually, but I'll not worry about it now.

review: Approve

=== modified file 'Makefile'
--- Makefile	2010-02-19 22:02:36 +0000
+++ Makefile	2010-03-03 00:11:15 +0000
@@ -28,8 +28,10 @@
 
 BZR_VERSION_INFO = bzr-version-info.py
 
-WADL_FILE = lib/canonical/launchpad/apidoc/wadl-$(LPCONFIG).xml
-API_INDEX = lib/canonical/launchpad/apidoc/index.html
+APIDOC_DIR = lib/canonical/launchpad/apidoc
+WADL_TEMPLATE = $(APIDOC_DIR).tmp/wadl-$(LPCONFIG)-%(version)s.xml
+DEVEL_WADL_FILE = $(APIDOC_DIR)/wadl-$(LPCONFIG)-devel.xml
+API_INDEX = $(APIDOC_DIR)/index.html
 
 # DO NOT ALTER : this should just build by default
 default: inplace
@@ -44,12 +46,14 @@
 hosted_branches: $(PY)
 	$(PY) ./utilities/make-dummy-hosted-branches
 
-$(WADL_FILE): $(BZR_VERSION_INFO)
-	LPCONFIG=$(LPCONFIG) $(PY) ./utilities/create-lp-wadl.py > $@.tmp
-	mv $@.tmp $@
+$(DEVEL_WADL_FILE): $(BZR_VERSION_INFO)
+	mkdir $(APIDOC_DIR).tmp
+	LPCONFIG=$(LPCONFIG) $(PY) ./utilities/create-lp-wadl.py "$(WADL_TEMPLATE)"
+	mv $(APIDOC_DIR).tmp/* $(APIDOC_DIR)
+	rmdir $(APIDOC_DIR).tmp
 
-$(API_INDEX): $(WADL_FILE)
-	bin/apiindex $(WADL_FILE) > $@.tmp
+$(API_INDEX): $(DEVEL_WADL_FILE)
+	bin/apiindex $(DEVEL_WADL_FILE) > $@.tmp
 	mv $@.tmp $@
 
 apidoc: compile $(API_INDEX)
@@ -240,7 +244,7 @@
 $(BZR_VERSION_INFO):
 	scripts/update-bzr-version-info.sh
 
-support_files: $(WADL_FILE) $(BZR_VERSION_INFO)
+support_files: $(DEVEL_WADL_FILE) $(BZR_VERSION_INFO)
 
 # Intended for use on developer machines
 start: inplace stop support_files initscript-start
@@ -309,7 +313,11 @@
 	$(RM) -r lib/mailman
 	$(RM) -rf lib/canonical/launchpad/icing/build/*
 	$(RM) -r $(CODEHOSTING_ROOT)
-	$(RM) $(WADL_FILE) $(API_INDEX)
+	mv $(APIDOC_DIR)/wadl-testrunner-devel.xml \
+	    $(APIDOC_DIR)/wadl-testrunner-devel.xml.bak
+	$(RM) $(APIDOC_DIR)/wadl*.xml $(API_INDEX)
+	mv $(APIDOC_DIR)/wadl-testrunner-devel.xml.bak \
+	    $(APIDOC_DIR)/wadl-testrunner-devel.xml
 	$(RM) $(BZR_VERSION_INFO)
 	$(RM) _pythonpath.py
 	$(RM) -rf \

=== modified file 'configs/development/launchpad-lazr.conf'
--- configs/development/launchpad-lazr.conf	2010-02-24 05:00:02 +0000
+++ configs/development/launchpad-lazr.conf	2010-03-03 00:11:15 +0000
@@ -262,7 +262,7 @@
 
 [vhost.api]
 hostname: api.launchpad.dev
-rooturl: https://api.launchpad.dev/beta/
+rooturl: https://api.launchpad.dev/
 
 [vhost.blueprints]
 hostname: blueprints.launchpad.dev

=== modified file 'configs/testrunner-appserver/launchpad-lazr.conf'
--- configs/testrunner-appserver/launchpad-lazr.conf	2010-02-12 19:34:42 +0000
+++ configs/testrunner-appserver/launchpad-lazr.conf	2010-03-03 00:11:15 +0000
@@ -23,7 +23,7 @@
 rooturl: http://launchpad.dev:8085/
 
 [vhost.api]
-rooturl: http://api.launchpad.dev:8085/beta/
+rooturl: http://api.launchpad.dev:8085/
 
 [vhost.blueprints]
 rooturl: http://blueprints.launchpad.dev:8085/

=== modified file 'configs/testrunner/launchpad-lazr.conf'
--- configs/testrunner/launchpad-lazr.conf	2010-02-24 05:00:02 +0000
+++ configs/testrunner/launchpad-lazr.conf	2010-03-03 00:11:15 +0000
@@ -229,7 +229,7 @@
 rooturl: http://launchpad.dev/
 
 [vhost.api]
-rooturl: http://api.launchpad.dev/beta/
+rooturl: http://api.launchpad.dev/
 
 [vhosts]
 use_https: False

=== renamed file 'lib/canonical/launchpad/apidoc/wadl-testrunner.xml' => 'lib/canonical/launchpad/apidoc/wadl-testrunner-devel.xml'
=== modified file 'lib/canonical/launchpad/doc/webapp-publication.txt'
--- lib/canonical/launchpad/doc/webapp-publication.txt	2010-02-12 19:34:42 +0000
+++ lib/canonical/launchpad/doc/webapp-publication.txt	2010-03-03 00:11:15 +0000
@@ -38,7 +38,7 @@
     althosts:
     ----
     api @ api.launchpad.dev
-    rooturl: http://api.launchpad.dev/beta/
+    rooturl: http://api.launchpad.dev/
     althosts:
     ----
     blueprints @ blueprints.launchpad.dev

=== modified file 'lib/canonical/launchpad/pagetests/webservice/xx-wadl.txt'
--- lib/canonical/launchpad/pagetests/webservice/xx-wadl.txt	2009-08-20 04:46:48 +0000
+++ lib/canonical/launchpad/pagetests/webservice/xx-wadl.txt	2010-03-03 00:11:15 +0000
@@ -1,16 +1,18 @@
 = Launchpad's WADL caching =
 
-Because Launchpad's main WADL file is so big, we cache it internally.
-Because it only changes when the Launchpad software changes, it is
-cached to a file.  Right now, the Launchpad webservice serves a
-special test file (canonical/launchpad/apidoc/wadl-testrunner.xml)
-when a client asks for the big WADL definition.  It was loaded on
-module import using the canonical.config.config.instance_name, so a
-development instance will use the file
-canonical/launchpad/apidoc/wadl-development.xml.
+Because Launchpad's main WADL files are so big, we cache them
+internally: one WADL file for every version of the web service.
+Because the WADL only changes when the Launchpad software changes,
+these documents are cached to files.  Right now, the Launchpad
+webservice serves a special test file
+(canonical/launchpad/apidoc/wadl-testrunner-devel.xml) when a client
+asks for the big WADL definition for the 'devel' version. The
+'testrunner' part comes from canonical.config.config.instance_name, so
+a development instance will use the file
+canonical/launchpad/apidoc/wadl-development-{version}.xml.
 
     >>> test_wadl = webservice.get(
-    ...     '/', 'application/vd.sun.wadl+xml').body
+    ...     '/', 'application/vd.sun.wadl+xml', api_version='devel').body
     >>> print test_wadl
     <?xml version="1.0"?>
     <wadl:application xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
@@ -24,15 +26,16 @@
     </wadl:application>
     <BLANKLINE>
 
-Let's look at the real contents, though.  To do this, we need to do
-what utilities/create-lp-wadl.py does: clear out the cache on the
-class.
+Let's look at the real contents, though.  To do this, we need to
+deactivate the cache. Simply clearing it out will just cause it to be
+filled up again.
 
     >>> from canonical.launchpad.systemhomes import WebServiceApplication
+    >>> old_cached_wadl = WebServiceApplication.cached_wadl
     >>> WebServiceApplication.cached_wadl = None
 
     >>> wadl = webservice.get(
-    ...     '/', 'application/vd.sun.wadl+xml').body
+    ...     '/', 'application/vd.sun.wadl+xml', api_version='devel').body
     >>> wadl = wadl.decode('UTF-8')
 
 The real file is much bigger than the test file.
@@ -40,15 +43,38 @@
     >>> len(wadl) > len(test_wadl)
     True
 
-The new output is now cached on the WebServiceApplication class.
-
-    >>> WebServiceApplication.cached_wadl == wadl
+The WADL we received is keyed to the 'devel' version of the web
+service. This version will always be present.
+
+    >>> 'http://api.launchpad.dev/devel/' in wadl
+    True
+
+If we retrieve the WADL for the '1.0' version of the web service,
+we'll get a document keyed to the '1.0' version. The '1.0' version
+will be deprecated along with the Lucid release of Ubuntu.
+
+    >>> wadl_10 = webservice.get(
+    ...     '/', 'application/vd.sun.wadl+xml', api_version='1.0').body
+    >>> wadl_10 = wadl_10.decode('UTF-8')
+
+    >>> 'http://api.launchpad.dev/1.0/' in wadl_10
+    True
+
+If we retrieve the WADL for the 'beta' version of the web service,
+we'll get a document keyed to the 'beta' version. The '1.0' version
+will be deprecated along with the Karmic release of Ubuntu.
+
+    >>> wadl_beta = webservice.get(
+    ...     '/', 'application/vd.sun.wadl+xml', api_version='beta').body
+    >>> wadl_beta = wadl_beta.decode('UTF-8')
+
+    >>> 'http://api.launchpad.dev/beta/' in wadl_beta
     True
 
 We don't need the cache anymore, so we'll reinstate the testing
 version. This way, other tests will have a clean slate.
 
-    >>> WebServiceApplication.cached_wadl = test_wadl
+    >>> WebServiceApplication.cached_wadl = old_cached_wadl
 
 Like all lazr.restful applications, Launchpad's web service generates
 valid WADL.

=== modified file 'lib/canonical/launchpad/systemhomes.py'
--- lib/canonical/launchpad/systemhomes.py	2010-02-16 14:07:17 +0000
+++ lib/canonical/launchpad/systemhomes.py	2010-03-03 00:11:15 +0000
@@ -343,15 +343,8 @@
 class WebServiceApplication(ServiceRootResource):
     """See `IWebServiceApplication`.
 
-    This implementation adds a 'cached_wadl' attribute.  If set, it will be
-    served by `toWADL` rather than calculating the toWADL result.
-
-    On import, the class tries to load a file to populate this attribute.  By
-    doing it on import, this makes it easy to clear, as is needed by
-    utilities/create-lp-wadl.py.
-
-    If the attribute is not set, toWADL will set the attribute on the class
-    once it is calculated.
+    This implementation adds a 'cached_wadl' attribute, which starts
+    out as an empty dict and is populated as needed.
     """
     implements(IWebServiceApplication, ICanonicalUrlData)
 
@@ -359,31 +352,41 @@
     path = ''
     rootsite = None
 
-    _wadl_filename = os.path.join(
-        os.path.dirname(os.path.normpath(__file__)),
-        'apidoc', 'wadl-%s.xml' % config.instance_name)
-
-    cached_wadl = None
-
-    # Attempt to load the WADL.
-    _wadl_fd = None
-    try:
-        _wadl_fd = codecs.open(_wadl_filename, encoding='UTF-8')
-        try:
-            cached_wadl = _wadl_fd.read()
-        finally:
-            _wadl_fd.close()
-    except IOError:
-        pass
-    del _wadl_fd
+    cached_wadl = {}
 
     def toWADL(self):
-        """See `IWebServiceApplication`."""
-        if self.cached_wadl is not None:
-            return self.cached_wadl
-        wadl = super(WebServiceApplication, self).toWADL()
-        self.__class__.cached_wadl = wadl
-        return wadl
+        """See `IWebServiceApplication`.
+
+        Look for a cached WADL file for the request version at the
+        location used by the script
+        utilities/create-launchpad-wadl.py. If the file is present,
+        load the file and cache its contents rather than generating
+        new WADL. Otherwise, generate new WADL and cache it.
+        """
+        version = self.request.version
+        if self.__class__.cached_wadl is None:
+            # The cache has been disabled for testing
+            # purposes. Generate the WADL.
+            return super(WebServiceApplication, self).toWADL()
+        if  version not in self.__class__.cached_wadl:
+            # It's not cached. Look for it on disk.
+            _wadl_filename = os.path.join(
+                os.path.dirname(os.path.normpath(__file__)),
+                'apidoc', 'wadl-%s-%s.xml' % (config.instance_name, version))
+
+            _wadl_fd = None
+            try:
+                _wadl_fd = codecs.open(_wadl_filename, encoding='UTF-8')
+                try:
+                    wadl = _wadl_fd.read()
+                finally:
+                    _wadl_fd.close()
+            except IOError:
+                # It's not on disk; generate it.
+                wadl = super(WebServiceApplication, self).toWADL()
+            del _wadl_fd
+            self.__class__.cached_wadl[version] = wadl
+        return self.__class__.cached_wadl[version]
 
 
 class TestOpenIDApplication:

=== modified file 'lib/lp/code/stories/webservice/xx-branchmergeproposal.txt'
--- lib/lp/code/stories/webservice/xx-branchmergeproposal.txt	2010-02-26 09:54:20 +0000
+++ lib/lp/code/stories/webservice/xx-branchmergeproposal.txt	2010-03-03 00:11:15 +0000
@@ -7,13 +7,31 @@
     >>> from lazr.restful.testing.webservice import pprint_entry
     >>> login('[e-mail]')
     >>> target = factory.makeBranch()
-    >>> target_url = str(canonical_url(target, rootsite='api'))
+
+    >>> from canonical.launchpad.webapp.servers import WebServiceTestRequest
+    >>> request = WebServiceTestRequest(version="beta")
+    >>> request.processInputs()
+
+    >>> from lazr.restful.utils import get_current_web_service_request
+    >>> request = get_current_web_service_request()
+
+    >>> def fix_url(url):
+    ...     """Convert a browser request to a web service client request.
+    ...     This is a bit of a hack, but it's the simplest way to get a
+    ...     URL that the web service client will respect."""
+    ...     return url.replace("launchpad.dev/api/", "api.launchpad.dev/")
+
+    >>> target_url = fix_url(str(canonical_url(
+    ...     target, request=request, rootsite='api')))
     >>> source = factory.makeBranchTargetBranch(target.target)
-    >>> source_url = str(canonical_url(source, rootsite='api'))
+    >>> source_url = fix_url(str(
+    ...     canonical_url(source, request=request, rootsite='api')))
     >>> prerequisite = factory.makeBranchTargetBranch(target.target)
-    >>> prerequisite_url = canonical_url(prerequisite, rootsite='api')
+    >>> prerequisite_url = fix_url(str(canonical_url(
+    ...     prerequisite, request=request, rootsite='api')))
     >>> registrant = source.registrant
-    >>> reviewer_url = canonical_url(factory.makePerson(), rootsite='api')
+    >>> reviewer_url = fix_url(str(canonical_url(
+    ...     factory.makePerson(), request=request, rootsite='api')))
     >>> logout()
     >>> registrant_webservice = webservice_for_person(
     ...     registrant, permission=OAuthPermission.WRITE_PUBLIC)
@@ -28,7 +46,7 @@
     >>> pprint_entry(bmp)
     address: u'[e-mail]'
     all_comments_collection_link:
-        u'http://api.launchpad.dev/beta/~.../+merge/.../all_comments'
+        u'http://api.launchpad.dev/devel/~.../+merge/.../all_comments'
     commit_message: u'It was merged!\n'
     date_created: u'...'
     date_merged: None
@@ -38,40 +56,40 @@
     description: u'Merge\nit!'
     merge_reporter_link: None
     merged_revno: None
-    prerequisite_branch_link: u'http://api.launchpad.dev/beta/~...'
+    prerequisite_branch_link: u'http://api.launchpad.dev/devel/~...'
     preview_diff_link: None
     private: False
     queue_position: None
     queue_status: u'Needs review'
     queued_revid: None
     queuer_link: None
-    registrant_link: u'http://api.launchpad.dev/beta/~person-name...'
+    registrant_link: u'http://api.launchpad.dev/devel/~person-name...'
     resource_type_link:
-        u'http://api.launchpad.dev/beta/#branch_merge_proposal'
+        u'http://api.launchpad.dev/devel/#branch_merge_proposal'
     reviewed_revid: None
     reviewer_link: None
-    self_link: u'http://api.launchpad.dev/beta/~.../+merge/...'
-    source_branch_link: u'http://api.launchpad.dev/beta/~...'
+    self_link: u'http://api.launchpad.dev/devel/~.../+merge/...'
+    source_branch_link: u'http://api.launchpad.dev/devel/~...'
     superseded_by_link: None
     supersedes_link: None
-    target_branch_link: u'http://api.launchpad.dev/beta/~...'
+    target_branch_link: u'http://api.launchpad.dev/devel/~...'
     votes_collection_link:
-        u'http://api.launchpad.dev/beta/~.../+merge/.../votes'
+        u'http://api.launchpad.dev/devel/~.../+merge/.../votes'
 
 Our review request is listed in the votes collection.
 
     >>> votes = webservice.get(
     ...     bmp['votes_collection_link']).jsonBody()
     >>> pprint_entry(votes['entries'][0])
-    branch_merge_proposal_link: u'http://api.launchpad.dev/beta/~.../+merge/...'
+    branch_merge_proposal_link: u'http://api.launchpad.dev/devel/~.../+merge/...'
     comment_link: None
     date_created: u'...'
     is_pending: True
-    registrant_link: u'http://api.launchpad.dev/beta/~person-name...'
-    resource_type_link: u'http://api.launchpad.dev/beta/#code_review_vote_reference'
+    registrant_link: u'http://api.launchpad.dev/devel/~person-name...'
+    resource_type_link: u'http://api.launchpad.dev/devel/#code_review_vote_reference'
     review_type: u'green'
-    reviewer_link: u'http://api.launchpad.dev/beta/~person-name...'
-    self_link: u'http://api.launchpad.dev/beta/~...'
+    reviewer_link: u'http://api.launchpad.dev/devel/~person-name...'
+    self_link: u'http://api.launchpad.dev/devel/~...'
 
 
 == Get an existing merge proposal ==
@@ -96,7 +114,8 @@
     ...     vote=CodeReviewVote.ABSTAIN, parent=comment,
     ...     merge_proposal=fixit_proposal)
 
-    >>> proposal_url = canonical_url(fixit_proposal, rootsite='api')
+    >>> proposal_url = fix_url(canonical_url(
+    ...     fixit_proposal, request=request, rootsite='api'))
     >>> new_person = factory.makePerson()
     >>> target_owner = fixit_proposal.target_branch.owner
     >>> logout()

=== modified file 'utilities/create-lp-wadl.py'
--- utilities/create-lp-wadl.py	2009-10-17 14:06:03 +0000
+++ utilities/create-lp-wadl.py	2010-03-03 00:11:15 +0000
@@ -7,12 +7,15 @@
 
 Usage hint:
 
-% LPCONFIG="edge" utilities/create-lp-wadl.py > launchpad.wadl
+% LPCONFIG="edge" utilities/create-lp-wadl.py launchpad-%(version)s.wadl
 """
 
 import _pythonpath
 
 import sys
+import urlparse
+
+from zope.component import getUtility
 
 from canonical.launchpad.ftests import login, ANONYMOUS
 from canonical.launchpad.scripts import execute_zcml_for_scripts
@@ -20,27 +23,41 @@
     WebServicePublication, WebServiceTestRequest)
 from canonical.launchpad.webapp.vhosts import allvhosts
 from canonical.launchpad.systemhomes import WebServiceApplication
+from lazr.restful.interfaces import IWebServiceConfiguration
 
-def main():
+def main(path_template):
     WebServiceApplication.cached_wadl = None # do not use cached file version
     execute_zcml_for_scripts()
+    config = getUtility(IWebServiceConfiguration)
 
     # Request the WADL from the root resource.
     # We do this by creating a request object asking for a WADL
     # representation.
-    request = WebServiceTestRequest(environ={
-        'SERVER_URL': allvhosts.configs['api'].rooturl,
-        'HTTP_HOST': allvhosts.configs['api'].hostname,
-        'HTTP_ACCEPT': 'application/vd.sun.wadl+xml'
-        })
-    # We then bypass the usual publisher processing by associating
-    # the request with the WebServicePublication (usually done  by the
-    # publisher) and then calling the root resource - retrieved through
-    # getApplication().
-    request.setPublication(WebServicePublication(None))
-    login(ANONYMOUS, request)
-    print request.publication.getApplication(request)(request)
+    for version in config.active_versions:
+        url = urlparse.urljoin(allvhosts.configs['api'].rooturl, version)
+        request = WebServiceTestRequest(version=version, environ={
+            'SERVER_URL': url,
+            'HTTP_HOST': allvhosts.configs['api'].hostname,
+            'HTTP_ACCEPT': 'application/vd.sun.wadl+xml'
+            })
+        # We then bypass the usual publisher processing by associating
+        # the request with the WebServicePublication (usually done  by the
+        # publisher) and then calling the root resource - retrieved through
+        # getApplication().
+        request.setPublication(WebServicePublication(None))
+        login(ANONYMOUS, request)
+        filename = path_template % {'version' : version}
+        print "Writing WADL for version %s to %s." % (version, filename)
+        f = open(filename, 'w')
+        content = request.publication.getApplication(request)(request)
+        f.write(content)
+        f.close()
     return 0
 
 if __name__ == '__main__':
-    sys.exit(main())
+    if len(sys.argv) != 2:
+        print "Usage: %s [WADL path template]" % sys.argv[0]
+        print " Example: %s path/to/wadl/wadl-%%(version).xml" % (
+            sys.argv[0])
+        sys.exit(-1)
+    sys.exit(main(sys.argv[1]))