Launchpad itself

Merge lp:~benji/launchpad/wadl-refactoring into lp:launchpad

  1. wadl-refactoring
  2. Merge into devel
Proposed by Benji York
Status: Merged
Approved by: Brad Crittenden
Approved revision: no longer in the source branch.
Merged at revision: 11755
Proposed branch: lp:~benji/launchpad/wadl-refactoring
Merge into: lp:launchpad
Diff against target: 365 lines (+223/-53)
6 files modified
Makefile (+1/-1)
lib/canonical/launchpad/ftests/test_wadl_generation.py (+35/-0)
lib/canonical/launchpad/rest/wadl.py (+62/-0)
lib/lp/testing/matchers.py (+35/-0)
lib/lp/testing/tests/test_matchers.py (+35/-0)
utilities/create-lp-wadl-and-apidoc.py (+55/-52)
To merge this branch: bzr merge lp:~benji/launchpad/wadl-refactoring
Reviewer Review Type Date Requested Status
Gavin Panella (community) Approve
Brad Crittenden (community) code Approve
Review via email: mp+36452@code.launchpad.net

Description of the change

This branch extracts the refactorings and tests from an earlier branch
that we've decided to scrap. All of the code in this branch has been
previously reviewed and approved.

The theme of the refactorings is to provide APIs to generate the WADL
and the web service documentation. The branch also includes smoke tests
to be sure that something resembling HTML and WADL are generated and no
exceptions are raised.

Some whitespace lint was fixed in this branch.

To run the added tests: bin/test -c test_wadl_generation

To post a comment you must log in.
Revision history for this message
Gavin Panella (allenap) wrote :

Nice clean up :)

I have a few fairly minor comments. [4] needs fixing before this
branch is safe to land.

[1]

+ self.assert_(wadl.startswith('<?xml '))

I think this might be better as:

   from lp.testing.matchers import Startswith
   self.assertThat(wadl, StartsWith('<?xml '))

You'll need to inherit from lp.testing.TestCase (or, more accurately,
testtools.TestCase) to get assertThat().

[2]

+ self.assert_('<html ' in html)

Same as [1].

[3]

+ return subprocess.Popen(
+ ['xsltproc', stylesheet, wadl_filename],
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE).communicate()[0]

Unless it's useless and/or noisy, I think stderr should be left alone,
so:

    return subprocess.Popen(
        ['xsltproc', stylesheet, wadl_filename],
        stdout=subprocess.PIPE).communicate()[0]

I also think the exit code should be checked, so:

    args = ['xsltproc', stylesheet, wadl_filename]
    process = subprocess.Popen(args, stdout=subprocess.PIPE)
    out, err = process.communicate()
    if process.returncode == 0:
        return out
    else
        raise subprocess.CalledProcessError(
            process.returncode, args)

This is similar to how subprocess.check_call() behaves.

[4]

-import _pythonpath
-

It's ugly, looks like lint, but you'll need this.

It may be worth adding a test to show that the script can run
successfully, even if it's only to show the help.

review: Needs Fixing (code)
Revision history for this message
Benji York (benji) wrote :

On Thu, Sep 23, 2010 at 12:34 PM, Gavin Panella
<email address hidden> wrote:
> Review: Needs Fixing code
> Nice clean up :)
>
> I have a few fairly minor comments. [4] needs fixing before this
> branch is safe to land.
>
>
> [1]
>
> +            self.assert_(wadl.startswith('<?xml '))
>
> I think this might be better as:
 [snip]

Done.
> [2]
>
> +            self.assert_('<html ' in html)
>
> Same as [1].

I had to implement a Contains matcher, but that wasn't hard.

> [3]
>
> Unless it's useless and/or noisy, I think stderr should be left alone,

Agreed.

> I also think the exit code should be checked, so:

Done.

> [4]
>
> -import _pythonpath
> -
>
> It's ugly, looks like lint, but you'll need this.

Fixed.

> It may be worth adding a test to show that the script can run
> successfully, even if it's only to show the help.

Since we can be sure that the script runs because it is run (almost)
every time make is run, I decided not to implement this.
--
Benji York

Revision history for this message
Brad Crittenden (bac) wrote :

Hi Benji,

