launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #13585
[Merge] lp:~wallyworld/launchpad/vouchers-timeout2-1014641 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/vouchers-timeout2-1014641 into lp:launchpad.
Commit message:
Move the voucher redemption code to a garbo job, making the process to redeem vouchers more robust to salesforce issues.
Requested reviews:
Curtis Hovey (sinzui): code
Related bugs:
Bug #1014641 in Launchpad itself: "+vouchers times out"
https://bugs.launchpad.net/launchpad/+bug/1014641
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/vouchers-timeout2-1014641/+merge/130719
== Implementation ==
This is a resubmission since the first attempt failed QA.
This branch changes the workflow of how vouchers are redeemed. The redemption via Salesforce has been moved from in-line to a garbo job. Viewing highlights:
- redeeming a voucher updates/creates the commercial subscription record and sets the sales_force_id to 'pending-xxxxx' where xxxx is the voucher id.
- the voucher is not redeemed immediately in salesforce
- the +vouchers view has been changed to remove from the salesforce unredeemed vouchers list any pending vouchers so that someone can not accidentally try and re-redeem a pending voucher
- any attempt to redeem a voucher which has already been submitted (because someone else got in first) results in an field error being displayed on the form
- a garbo job runs frequently to redeem pending vouchers in salesforce and update the commercial subscription record in Launchpad
This version of the branch adds extra functionality to allow the salesforce proxy to specify a timeout when making its RPCs. Previously, the salesforce proxy was only used within Launchpad, and so the lp default request timeout was used. Now, the proxy can be used within a garbo job, where no default timeout is set. I've introduced a new [commercial] configuration setting: voucher_proxy_timeout. This compliments the existing settings voucher_proxy_url, voucher_proxy_port etc. The value is set to 60s. I like the idea of a separately configured timeout for salesforce, rather than reusing the lp timeout, since they are separate systems.
So now the salesforce proxy initialises it's transport with the configured timeout. A little extra infrastructure was required to allow the @with_timeout decorator to get the timeout value from an instance variable on the calling class rather than a hard coded value.
== Tests ==
Add test for new garbo job.
Add tests for new view behaviour regarding not showing pending vouchers and handling submission of vouchers which have been already processed
Update some doctests which repeatedly submit the same voucher id for different projects because this is no longer allowed.
Update timeout.txt to test the new timeout functionality.
== Lint ==
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
configs/development/launchpad-lazr.conf
database/schema/security.cfg
lib/lp/registry/errors.py
lib/lp/registry/browser/person.py
lib/lp/registry/browser/tests/test_commercialsubscription.py
lib/lp/registry/doc/commercialsubscription.txt
lib/lp/registry/model/person.py
lib/lp/registry/model/product.py
lib/lp/registry/stories/vouchers/xx-voucher-redemption.txt
lib/lp/registry/templates/person-vouchers.pt
lib/lp/scripts/garbo.py
lib/lp/scripts/tests/test_garbo.py
lib/lp/services/timeout.py
lib/lp/services/config/schema-lazr.conf
lib/lp/services/salesforce/proxy.py
lib/lp/services/webapp/doc/timeout.txt
lib/lp/testing/factory.py
--
https://code.launchpad.net/~wallyworld/launchpad/vouchers-timeout2-1014641/+merge/130719
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
=== modified file 'configs/development/launchpad-lazr.conf'
--- configs/development/launchpad-lazr.conf 2012-09-28 02:41:34 +0000
+++ configs/development/launchpad-lazr.conf 2012-10-22 03:05:23 +0000
@@ -59,6 +59,7 @@
[commercial]
voucher_proxy_url: http://launchpad.dev
voucher_proxy_port: 2323
+voucher_proxy_timeout: 60000
purchase_subscription_url: http://ubuntu.recycledmania.com/product_info.php?products_id=227
[database]
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2012-10-19 16:54:35 +0000
+++ database/schema/security.cfg 2012-10-22 03:05:23 +0000
@@ -2251,7 +2251,7 @@
public.codeimportevent = SELECT, DELETE
public.codeimporteventdata = SELECT, DELETE
public.codeimportresult = SELECT, DELETE
-public.commercialsubscription = SELECT
+public.commercialsubscription = SELECT, UPDATE
public.emailaddress = SELECT, UPDATE, DELETE
public.hwsubmission = SELECT, UPDATE
public.job = SELECT, INSERT, DELETE
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2012-10-19 16:54:35 +0000
+++ lib/lp/registry/browser/person.py 2012-10-22 03:05:23 +0000
@@ -157,6 +157,7 @@
)
from lp.registry.browser.teamjoin import TeamJoinMixin
from lp.registry.enums import PersonVisibility
+from lp.registry.errors import VoucherAlreadyRedeemed
from lp.registry.interfaces.codeofconduct import ISignedCodeOfConductSet
from lp.registry.interfaces.gpg import IGPGKeySet
from lp.registry.interfaces.irc import IIrcIDSet
@@ -230,6 +231,7 @@
)
from lp.services.salesforce.interfaces import (
ISalesforceVoucherProxy,
+ REDEEMABLE_VOUCHER_STATUSES,
SalesforceVoucherProxyException,
)
from lp.services.verification.interfaces.authtoken import LoginTokenType
@@ -1257,10 +1259,7 @@
"""Set up the fields for this view."""
self.form_fields = []
- # Make the less expensive test for commercial projects first
- # to avoid the more costly fetching of redeemable vouchers.
- if (self.has_commercial_projects and
- len(self.redeemable_vouchers) > 0):
+ if self.has_commercial_projects:
self.form_fields = (self.createProjectField() +
self.createVoucherField())
@@ -1299,6 +1298,10 @@
return field
@cachedproperty
+ def show_voucher_selection(self):
+ return self.redeemable_vouchers or self.errors
+
+ @cachedproperty
def redeemable_vouchers(self):
"""Get the list redeemable vouchers owned by the user."""
vouchers = [
@@ -1343,17 +1346,25 @@
voucher = data['voucher']
try:
- # The call to redeemVoucher returns True if it succeeds or it
- # raises an exception. Therefore the return value does not need
- # to be checked.
- salesforce_proxy.redeemVoucher(voucher.voucher_id,
- self.context,
- project)
- project.redeemSubscriptionVoucher(
- voucher=voucher.voucher_id,
- registrant=self.context,
- purchaser=self.context,
- subscription_months=voucher.term_months)
+ # Perform a check that the submitted voucher id is valid.
+ check_voucher = salesforce_proxy.getVoucher(voucher.voucher_id)
+ if not check_voucher.status in REDEEMABLE_VOUCHER_STATUSES:
+ self.addError(
+ _("Voucher %s has invalid status %s"
+ % (check_voucher.voucher_id, check_voucher.status)))
+ return
+ # Redeem the voucher in Launchpad, marking the subscription as
+ # pending. Launchpad will honour the subscription but a job will
+ # still need to be run to notify Salesforce.
+ try:
+ project.redeemSubscriptionVoucher(
+ voucher='pending-' + voucher.voucher_id,
+ registrant=self.context,
+ purchaser=self.context,
+ subscription_months=voucher.term_months)
+ except VoucherAlreadyRedeemed as error:
+ self.setFieldError('voucher', _(error.message))
+ return
self.request.response.addInfoNotification(
_("Voucher redeemed successfully"))
self.removeRedeemableVoucher(voucher)
=== modified file 'lib/lp/registry/browser/tests/test_commercialsubscription.py'
--- lib/lp/registry/browser/tests/test_commercialsubscription.py 2012-10-19 16:54:35 +0000
+++ lib/lp/registry/browser/tests/test_commercialsubscription.py 2012-10-22 03:05:23 +0000
@@ -12,9 +12,14 @@
FakeAdapterMixin,
login_celebrity,
login_person,
+ person_logged_in,
TestCaseWithFactory,
)
from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing.pages import (
+ extract_text,
+ find_tags_by_class,
+ )
from lp.testing.views import create_initialized_view
@@ -22,7 +27,7 @@
layer = DatabaseFunctionalLayer
def test_init_without_vouchers_or_projects(self):
- # Thhe view provides common view properties, but the form is disabled.
+ # The view provides common view properties, but the form is disabled.
user = self.factory.makePerson()
self.factory.makeProduct(owner=user)
voucher_proxy = TestSalesforceVoucherProxy()
@@ -33,7 +38,6 @@
self.assertEqual(user_url, view.cancel_url)
self.assertIs(None, view.next_url)
self.assertEqual(0, len(view.redeemable_vouchers))
- self.assertEqual([], view.form_fields)
def test_init_with_vouchers_and_projects(self):
# The fields are setup when the user hase both vouchers and projects.
@@ -105,6 +109,11 @@
self.assertEqual([], view.errors)
self.assertIsNot(None, project_1.commercial_subscription)
self.assertEqual(1, len(view.redeemable_vouchers))
+ # A job will notify Salesforce of the voucher redemption but here we
+ # will do it manually.
+ voucher_proxy.redeemVoucher(
+ voucher_id_1, commercial_admin, project_1)
+
form = {
'field.project': project_2.name,
'field.voucher': voucher_id_2,
@@ -115,3 +124,53 @@
self.assertEqual([], view.errors)
self.assertIsNot(None, project_2.commercial_subscription)
self.assertEqual(0, len(view.redeemable_vouchers))
+
+ def test_pending_vouchers_excluded(self):
+ # Vouchers pending redemption in Salesforce are not included in choice.
+ commercial_admin = login_celebrity('commercial_admin')
+ voucher_proxy = TestSalesforceVoucherProxy()
+ voucher_id_1 = voucher_proxy.grantVoucher(
+ commercial_admin, commercial_admin, commercial_admin, 12)
+ voucher_id_2 = voucher_proxy.grantVoucher(
+ commercial_admin, commercial_admin, commercial_admin, 12)
+ self.registerUtility(voucher_proxy, ISalesforceVoucherProxy)
+ project_1 = self.factory.makeProduct()
+ self.factory.makeCommercialSubscription(
+ project_1, False, 'pending-' + voucher_id_1)
+ view = create_initialized_view(commercial_admin, '+vouchers')
+ vouchers = list(view.widgets['voucher'].vocabulary)
+ # Only voucher2 in vocab since voucher1 is pending redemption.
+ self.assertEqual(1, len(vouchers))
+ self.assertEqual(voucher_id_2, vouchers[0].token)
+
+ def test_redeem_twice_causes_error(self):
+ # If a voucher is redeemed twice, the second attempt is rejected.
+ commercial_admin = login_celebrity('commercial_admin')
+ voucher_proxy = TestSalesforceVoucherProxy()
+ voucher_id_1 = voucher_proxy.grantVoucher(
+ commercial_admin, commercial_admin, commercial_admin, 12)
+ self.registerUtility(voucher_proxy, ISalesforceVoucherProxy)
+ project_1 = self.factory.makeProduct(name='p1')
+ project_2 = self.factory.makeProduct(name='p2')
+ url = canonical_url(commercial_admin, view_name='+vouchers')
+ browser = self.getUserBrowser(url, commercial_admin)
+ # A second browser opens the +vouchers page before the first browser
+ # attempts to redeem the voucher.
+ browser2 = self.getUserBrowser(url, commercial_admin)
+ browser.getControl(
+ 'Select the project you wish to subscribe').value = 'p1'
+ browser.getControl(
+ 'Select a voucher').getControl(voucher_id_1).selected = True
+ browser.getControl('Redeem').click()
+ with person_logged_in(commercial_admin):
+ self.assertIsNotNone(project_1.commercial_subscription)
+
+ browser2.getControl(
+ 'Select the project you wish to subscribe').value = 'p2'
+ browser2.getControl(
+ 'Select a voucher').getControl(voucher_id_1).selected = True
+ browser2.getControl('Redeem').click()
+ with person_logged_in(commercial_admin):
+ self.assertIsNone(project_2.commercial_subscription)
+ error_messages = find_tags_by_class(browser2.contents, 'message')
+ self.assertEqual(extract_text(error_messages[1]), 'Invalid value')
=== modified file 'lib/lp/registry/doc/commercialsubscription.txt'
--- lib/lp/registry/doc/commercialsubscription.txt 2012-10-19 16:54:35 +0000
+++ lib/lp/registry/doc/commercialsubscription.txt 2012-10-22 03:05:23 +0000
@@ -35,6 +35,12 @@
>>> print bzr.commercial_subscription.sales_system_id
asdf123
+If a voucher is attempted to be redeemed twice, an error occurs.
+ >>> bzr.redeemSubscriptionVoucher('asdf123', owner, owner, 12)
+ Traceback (most recent call last):
+ ...
+ VoucherAlreadyRedeemed: Voucher asdf123 has already been redeemed for Bazaar
+
Commercial subscriptions have zope.Public permissions for reading.
>>> from lp.services.webapp.authorization import check_permission
@@ -143,7 +149,7 @@
>>> subscription.date_expires = datetime(2009,11,15,tzinfo=UTC)
>>> old_date_expires = bzr.commercial_subscription.date_expires
- >>> bzr.redeemSubscriptionVoucher('foo456', owner, owner, 2,
+ >>> bzr.redeemSubscriptionVoucher('foo1456', owner, owner, 2,
... 'notes', current_datetime=now)
>>> print subscription.date_expires
2010-01-15...
@@ -152,7 +158,7 @@
>>> subscription.date_expires = datetime(2009,11,15,tzinfo=UTC)
>>> old_date_expires = bzr.commercial_subscription.date_expires
- >>> bzr.redeemSubscriptionVoucher('foo456', owner, owner, 11,
+ >>> bzr.redeemSubscriptionVoucher('foo2456', owner, owner, 11,
... 'notes', current_datetime=now)
>>> print subscription.date_expires
2010-10-15...
@@ -165,7 +171,7 @@
>>> subscription.date_expires = datetime(2009,10,31,tzinfo=UTC)
>>> old_date_expires = bzr.commercial_subscription.date_expires
- >>> bzr.redeemSubscriptionVoucher('foo456', owner, owner, 4,
+ >>> bzr.redeemSubscriptionVoucher('foo3456', owner, owner, 4,
... 'notes', current_datetime=now)
>>> print subscription.date_expires
2010-02-28...
@@ -175,7 +181,7 @@
>>> subscription.date_expires = datetime(2011,10,31,tzinfo=UTC)
>>> old_date_expires = bzr.commercial_subscription.date_expires
- >>> bzr.redeemSubscriptionVoucher('foo456', owner, owner, 4,
+ >>> bzr.redeemSubscriptionVoucher('foo4456', owner, owner, 4,
... 'notes', current_datetime=now)
>>> print subscription.date_expires
2012-02-29...
@@ -186,7 +192,7 @@
>>> subscription.date_starts = datetime(2008,1,15,tzinfo=UTC)
>>> subscription.date_expires = datetime(2009,1,15,tzinfo=UTC)
>>> now = datetime(2010, 1, 1, tzinfo=UTC)
- >>> bzr.redeemSubscriptionVoucher('foo456', owner, owner, 12,
+ >>> bzr.redeemSubscriptionVoucher('foo5456', owner, owner, 12,
... 'notes', current_datetime=now)
>>> print subscription.date_expires
2011-01-01...
@@ -198,7 +204,7 @@
>>> subscription.date_starts = datetime(2008,1,15,tzinfo=UTC)
>>> subscription.date_expires = datetime(2009,1,15,tzinfo=UTC)
>>> now = datetime(2009, 1, 1, tzinfo=UTC)
- >>> bzr.redeemSubscriptionVoucher('foo456', owner, owner, 12,
+ >>> bzr.redeemSubscriptionVoucher('foo6456', owner, owner, 12,
... 'notes', current_datetime=now)
>>> print subscription.date_expires
2010-01-15...
=== modified file 'lib/lp/registry/errors.py'
--- lib/lp/registry/errors.py 2012-10-19 16:54:35 +0000
+++ lib/lp/registry/errors.py 2012-10-22 03:05:23 +0000
@@ -28,6 +28,7 @@
'TeamMembershipPolicyError',
'UserCannotChangeMembershipSilently',
'UserCannotSubscribePerson',
+ 'VoucherAlreadyRedeemed',
]
import httplib
@@ -207,3 +208,7 @@
class CannotPackageProprietaryProduct(Exception):
"""Raised when a non-PUBLIC product's series is linked to a package."""
+
+
+class VoucherAlreadyRedeemed(Exception):
+ """Raised when a voucher is redeemed more than once."""
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2012-10-19 16:54:35 +0000
+++ lib/lp/registry/model/person.py 2012-10-22 03:05:23 +0000
@@ -1137,14 +1137,28 @@
def getRedeemableCommercialSubscriptionVouchers(self, voucher_proxy=None):
"""See `IPerson`."""
+ # Circular imports.
+ from lp.registry.model.commercialsubscription import (
+ CommercialSubscription,
+ )
if voucher_proxy is None:
voucher_proxy = getUtility(ISalesforceVoucherProxy)
vouchers = voucher_proxy.getUnredeemedVouchers(self)
- for voucher in vouchers:
+ # Exclude pending vouchers being sent to Salesforce and vouchers which
+ # have already been redeemed.
+ voucher_ids = [unicode(voucher.voucher_id) for voucher in vouchers]
+ voucher_expr = (
+ "trim(leading 'pending-' "
+ "from CommercialSubscription.sales_system_id)")
+ already_redeemed = list(Store.of(self).using(CommercialSubscription)
+ .find(SQL(voucher_expr), SQL(voucher_expr).is_in(voucher_ids)))
+ redeemable_vouchers = [voucher for voucher in vouchers
+ if voucher.voucher_id not in already_redeemed]
+ for voucher in redeemable_vouchers:
assert voucher.status in REDEEMABLE_VOUCHER_STATUSES, (
"Voucher %s has invalid status %s" %
(voucher.voucher_id, voucher.status))
- return vouchers
+ return redeemable_vouchers
def hasCurrentCommercialSubscription(self):
"""See `IPerson`."""
=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py 2012-10-19 16:54:35 +0000
+++ lib/lp/registry/model/product.py 2012-10-22 03:05:23 +0000
@@ -138,6 +138,7 @@
from lp.registry.errors import (
CannotChangeInformationType,
CommercialSubscribersOnly,
+ VoucherAlreadyRedeemed,
)
from lp.registry.interfaces.accesspolicy import (
IAccessPolicyArtifactSource,
@@ -753,6 +754,19 @@
day=new_day)
return new_date
+ # The voucher may already have been redeemed or marked as redeemed
+ # pending notification being sent to Salesforce.
+ voucher_expr = (
+ "trim(leading 'pending-' "
+ "from CommercialSubscription.sales_system_id)")
+ already_redeemed = Store.of(self).find(
+ CommercialSubscription,
+ SQL(voucher_expr) == unicode(voucher)).any()
+ if already_redeemed:
+ raise VoucherAlreadyRedeemed(
+ "Voucher %s has already been redeemed for %s"
+ % (voucher, already_redeemed.product.displayname))
+
if current_datetime is None:
current_datetime = datetime.datetime.now(pytz.timezone('UTC'))
=== modified file 'lib/lp/registry/stories/vouchers/xx-voucher-redemption.txt'
--- lib/lp/registry/stories/vouchers/xx-voucher-redemption.txt 2012-10-19 16:54:35 +0000
+++ lib/lp/registry/stories/vouchers/xx-voucher-redemption.txt 2012-10-22 03:05:23 +0000
@@ -142,51 +142,3 @@
>>> for message in get_feedback_messages(browser.contents):
... print message
Voucher redeemed successfully
-
-
-OOPS handling
--------------
-
-If an error occurs in the proxy while trying to redeem a voucher an
-OOPS is recorded but an error is not raised. The user is shown an
-error message instead.
-
-Set the canned fault.
-
- >>> from lp.services.salesforce.tests.proxy import (
- ... SalesforceXMLRPCTestTransport)
- >>> SalesforceXMLRPCTestTransport.forced_fault = (
- ... 'redeemVoucher', 'NotFound','Something was not found.')
-
-Log in as a user with lots of vouchers.
-
- >>> browser = setupBrowser(auth='Basic bac@xxxxxxxxxxxxx:test')
- >>> browser.open('http://launchpad.dev/~bac/+vouchers')
-
-Attempt to redeem a valid voucher and see the error message.
-
- >>> from lp.testing.fixture import CaptureOops
- >>> capture = CaptureOops()
- >>> capture.setUp()
- >>> browser.getControl(name='field.project').value='mega-money-maker'
- >>> browser.getControl(name='field.voucher').value = [
- ... 'LPCBS12-f78df324-0cc2-11dd-8b6b-bac000000001']
- >>> browser.getControl('Redeem').click()
- >>> for message in get_feedback_messages(browser.contents):
- ... print message
- There is 1 error.
- The voucher could not be redeemed at this time.
-
-The OOPS report shows the error was logged even though the OOPS was
-not shown to the user.
-
- >>> report = capture.oopses[0]
- >>> capture.cleanUp()
- >>> report['type']
- 'SVPNotFoundException'
- >>> report['value']
- u'Something was not found.'
-
-Reset the class variable to disable forced faults.
-
- >>> SalesforceXMLRPCTestTransport.forced_fault = None
=== modified file 'lib/lp/registry/templates/person-vouchers.pt'
--- lib/lp/registry/templates/person-vouchers.pt 2012-10-19 16:54:35 +0000
+++ lib/lp/registry/templates/person-vouchers.pt 2012-10-22 03:05:23 +0000
@@ -21,8 +21,9 @@
vouchers for a project.
</tal:not_commercial>
<tal:commercial condition="commercial_projects">
- <tal:define_vouchers define="vouchers view/redeemable_vouchers" >
- <tal:no_vouchers condition="not: vouchers">
+ <tal:define_vouchers
+ define="vouchers view/redeemable_vouchers" >
+ <tal:no_vouchers condition="not: view/show_voucher_selection">
<p>
<tal:name replace="context/fmt:displayname" />
does not have any redeemable vouchers available.</p>
@@ -32,7 +33,7 @@
/>
</tal:no_vouchers>
- <tal:has_voucher condition="vouchers">
+ <tal:has_voucher condition="view/show_voucher_selection">
<div metal:use-macro="context/@@launchpad_form/form">
<metal:widgets fill-slot="widgets">
<table class="form">
=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py 2012-10-19 16:54:35 +0000
+++ lib/lp/scripts/garbo.py 2012-10-22 03:05:23 +0000
@@ -29,6 +29,7 @@
import pytz
from storm.expr import (
In,
+ Like,
Select,
Update,
)
@@ -60,6 +61,7 @@
RevisionCache,
)
from lp.hardwaredb.model.hwdb import HWSubmission
+from lp.registry.model.commercialsubscription import CommercialSubscription
from lp.registry.model.person import Person
from lp.registry.model.product import Product
from lp.services.config import config
@@ -92,6 +94,10 @@
from lp.services.oauth.model import OAuthNonce
from lp.services.openid.model.openidconsumer import OpenIDConsumerNonce
from lp.services.propertycache import cachedproperty
+from lp.services.salesforce.interfaces import (
+ ISalesforceVoucherProxy,
+ SalesforceVoucherProxyException,
+ )
from lp.services.scripts.base import (
LaunchpadCronScript,
LOCK_PATH,
@@ -365,6 +371,58 @@
self.store.commit()
+class VoucherRedeemer(TunableLoop):
+ """Redeem pending sales vouchers with Salesforce."""
+ maximum_chunk_size = 5
+
+ voucher_expr = (
+ "trim(leading 'pending-' "
+ "from CommercialSubscription.sales_system_id)")
+
+ def __init__(self, log, abort_time=None):
+ super(VoucherRedeemer, self).__init__(log, abort_time)
+ self.store = getUtility(IStoreSelector).get(MAIN_STORE, MASTER_FLAVOR)
+
+ @cachedproperty
+ def _salesforce_proxy(self):
+ return getUtility(ISalesforceVoucherProxy)
+
+ @property
+ def _pending_subscriptions(self):
+ return self.store.find(
+ CommercialSubscription,
+ Like(CommercialSubscription.sales_system_id, u'pending-%')
+ )
+
+ def isDone(self):
+ return self._pending_subscriptions.count() == 0
+
+ def __call__(self, chunk_size):
+ successful_ids = []
+ for sub in self._pending_subscriptions[:chunk_size]:
+ sales_system_id = sub.sales_system_id[len('pending-'):]
+ try:
+ # The call to redeemVoucher returns True if it succeeds or it
+ # raises an exception. Therefore the return value does not
+ # need to be checked.
+ self._salesforce_proxy.redeemVoucher(
+ sales_system_id, sub.purchaser, sub.product)
+ successful_ids.append(unicode(sub.sales_system_id))
+ except SalesforceVoucherProxyException as error:
+ self.log.error(
+ "Failed to redeem voucher %s: %s"
+ % (sales_system_id, error.message))
+ # Update the successfully redeemed voucher ids to be not pending.
+ if successful_ids:
+ self.store.find(
+ CommercialSubscription,
+ CommercialSubscription.sales_system_id.is_in(successful_ids)
+ ).set(
+ CommercialSubscription.sales_system_id ==
+ SQL(self.voucher_expr))
+ transaction.commit()
+
+
class OpenIDConsumerNoncePruner(TunableLoop):
"""An ITunableLoop to prune old OpenIDConsumerNonce records.
@@ -1280,6 +1338,7 @@
OpenIDConsumerNoncePruner,
OpenIDConsumerAssociationPruner,
AntiqueSessionPruner,
+ VoucherRedeemer,
]
experimental_tunable_loops = []
=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py 2012-10-19 16:54:35 +0000
+++ lib/lp/scripts/tests/test_garbo.py 2012-10-22 03:05:23 +0000
@@ -17,6 +17,7 @@
from pytz import UTC
from storm.expr import (
In,
+ Like,
Min,
Not,
SQL,
@@ -59,6 +60,7 @@
)
from lp.registry.interfaces.accesspolicy import IAccessPolicySource
from lp.registry.interfaces.person import IPersonSet
+from lp.registry.model.commercialsubscription import CommercialSubscription
from lp.registry.model.product import Product
from lp.scripts.garbo import (
AntiqueSessionPruner,
@@ -97,6 +99,8 @@
OAuthNonce,
)
from lp.services.openid.model.openidconsumer import OpenIDConsumerNonce
+from lp.services.salesforce.interfaces import ISalesforceVoucherProxy
+from lp.services.salesforce.tests.proxy import TestSalesforceVoucherProxy
from lp.services.scripts.tests import run_script
from lp.services.session.model import (
SessionData,
@@ -106,6 +110,7 @@
from lp.services.verification.model.logintoken import LoginToken
from lp.services.worlddata.interfaces.language import ILanguageSet
from lp.testing import (
+ FakeAdapterMixin,
person_logged_in,
TestCase,
TestCaseWithFactory,
@@ -370,7 +375,7 @@
self.assertEqual(expected_sessions, found_sessions)
-class TestGarbo(TestCaseWithFactory):
+class TestGarbo(FakeAdapterMixin, TestCaseWithFactory):
layer = LaunchpadZopelessLayer
def setUp(self):
@@ -966,6 +971,36 @@
"SELECT COUNT(*) FROM BugSummaryJournal").get_one()[0]
self.assertThat(num_rows, Equals(0))
+ def test_VoucherRedeemer(self):
+ switch_dbuser('testadmin')
+ store = getUtility(IStoreSelector).get(MAIN_STORE, MASTER_FLAVOR)
+
+ voucher_proxy = TestSalesforceVoucherProxy()
+ self.registerUtility(voucher_proxy, ISalesforceVoucherProxy)
+
+ # Mark has some unredeemed vouchers so set one of them as pending.
+ mark = getUtility(IPersonSet).getByName('mark')
+ voucher = voucher_proxy.getUnredeemedVouchers(mark)[0]
+ product = self.factory.makeProduct(owner=mark)
+ redeemed_id = voucher.voucher_id
+ self.factory.makeCommercialSubscription(
+ product, False, 'pending-%s' % redeemed_id)
+ transaction.commit()
+
+ self.runFrequently()
+
+ # There should now be 0 pending vouchers in Launchpad.
+ num_rows = store.find(
+ CommercialSubscription,
+ Like(CommercialSubscription.sales_system_id, u'pending-%')
+ ).count()
+ self.assertThat(num_rows, Equals(0))
+ # Salesforce should also now have redeemed the voucher.
+ unredeemed_ids = [
+ voucher.voucher_id
+ for voucher in voucher_proxy.getUnredeemedVouchers(mark)]
+ self.assertNotIn(redeemed_id, unredeemed_ids)
+
def test_UnusedPOTMsgSetPruner_removes_obsolete_message_sets(self):
# UnusedPOTMsgSetPruner removes any POTMsgSet that are
# participating in a POTemplate only as obsolete messages.
=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf 2012-10-09 00:09:15 +0000
+++ lib/lp/services/config/schema-lazr.conf 2012-10-22 03:05:23 +0000
@@ -431,6 +431,11 @@
# datatype: integer
voucher_proxy_port:
+# Salesforce proxy timeout in milliseconds. Calls taking longer than this value
+# will be aborted.
+# datatype: integer
+voucher_proxy_timeout: 60000
+
# URL for purchasing a commercial subscription.
# datatype: string; a url
purchase_subscription_url:
=== modified file 'lib/lp/services/salesforce/proxy.py'
--- lib/lp/services/salesforce/proxy.py 2012-10-17 12:51:44 +0000
+++ lib/lp/services/salesforce/proxy.py 2012-10-22 03:05:23 +0000
@@ -93,7 +93,8 @@
implements(ISalesforceVoucherProxy)
def __init__(self):
- self.xmlrpc_transport = SafeTransportWithTimeout()
+ self.xmlrpc_transport = SafeTransportWithTimeout(
+ config.commercial.voucher_proxy_timeout / 1000.0)
@cachedproperty
def url(self):
=== modified file 'lib/lp/services/timeout.py'
--- lib/lp/services/timeout.py 2012-06-28 16:05:05 +0000
+++ lib/lp/services/timeout.py 2012-10-22 03:05:23 +0000
@@ -116,7 +116,15 @@
"""Wraps the method."""
def call_with_timeout(*args, **kwargs):
# Ensure that we have a timeout before we start the thread
- timeout = self.timeout
+ if getattr(self.timeout, '__call__', None):
+ # timeout may be a method or a function on the calling
+ # instance class.
+ if args:
+ timeout = self.timeout(args[0])
+ else:
+ timeout = self.timeout()
+ else:
+ timeout = self.timeout
t = ThreadCapturingResult(f, args, kwargs)
t.start()
t.join(timeout)
@@ -215,12 +223,19 @@
class SafeTransportWithTimeout(SafeTransport):
"""Create a HTTPS transport for XMLRPC with timeouts."""
+ timeout = None
+
+ def __init__(self, timeout=None):
+ # Old style class call to super required.
+ SafeTransport.__init__(self)
+ self.timeout = timeout
+
def make_connection(self, host):
"""Create the connection for the transport and save it."""
self.conn = SafeTransport.make_connection(self, host)
return self.conn
- @with_timeout(cleanup='cleanup')
+ @with_timeout(cleanup='cleanup', timeout=lambda self: self.timeout)
def request(self, host, handler, request_body, verbose=0):
"""Make the request but using the with_timeout decorator."""
return SafeTransport.request(
=== modified file 'lib/lp/services/webapp/doc/timeout.txt'
--- lib/lp/services/webapp/doc/timeout.txt 2011-12-30 08:13:14 +0000
+++ lib/lp/services/webapp/doc/timeout.txt 2012-10-22 03:05:23 +0000
@@ -65,6 +65,32 @@
...
RequestExpired: request expired.
+@with_timeout allows the actual timeout value to be specified, either as a
+numeric argument or a function argument returning the required value. Here we
+specify a timeout of 2 seconds to allow the called function to complete
+successfully.
+
+ >>> @with_timeout(timeout=2)
+ ... def wait_a_little_more():
+ ... time.sleep(1)
+ >>> wait_a_little_more()
+
+ >>> def _timeout():
+ ... return 2
+ >>> @with_timeout(timeout=_timeout)
+ ... def wait_a_little_again():
+ ... time.sleep(1)
+ >>> wait_a_little_again()
+
+ >>> class Foo:
+ ... @property
+ ... def _timeout(self):
+ ... return 2
+ ... @with_timeout(timeout=lambda self: self._timeout)
+ ... def wait_a_little(self):
+ ... time.sleep(1)
+ >>> Foo().wait_a_little()
+
If there is no db_statement_timeout, then the default timeout is None
and a TimeoutError is never raised.
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2012-10-19 16:54:35 +0000
+++ lib/lp/testing/factory.py 2012-10-22 03:05:23 +0000
@@ -4266,7 +4266,8 @@
}
return fileupload
- def makeCommercialSubscription(self, product, expired=False):
+ def makeCommercialSubscription(self, product, expired=False,
+ voucher_id='new'):
"""Create a commercial subscription for the given product."""
if CommercialSubscription.selectOneBy(product=product) is not None:
raise AssertionError(
@@ -4281,7 +4282,7 @@
date_expires=expiry,
registrant=product.owner,
purchaser=product.owner,
- sales_system_id='new',
+ sales_system_id=voucher_id,
whiteboard='')
def grantCommercialSubscription(self, person, months=12):
Follow ups
