Code review comment for lp:~leonardr/lazr.restful/launchpad-integration

Revision history for this message
Leonard Richardson (leonardr) wrote :

This branch creates a new utility function get_current_web_service_request() and changes all lazr.restful code to use it instead of get_current_browser_request(). I have not removed get_current_browser_request(), because I know it's used correctly elsewhere (eg. in Launchpad).

The multi-version code creates a big distinction between requests initiated by a web service client, and requests initiated by a web browser. Specifically, web browser requests don't have a version number or version-specific marker interface associated with them.

When I integrated the multi-version lazr.restful into Launchpadlib, this caused many Windmill tests to fail: the browser would make a request, and Launchpad would try to generate a representation of the context as though the incoming request were a web service request. But since the incoming request wasn't versioned, the versioned IEntry lookup would fail, and it would appear as though (eg.) bugs and users weren't published on the web service. Without a value in LP.cache.context, subsequent Ajax requests would fail, causing the Windmill test to fail.

The most confusing part of this branch is only somewhat related. While trying to get test_applyChanges_binds_to_resource_context to use a versioned request object, I noticed that applyChanges wasn't really doing anything (even in the case where the test supposedly passed). I added tests that prove that applyChanges changes the value for the field it's trying to change, and to get them to work I had to define a dummy IAbsoluteURL implementation for the entry on which applyChanges was running.

« Back to merge proposal