Your changes as suggested by Gavin look good. We discussed the following on IRC:

1) Capitalize and punctuate

import _pythonpath # not lint, actually needed

2) alphabetize:

 20 from lp.testing.matchers import (
 21 StartsWith,
 22 Contains,
 23 )

3) Use a tuple instead of a list:
54 args = ['xsltproc', stylesheet, wadl_filename]

Thanks for the branch. I enjoyed seeing how matchers work and are extended.

review: Approve (code)
Revision history for this message
Gavin Panella (allenap) wrote :

Looks good :)

review: Approve
Revision history for this message
Brad Crittenden (bac) wrote :

Hi Benji,

I notice this branch has not landed yet. Is there something blocking you or has the work been reconsidered?

The reason I saw that it has not landed is I remember you implementing the contains matcher and I wanted to use it.

Revision history for this message
Benji York (benji) wrote :

On Mon, Oct 4, 2010 at 2:15 AM, Brad Crittenden <email address hidden> wrote:
> I notice this branch has not landed yet.  Is there something blocking you or has the work been reconsidered?
>
> The reason I saw that it has not landed is I remember you implementing the contains matcher and I wanted to use it.

There have been some EC2 hitches that have kept it from landing. As
soon as the current EC2 issue is resolved, I'll be landing the branch.
--
Benji York

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'Makefile'
--- Makefile 2010-10-19 15:56:07 +0000
+++ Makefile 2010-10-19 18:54:00 +0000
@@ -60,7 +60,7 @@
6060
61$(API_INDEX): $(BZR_VERSION_INFO)61$(API_INDEX): $(BZR_VERSION_INFO)
62 mkdir -p $(APIDOC_DIR).tmp62 mkdir -p $(APIDOC_DIR).tmp
63 LPCONFIG=$(LPCONFIG) $(PY) ./utilities/create-lp-wadl-and-apidoc.py "$(WADL_TEMPLATE)"63 LPCONFIG=$(LPCONFIG) $(PY) ./utilities/create-lp-wadl-and-apidoc.py --force "$(WADL_TEMPLATE)"
64 mv $(APIDOC_DIR).tmp $(APIDOC_DIR)64 mv $(APIDOC_DIR).tmp $(APIDOC_DIR)
6565
66apidoc: compile $(API_INDEX)66apidoc: compile $(API_INDEX)
6767
=== added file 'lib/canonical/launchpad/ftests/test_wadl_generation.py'
--- lib/canonical/launchpad/ftests/test_wadl_generation.py 1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/ftests/test_wadl_generation.py 2010-10-19 18:54:00 +0000
@@ -0,0 +1,35 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""Tests for the web service WADL and HTML generation APIs."""
5
6__metaclass__ = type
7
8import pkg_resources
9import shutil
10import subprocess
11import tempfile
12
13from zope.component import getUtility
14
15from canonical.launchpad.rest.wadl import generate_wadl, generate_html
16from canonical.launchpad.systemhomes import WebServiceApplication
17from canonical.testing import LaunchpadFunctionalLayer
18from lazr.restful.interfaces import IWebServiceConfiguration
19from lp.testing import TestCase
20from lp.testing.matchers import (
21 Contains,
22 StartsWith,
23 )
24
25
26class SmokeTestWadlAndDocGeneration(TestCase):
27 """Smoke test the WADL and HTML generation front-end functions."""
28
29 layer = LaunchpadFunctionalLayer
30
31 def test_wadl(self):
32 config = getUtility(IWebServiceConfiguration)
33 for version in config.active_versions:
34 wadl = generate_wadl(version)
35 self.assertThat(wadl[:40], StartsWith('<?xml '))
036
=== added file 'lib/canonical/launchpad/rest/wadl.py'
--- lib/canonical/launchpad/rest/wadl.py 1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/rest/wadl.py 2010-10-19 18:54:00 +0000
@@ -0,0 +1,62 @@
1# Copyright 2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""APIs to generate the web sercice WADL and documentation HTML."""
5
6__metaclass__ = type
7
8import pkg_resources
9import subprocess
10import urlparse
11
12from canonical.launchpad.webapp.interaction import (
13 ANONYMOUS,
14 setupInteractionByEmail,
15 )
16from canonical.launchpad.webapp.servers import (
17 WebServicePublication,
18 WebServiceTestRequest,
19 )
20from canonical.launchpad.webapp.vhosts import allvhosts
21
22
23def generate_wadl(version):
24 """Generate the WADL for the given version of the web service."""
25 url = urlparse.urljoin(allvhosts.configs['api'].rooturl, version)
26 # Since we want HTTPS URLs we have to munge the request URL.
27 url = url.replace('http://', 'https://')
28 request = WebServiceTestRequest(version=version, environ={
29 'SERVER_URL': url,
30 'HTTP_HOST': allvhosts.configs['api'].hostname,
31 'HTTP_ACCEPT': 'application/vd.sun.wadl+xml',
32 })
33 # We then bypass the usual publisher processing by associating
34 # the request with the WebServicePublication (usually done by the
35 # publisher) and then calling the root resource - retrieved
36 # through getApplication().
37 request.setPublication(WebServicePublication(None))
38 setupInteractionByEmail(ANONYMOUS, request)
39 return request.publication.getApplication(request)(request)
40
41
42def generate_html(wadl_filename, suppress_stderr=True):
43 """Given a WADL file generate HTML documentation from it."""
44 # If we're supposed to prevent the subprocess from generating output on
45 # stderr (like we want to do during test runs), we reassign the subprocess
46 # stderr file handle and then discard the output. Otherwise we let the
47 # subprocess inherit stderr.
48 stylesheet = pkg_resources.resource_filename(
49 'launchpadlib', 'wadl-to-refhtml.xsl')
50 if suppress_stderr:
51 stderr = subprocess.PIPE
52 else:
53 stderr = None
54 args = ('xsltproc', stylesheet, wadl_filename)
55 process = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=stderr)
56
57 output = process.communicate()[0]
58 if process.returncode != 0:
59 raise subprocess.CalledProcessError(process.returncode, args)
60
61 return output
62
063
=== modified file 'lib/lp/testing/matchers.py'
--- lib/lp/testing/matchers.py 2010-09-19 23:52:49 +0000
+++ lib/lp/testing/matchers.py 2010-10-19 18:54:00 +0000
@@ -227,6 +227,41 @@
227 return None227 return None
228228
229229
230class DoesNotContain(Mismatch):
231
232 def __init__(self, matchee, expected):
233 """Create a DoesNotContain Mismatch.
234
235 :param matchee: the string that did not match.
236 :param expected: the string that `matchee` was expected to contain.
237 """
238 self.matchee = matchee
239 self.expected = expected
240
241 def describe(self):
242 return "'%s' does not contain '%s'." % (
243 self.matchee, self.expected)
244
245
246class Contains(Matcher):
247 """Checks whether one string contains another."""
248
249 def __init__(self, expected):
250 """Create a Contains Matcher.
251
252 :param expected: the string that matchees should contain.
253 """
254 self.expected = expected
255
256 def __str__(self):
257 return "Contains '%s'." % self.expected
258
259 def match(self, matchee):
260 if self.expected not in matchee:
261 return DoesNotContain(matchee, self.expected)
262 return None
263
264
230class IsConfiguredBatchNavigator(Matcher):265class IsConfiguredBatchNavigator(Matcher):
231 """Check that an object is a batch navigator."""266 """Check that an object is a batch navigator."""
232267
233268
=== modified file 'lib/lp/testing/tests/test_matchers.py'
--- lib/lp/testing/tests/test_matchers.py 2010-08-20 20:31:18 +0000
+++ lib/lp/testing/tests/test_matchers.py 2010-10-19 18:54:00 +0000
@@ -20,6 +20,8 @@
20from lp.testing import TestCase20from lp.testing import TestCase
21from lp.testing._webservice import QueryCollector21from lp.testing._webservice import QueryCollector
22from lp.testing.matchers import (22from lp.testing.matchers import (
23 Contains,
24 DoesNotContain,
23 DoesNotCorrectlyProvide,25 DoesNotCorrectlyProvide,
24 DoesNotProvide,26 DoesNotProvide,
25 DoesNotStartWith,27 DoesNotStartWith,
@@ -257,3 +259,36 @@
257 matcher = StartsWith("bar")259 matcher = StartsWith("bar")
258 mismatch = matcher.match("foo")260 mismatch = matcher.match("foo")
259 self.assertEqual("bar", mismatch.expected)261 self.assertEqual("bar", mismatch.expected)
262
263
264class DoesNotContainTests(TestCase):
265
266 def test_describe(self):
267 mismatch = DoesNotContain("foo", "bar")
268 self.assertEqual(
269 "'foo' does not contain 'bar'.", mismatch.describe())
270
271
272class ContainsTests(TestCase):
273
274 def test_str(self):
275 matcher = Contains("bar")
276 self.assertEqual("Contains 'bar'.", str(matcher))
277
278 def test_match(self):
279 matcher = Contains("bar")
280 self.assertIs(None, matcher.match("foo bar baz"))
281
282 def test_mismatch_returns_does_not_start_with(self):
283 matcher = Contains("bar")
284 self.assertIsInstance(matcher.match("foo"), DoesNotContain)
285
286 def test_mismatch_sets_matchee(self):
287 matcher = Contains("bar")
288 mismatch = matcher.match("foo")
289 self.assertEqual("foo", mismatch.matchee)
290
291 def test_mismatch_sets_expected(self):
292 matcher = Contains("bar")
293 mismatch = matcher.match("foo")
294 self.assertEqual("bar", mismatch.expected)
260295
=== modified file 'utilities/create-lp-wadl-and-apidoc.py'
--- utilities/create-lp-wadl-and-apidoc.py 2010-10-18 21:53:28 +0000
+++ utilities/create-lp-wadl-and-apidoc.py 2010-10-19 18:54:00 +0000
@@ -1,90 +1,93 @@
1#! /usr/bin/python -S1#! /usr/bin/python -S
2#2#
3# Copyright 2009 Canonical Ltd. This software is licensed under the3# Copyright 2010 Canonical Ltd. This software is licensed under the
4# GNU Affero General Public License version 3 (see the file LICENSE).4# GNU Affero General Public License version 3 (see the file LICENSE).
55
6"""Create a static WADL file describing the current webservice.6"""Create a static WADL file describing the current webservice.
77
8Usage hint:8Example:
99
10% LPCONFIG="edge" utilities/create-lp-wadl.py launchpad-%(version)s.wadl10 % LPCONFIG=development bin/py utilities/create-lp-wadl-and-apidoc.py \\
11 "lib/canonical/launchpad/apidoc/wadl-development-%(version)s.xml"
11"""12"""
1213import _pythonpath # Not lint, actually needed.
13import _pythonpath14
1415import optparse
15import os16import os
16import pkg_resources
17import subprocess
18import sys17import sys
19import urlparse
2018
21from zope.component import getUtility19from zope.component import getUtility
22from zope.pagetemplate.pagetemplatefile import PageTemplateFile20from zope.pagetemplate.pagetemplatefile import PageTemplateFile
2321
22from canonical.launchpad.rest.wadl import generate_wadl, generate_html
24from canonical.launchpad.scripts import execute_zcml_for_scripts23from canonical.launchpad.scripts import execute_zcml_for_scripts
25from canonical.launchpad.webapp.interaction import (
26 ANONYMOUS, setupInteractionByEmail)
27from canonical.launchpad.webapp.servers import (
28 WebServicePublication, WebServiceTestRequest)
29from canonical.launchpad.webapp.vhosts import allvhosts
30from canonical.launchpad.systemhomes import WebServiceApplication24from canonical.launchpad.systemhomes import WebServiceApplication
31from lazr.restful.interfaces import IWebServiceConfiguration25from lazr.restful.interfaces import IWebServiceConfiguration
3226
3327
34def main(path_template):28def write(filename, content):
29 """Replace the named file with the given string."""
30 f = open(filename, 'w')
31 f.write(content)
32 f.close()
33
34
35def main(path_template, force=False):
35 WebServiceApplication.cached_wadl = None # do not use cached file version36 WebServiceApplication.cached_wadl = None # do not use cached file version
36 execute_zcml_for_scripts()37 execute_zcml_for_scripts()
37 config = getUtility(IWebServiceConfiguration)38 config = getUtility(IWebServiceConfiguration)
38 directory, ignore = os.path.split(path_template)39 directory = os.path.dirname(path_template)
39
40 stylesheet = pkg_resources.resource_filename(
41 'launchpadlib', 'wadl-to-refhtml.xsl')
4240
43 # First, create an index.html with links to all the HTML41 # First, create an index.html with links to all the HTML
44 # documentation files we're about to generate.42 # documentation files we're about to generate.
45 template_file = 'apidoc-index.pt'43 template_file = 'apidoc-index.pt'
46 template = PageTemplateFile(template_file)44 template = PageTemplateFile(template_file)
47 f = open(os.path.join(directory, "index.html"), 'w')45 index_filename = os.path.join(directory, "index.html")
46 print "Writing index:", index_filename
47 f = open(index_filename, 'w')
48 f.write(template(config=config))48 f.write(template(config=config))
4949
50 # Request the WADL from the root resource.
51 # We do this by creating a request object asking for a WADL
52 # representation.
53 for version in config.active_versions:50 for version in config.active_versions:
54 url = urlparse.urljoin(allvhosts.configs['api'].rooturl, version)51 wadl_filename = path_template % {'version': version}
55 request = WebServiceTestRequest(version=version, environ={52 # If the WADL file doesn't exist or we're being forced to regenerate
56 'SERVER_URL': url,53 # it...
57 'HTTP_HOST': allvhosts.configs['api'].hostname,54 if (not os.path.exists(wadl_filename) or force):
58 'HTTP_ACCEPT': 'application/vd.sun.wadl+xml'55 print "Writing WADL for version %s to %s." % (
59 })56 version, wadl_filename)
60 # We then bypass the usual publisher processing by associating57 write(wadl_filename, generate_wadl(version))
61 # the request with the WebServicePublication (usually done by the58 else:
62 # publisher) and then calling the root resource - retrieved through59 print "Skipping already present WADL file:", wadl_filename
63 # getApplication().
64 request.setPublication(WebServicePublication(None))
65 setupInteractionByEmail(ANONYMOUS, request)
66 filename = path_template % {'version' : version}
67 print "Writing WADL for version %s to %s." % (version, filename)
68 f = open(filename, 'w')
69 content = request.publication.getApplication(request)(request)
70 f.write(content)
71 f.close()
7260
73 # Now, convert the WADL into an human-readable description and61 # Now, convert the WADL into an human-readable description and
74 # put the HTML in the same directory as the WADL.62 # put the HTML in the same directory as the WADL.
75 html_filename = os.path.join(directory, version + ".html")63 html_filename = os.path.join(directory, version + ".html")
76 print "Writing apidoc for version %s to %s" % (64 # If the HTML file doesn't exist or we're being forced to regenerate
77 version, html_filename)65 # it...
78 stdout = open(html_filename, "w")66 if (not os.path.exists(html_filename) or force):
79 subprocess.Popen(['xsltproc', stylesheet, filename], stdout=stdout)67 print "Writing apidoc for version %s to %s" % (
80 stdout.close()68 version, html_filename)
69 write(html_filename, generate_html(wadl_filename,
70 suppress_stderr=False))
71 else:
72 print "Skipping already present HTML file:", html_filename
8173
82 return 074 return 0
8375
76
77def parse_args(args):
78 usage = "usage: %prog [options] PATH_TEMPLATE"
79 parser = optparse.OptionParser(usage=usage)
80 parser.add_option(
81 "--force", action="store_true",
82 help="Replace any already-existing files.")
83 parser.set_defaults(force=False)
84 options, args = parser.parse_args(args)
85 if len(args) != 2:
86 parser.error("A path template is required.")
87
88 return options, args
89
90
84if __name__ == '__main__':91if __name__ == '__main__':
85 if len(sys.argv) != 2:92 options, args = parse_args(sys.argv)
86 print "Usage: %s [WADL path template]" % sys.argv[0]93 sys.exit(main(args[1], options.force))
87 print " Example: %s path/to/wadl/wadl-%%(version).xml" % (
88 sys.argv[0])
89 sys.exit(-1)
90 sys.exit(main(sys.argv[1]))