diff --git a/conftest.py b/conftest.py deleted file mode 100644 index a9b6707..0000000 --- a/conftest.py +++ /dev/null @@ -1,14 +0,0 @@ -import pytest -from api import payments - - -@pytest.fixture(scope="module") -def create_customer_for_processing(): - customer = payments.create_customer('tok_visa', 'process_customer', 'test_fixture@tester.com') - yield customer - - -@pytest.fixture(scope="module") -def create_subscription_for_processing(create_customer_for_processing): - subscription = payments.subscribe_to_plan('subscribe_test', {"pmt_token": 'tok_visa', "plan_id": 'plan_EtMcOlFMNWW4nd', "email": 'subscribe_test@tester.com'}) - yield subscription diff --git a/services/subhub/serverless.yml b/services/subhub/serverless.yml index 3cd0549..230d5bc 100644 --- a/services/subhub/serverless.yml +++ b/services/subhub/serverless.yml @@ -13,7 +13,6 @@ provider: runtime: python3.7 region: us-west-2 stage: ${opt:stage, self:custom.defaultStage} - tableName: ${self:custom.tableName} memorySize: 256 timeout: 10 tracing: true @@ -28,6 +27,8 @@ provider: - "dynamodb:PutItem" - "dynamodb:UpdateItem" - "dynamodb:DeleteItem" + - "dynamodb:DescribeTable" + - "dynamodb:CreateTable" Resource: - { "Fn::GetAtt": ["SubhubDynamoDBTable", "Arn" ] } - Effect: Allow @@ -54,8 +55,6 @@ provider: - "xray:PutTelemetryRecords" Resource: - "*" - environment: - SUBHUB_ACCT: ${self:custom.tableName} package: exclude: @@ -71,7 +70,6 @@ custom: dockerizePip: 'non-linux' fileName: subhub/requirements.txt git-repo: https://github.com/mozilla/subhub - tableName: 'subhub-acct-table-${self:provider.stage}' dynamodb: start: migrate: true @@ -94,7 +92,8 @@ functions: cors: true environment: STAGE: ${self:provider.stage} - SUBHUB_TABLE: ${self:provider.tableName} + USER_TABLE: + Ref: "SubhubDynamoDBTable" resources: Resources: @@ -110,4 +109,3 @@ resources: AttributeName: userId KeyType: HASH BillingMode: PAY_PER_REQUEST - TableName: ${self:custom.tableName} diff --git a/setup.py b/setup.py index abe31e7..6a46e5e 100644 --- a/setup.py +++ b/setup.py @@ -20,6 +20,7 @@ requirements = [ 'urlpath', 'pathlib2', 'packaging', + 'pynamodb', 'virtualenv', 'Werkzeug', 'sh', diff --git a/subhub/api/payments.py b/subhub/api/payments.py index cbd6c8a..640150e 100644 --- a/subhub/api/payments.py +++ b/subhub/api/payments.py @@ -3,7 +3,7 @@ from typing import Optional import stripe from flask import g, app -from stripe.error import InvalidRequestError as StripeInvalidRequest +from stripe.error import InvalidRequestError as StripeInvalidRequest, APIConnectionError, APIError, AuthenticationError, CardError from subhub.cfg import CFG from subhub.secrets import get_secret @@ -27,6 +27,7 @@ def create_customer(source_token, userid, email): :param email: user's email :return: Stripe Customer """ + # TODO Add error handlers and static typing try: customer = stripe.Customer.create( source=source_token, @@ -35,6 +36,7 @@ def create_customer(source_token, userid, email): metadata={'userid': userid} ) return customer + # TODO: Verify that this is the only error we need to worry about except stripe.error.InvalidRequestError as e: return str(e) @@ -46,6 +48,7 @@ def subscribe_customer(customer, plan): :param plan: :return: Subscription Object """ + # TODO Add error handlers and static typing try: subscription = stripe.Subscription.create( customer=customer, @@ -75,8 +78,6 @@ def existing_or_new_subscriber(uid, data): else: existing_or_new_subscriber(uid, data) if not subscription_user.custId: - if data['email'] is None: - return 'Missing email parameter.', 400 customer = create_customer(data['pmt_token'], uid, data['email']) if 'No such token:' in customer: return 'Token not valid', 400 @@ -84,12 +85,12 @@ def existing_or_new_subscriber(uid, data): if not update_successful: return "Customer not saved successfully.", 400 updated_user = g.subhub_account.get_user(uid) - return updated_user + return updated_user, 200 else: - return subscription_user + return subscription_user, 200 -def has_existing_plan(user, data): +def has_existing_plan(user, data) -> bool: """ Check if user has the existing plan in an active or trialing state. :param user: @@ -103,14 +104,16 @@ def has_existing_plan(user, data): return False -def subscribe_to_plan(uid, data): +def subscribe_to_plan(uid, data) -> tuple: """ Subscribe to a plan given a user id, payment token, email, orig_system :param uid: :param data: :return: current subscriptions for user. """ - sub_user = existing_or_new_subscriber(uid, data) + sub_user, code = existing_or_new_subscriber(uid, data) + if code == 400: + return "Customer issue.", 400 existing_plan = has_existing_plan(sub_user, data) if existing_plan: return "User already subscribed.", 400 @@ -124,7 +127,7 @@ def subscribe_to_plan(uid, data): return return_data, 201 -def list_all_plans(): +def list_all_plans() -> tuple: """ List all available plans for a user to purchase. :return: @@ -137,17 +140,13 @@ def list_all_plans(): return stripe_plans, 200 -def cancel_subscription(uid, sub_id): +def cancel_subscription(uid, sub_id) -> tuple: """ Cancel an existing subscription for a user. :param uid: :param sub_id: :return: Success or failure message for the cancellation. """ - if not isinstance(uid, str): - return 'Invalid ID', 400 - if not isinstance(sub_id, str): - return 'Invalid Subscription ', 400 # TODO Remove payment source on cancel subscription_user = g.subhub_account.get_user(uid) if not subscription_user: @@ -158,6 +157,7 @@ def cancel_subscription(uid, sub_id): try: tocancel = stripe.Subscription.retrieve(sub_id) except StripeInvalidRequest as e: + # TODO handle other errors: APIConnectionError, APIError, AuthenticationError, CardError return {"message": e}, 400 if 'No such subscription:' in tocancel: return 'Invalid subscription.', 400 @@ -170,14 +170,12 @@ def cancel_subscription(uid, sub_id): return {"message": 'Subscription not available.'}, 400 -def subscription_status(uid): +def subscription_status(uid) -> tuple: """ Given a user id return the current subscription status :param uid: :return: Current subscriptions """ - if not isinstance(uid, str): - return 'Invalid ID', 400 items = g.subhub_account.get_user(uid) if not items or not items.custId: return 'Customer does not exist.', 404 @@ -188,7 +186,7 @@ def subscription_status(uid): return return_data, 201 -def create_return_data(subscriptions): +def create_return_data(subscriptions) -> dict: """ Create json object subscriptions object :param subscriptions: @@ -197,16 +195,6 @@ def create_return_data(subscriptions): return_data = dict() return_data['subscriptions'] = [] for subscription in subscriptions["data"]: - ended_val = 'None' - if subscription['ended_at']: - ended_val = str(subscription['ended_at']) - item = {'subscription_id': subscription['id'], - 'current_period_end': str(subscription['current_period_end']), - 'current_period_start': str(subscription['current_period_start']), - 'plan_id': subscription['plan']['id'], - 'ended_at': ended_val, - 'status': subscription['status'], - 'nickname': subscription['plan']['nickname']} return_data['subscriptions'].append({ 'current_period_end': subscription['current_period_end'], 'current_period_start': subscription['current_period_start'], @@ -218,17 +206,13 @@ def create_return_data(subscriptions): return return_data -def update_payment_method(uid, data): +def update_payment_method(uid, data) -> tuple: """ Given a user id and a payment token, update user's payment method :param uid: :param data: :return: Success or failure message. """ - if not isinstance(data['pmt_token'], str): - return 'Missing token', 400 - if not isinstance(uid, str): - return 'Missing or invalid user.', 400 items = g.subhub_account.get_user(uid) if not items or not items.custId: return 'Customer does not exist.', 404 @@ -239,6 +223,7 @@ def update_payment_method(uid, data): customer.modify(items.custId, source=data['pmt_token']) return 'Payment method updated successfully.', 201 except StripeInvalidRequest as e: + # TODO handle other errors: APIConnectionError, APIError, AuthenticationError, CardError return str(e), 400 else: return 'Customer mismatch.', 400 @@ -246,14 +231,12 @@ def update_payment_method(uid, data): return f'Customer does not exist: missing {e}', 404 -def customer_update(uid): +def customer_update(uid) -> tuple: """ Provide latest data for a given user :param uid: :return: return_data dict with credit card info and subscriptions """ - if not isinstance(uid, str): - return 'Invalid ID', 400 items = g.subhub_account.get_user(uid) if not items or not items.custId: return 'Customer does not exist.', 404 @@ -268,7 +251,7 @@ def customer_update(uid): return f'Customer does not exist: missing {e}', 404 -def create_update_data(customer): +def create_update_data(customer) -> dict: """ Provide readable data for customer update to display :param customer: diff --git a/subhub/app.py b/subhub/app.py index 375fe49..270eb12 100644 --- a/subhub/app.py +++ b/subhub/app.py @@ -1,4 +1,4 @@ -from flask import Flask, request, jsonify, render_template, url_for, logging, g +from flask import Flask, request, jsonify, render_template, url_for, logging, g, current_app from flask_cors import CORS # from flask.views import MethodView import connexion @@ -17,16 +17,22 @@ def create_app(config=None): else: options = {"swagger_ui": False} region = 'us-west-2' + host = None - print(f'options {options}') app = connexion.FlaskApp(__name__, specification_dir='./', options=options) app.add_api('subhub_api.yaml', pass_context_arg_name='request', strict_validation=True) + + if host: + app.app.subhub_account = SubHubAccount(table_name=CFG.USER_TABLE, region=region, host=host) + else: + app.app.subhub_account = SubHubAccount(table_name=CFG.USER_TABLE, region=region) + if not app.app.subhub_account.model.exists(): + app.app.subhub_account.model.create_table(read_capacity_units=1, write_capacity_units=1, wait=True) + @app.app.before_request def before_request(): - g.subhub_account = SubHubAccount(table_name = CFG.SUBHUB_TABLE, region=region, host=host) - if not g.subhub_account.model.exists(): - g.subhub_account.model.create_table(read_capacity_units=1, write_capacity_units=1, wait=True) + g.subhub_account = current_app.subhub_account CORS(app.app) return app diff --git a/subhub/requirements.txt b/subhub/requirements.txt index 411f643..16f578e 100644 --- a/subhub/requirements.txt +++ b/subhub/requirements.txt @@ -18,7 +18,7 @@ jsonschema==2.6.0 MarkupSafe==1.1.1 openapi-spec-validator==0.2.6 pathlib==1.0.1 -pynamodb=3.3.3 +pynamodb==3.3.3 python-dateutil==2.8.0 python-decouple==3.1 PyYAML==5.1 diff --git a/subhub/subhub_api.yaml b/subhub/subhub_api.yaml index b91922e..c6f0abe 100644 --- a/subhub/subhub_api.yaml +++ b/subhub/subhub_api.yaml @@ -82,6 +82,7 @@ paths: - pmt_token - plan_id - email + - orig_system properties: pmt_token: type: string diff --git a/subhub/subhub_dynamodb.py b/subhub/subhub_dynamodb.py index cb2a49b..be571ca 100644 --- a/subhub/subhub_dynamodb.py +++ b/subhub/subhub_dynamodb.py @@ -31,7 +31,7 @@ class SubHubAccount(): orig_system = UnicodeAttribute() self.model = SubHubAccountModel - def get_user(self, uid): + def get_user(self, uid) -> SubHubAccountModel: try: subscription_user = self.model.get(uid, consistent_read=True) return subscription_user @@ -46,7 +46,7 @@ class SubHubAccount(): except PutError: return False - def append_custid(self, uid, custId): + def append_custid(self, uid, custId) -> bool: try: update_user = self.model.get(uid, consistent_read=True) update_user.custId = custId @@ -55,9 +55,9 @@ class SubHubAccount(): except DoesNotExist: return False - def remove_from_db(self, uid): + def remove_from_db(self, uid) -> bool: try: self.model.get(uid, consistent_read=True).delete() - return 'User deleted', 200 + return True except DoesNotExist as e: - return e, 404 + False diff --git a/subhub/tests/conftest.py b/subhub/tests/conftest.py index b6ce828..aa39d9c 100644 --- a/subhub/tests/conftest.py +++ b/subhub/tests/conftest.py @@ -1,11 +1,21 @@ import pytest -from api import payments +from flask import g + +from subhub.api import payments +from subhub.app import create_app + + +@pytest.fixture(scope="module") +def app(): + app = create_app() + with app.app.app_context(): + g.subhub_account = app.app.subhub_account + yield app @pytest.fixture(scope="module") def create_customer_for_processing(): customer = payments.create_customer('tok_visa', 'process_customer', 'test_fixture@tester.com') - print(f'customer {customer}') yield customer @pytest.fixture(scope="function") @@ -14,4 +24,4 @@ def create_subscription_for_processing(): "plan_id": "plan_EtMcOlFMNWW4nd", "orig_system": "Test_system", "email": "subtest@tester.com"}) - yield subscription \ No newline at end of file + yield subscription diff --git a/subhub/tests/unit/test_stripe_calls.py b/subhub/tests/unit/test_stripe_calls.py index f9f1d40..14c71b5 100644 --- a/subhub/tests/unit/test_stripe_calls.py +++ b/subhub/tests/unit/test_stripe_calls.py @@ -1,6 +1,7 @@ -from subhub import subhub_dynamodb -from api import payments +from flask import g +from subhub.api import payments import pytest +from subhub import app def test_create_customer_tok_visa(): @@ -72,85 +73,33 @@ def test_subscribe_customer_invalid_plan(create_customer_for_processing): assert 'No such plan: plan_notvalid' in subscription -def test_create_subscription_with_valid_data(create_customer_for_processing): +def test_create_subscription_with_valid_data(app): """ GIVEN create a subscription WHEN provided a api_token, userid, pmt_token, plan_id, cust_id THEN validate subscription is created """ - customer = create_customer_for_processing - subscription = payments.subscribe_to_plan('process_customer', {"pmt_token": 'tok_visa', - "plan_id": 'plan_EtMcOlFMNWW4nd', - "email": customer['email'], - "orig_system": "Test_system"}) - print(f'subscription {subscription}') - # assert 201 in subscription - # stripe_calls.cancel_subscription('process_customer', subscription['subscriptions'][0]["subscription_id"]) - subhub_dynamodb.remove_from_db('process_customer') + subscription, code = payments.subscribe_to_plan('validcustomer', {"pmt_token": 'tok_visa', + "plan_id": 'plan_EtMcOlFMNWW4nd', + "email": 'valid@customer.com', + "orig_system": "Test_system"}) + assert 201 == code + payments.cancel_subscription('validcustomer', subscription['subscriptions'][0]["subscription_id"]) + g.subhub_account.remove_from_db('validcustomer') -def test_create_subscription_with_missing_fxa_id(create_customer_for_processing): - """ - GIVEN should not create a subscription - WHEN provided a api_token, no userid, pmt_token, plan_id, email - THEN validate subscription is created - """ - customer = create_customer_for_processing - subscription = payments.subscribe_to_plan(None, {"pmt_token": 'tok_visa', "plan_id": 'plan_EtMcOlFMNWW4nd', "email": customer['email'], "orig_system": "Test_system"}) - assert 400 in subscription - - -def test_create_subscription_with_invalid_fxa_id(create_customer_for_processing): - """ - GIVEN should not create a subscription - WHEN provided a api_token, invalid userid, pmt_token, plan_id, email - THEN validate subscription is created - """ - customer = create_customer_for_processing - subscription = payments.subscribe_to_plan(123, {"pmt_token": 'tok_visa', "plan_id": 'plan_EtMcOlFMNWW4nd', "email": customer['email'], "orig_system": "Test_system"}) - assert 400 in subscription - - -def test_create_subscription_with_missing_payment_token(): +def test_create_subscription_with_invalid_payment_token(app): """ GIVEN should not create a subscription WHEN provided a api_token, userid, invalid pmt_token, plan_id, email THEN validate subscription is created """ - subscription = payments.subscribe_to_plan('123456', {"pmt_token": 'tok_invalid', "plan_id": 'plan_EtMcOlFMNWW4nd', "email": 'invalid_test@test.com', "orig_system": "Test_system"}) - assert 400 in subscription - subhub_dynamodb.remove_from_db('123456') + subscription, code = payments.subscribe_to_plan('invalid_test', {"pmt_token": 'tok_invalid', "plan_id": 'plan_EtMcOlFMNWW4nd', "email": 'invalid_test@test.com', "orig_system": "Test_system"}) + assert 400 == code + g.subhub_account.remove_from_db('invalid_test') -def test_create_subscription_with_invalid_payment_token(): - """ - GIVEN should not create a subscription - WHEN provided a api_token, userid, invalid pmt_token, plan_id, email - THEN validate subscription is created - """ - subscription = payments.subscribe_to_plan('12345', {"pmt_token": 1234, - "plan_id": 'plan_EtMcOlFMNWW4nd', - "email": 'invalid_test@test.com', - "orig_system": "Test_system"}) - assert 400 in subscription - subhub_dynamodb.remove_from_db('12345') - - -def test_create_subscription_with_missing_plan_id(): - """ - GIVEN should not create a subscription - WHEN provided a api_token, userid, pmt_token, missing plan_id, email - THEN validate subscription is created - """ - subscription = payments.subscribe_to_plan('missing_plan', {"pmt_token": 'tok_visa', - "plan_id": None, - "email": 'missing_plan@tester.com', - "orig_system": "Test_system"}) - assert 400 in subscription - subhub_dynamodb.remove_from_db('missing_plan') - - -def test_create_subscription_with_invalid_plan_id(): +def test_create_subscription_with_invalid_plan_id(app): """ GIVEN should not create a subscription WHEN provided a api_token, userid, pmt_token, invalid plan_id, email @@ -161,21 +110,7 @@ def test_create_subscription_with_invalid_plan_id(): "email": 'invalid_plan@tester.com', "orig_system": "Test_system"}) assert 400 in subscription - subhub_dynamodb.remove_from_db('invalid_plan') - - -def test_create_subscription_with_missing_email_id(): - """ - GIVEN should not create a subscription - WHEN provided a api_token, userid, pmt_token, plan_id, missing email - THEN validate subscription is created - """ - subscription = payments.subscribe_to_plan('missing_email', {"pmt_token": 'tok_visa', - "plan_id": 'plan_EtMcOlFMNWW4nd', - "email": None, - "orig_system": "Test_system"}) - assert 400 in subscription - subhub_dynamodb.remove_from_db('missing_email') + g.subhub_account.remove_from_db('invalid_plan') def test_list_all_plans_valid(): @@ -189,7 +124,7 @@ def test_list_all_plans_valid(): assert 200 == code -def test_cancel_subscription_with_valid_data(create_subscription_for_processing): +def test_cancel_subscription_with_valid_data(app, create_subscription_for_processing): """ GIVEN should cancel an active subscription WHEN provided a api_token, and subscription id @@ -197,22 +132,12 @@ def test_cancel_subscription_with_valid_data(create_subscription_for_processing) """ (subscription, code) = create_subscription_for_processing (cancelled, code) = payments.cancel_subscription('process_test', subscription['subscriptions'][0]['subscription_id']) - assert cancelled['status'] == 'canceled' + assert cancelled['message'] == 'Subscription cancellation successful' assert 201 == code - subhub_dynamodb.remove_from_db('process_test') + g.subhub_account.remove_from_db('process_test') -def test_cancel_subscription_with_missing_subscription_id(): - """ - GIVEN should not cancel an active subscription - WHEN provided a api_token, and missing subscription id - THEN validate should not cancel subscription - """ - (cancelled, code) = payments.cancel_subscription('process_test', None) - assert 400 == code - - -def test_check_subscription_with_valid_parameters(create_subscription_for_processing): +def test_check_subscription_with_valid_parameters(app, create_subscription_for_processing): """ GIVEN should get a list of active subscriptions WHEN provided an api_token and a userid id @@ -222,33 +147,10 @@ def test_check_subscription_with_valid_parameters(create_subscription_for_proces (sub_status, code) = payments.subscription_status('process_test') assert 201 == code assert len(sub_status) > 0 - subhub_dynamodb.remove_from_db('process_test') + g.subhub_account.remove_from_db('process_test') - -def test_check_subscription_with_missing_fxa_id(): - """ - GIVEN should not get a list of active subscriptions - WHEN provided an api_token and a missing userid id - THEN validate should not return list of active subscriptions - """ - (sub_status, code) = payments.subscription_status(None) - assert 400 == code - assert 'Invalid ID' in sub_status - - -def test_check_subscription_with_invalid_fxa_id(): - """ - GIVEN should not get a list of active subscriptions - WHEN provided an api_token and an invalid userid id - THEN validate should not return list of active subscriptions - """ - (sub_status, code) = payments.subscription_status(42) - assert 400 == code - assert 'Invalid ID' in sub_status - - -def test_update_payment_method_valid_parameters(create_subscription_for_processing): +def test_update_payment_method_valid_parameters(app, create_subscription_for_processing): """ GIVEN api_token, userid, pmt_token WHEN all parameters are valid @@ -257,43 +159,10 @@ def test_update_payment_method_valid_parameters(create_subscription_for_processi (subscription, code) = create_subscription_for_processing (updated_pmt, code) = payments.update_payment_method('process_test', {"pmt_token": 'tok_mastercard'}) assert 201 == code - subhub_dynamodb.remove_from_db('process_test') + g.subhub_account.remove_from_db('process_test') -def test_update_payment_method_missing_fxa_id(): - """ - GIVEN api_token, userid, pmt_token - WHEN missing userid id - THEN do not update payment method for a customer - """ - (updated_pmt, code) = payments.update_payment_method(None, {"pmt_token": 'tok_mastercard'}) - assert 400 == code - assert 'Missing or invalid user' in updated_pmt - - -def test_update_payment_method_invalid_fxa_id(): - """ - GIVEN api_token, userid, pmt_token - WHEN invalid userid id - THEN do not update payment method for a customer - """ - (updated_pmt, code) = payments.update_payment_method(42, {"pmt_token": 'tok_mastercard'}) - assert 400 == code - assert 'Missing or invalid user' in updated_pmt - - -def test_update_payment_method_missing_payment_token(): - """ - GIVEN api_token, userid, pmt_token - WHEN missing pmt_token - THEN do not update payment method for a customer - """ - (updated_pmt, code) = payments.update_payment_method('moz12345', {"pmt_token": None}) - assert 400 == code - assert 'Missing token' in updated_pmt - - -def test_update_payment_method_invalid_payment_token(create_subscription_for_processing): +def test_update_payment_method_invalid_payment_token(app, create_subscription_for_processing): """ GIVEN api_token, userid, pmt_token WHEN invalid pmt_token @@ -302,4 +171,4 @@ def test_update_payment_method_invalid_payment_token(create_subscription_for_pro (updated_pmt, code) = payments.update_payment_method('process_test', {"pmt_token": 'tok_invalid'}) assert 400 == code assert 'No such token:' in updated_pmt - subhub_dynamodb.remove_from_db('process_test') + g.subhub_account.remove_from_db('process_test') diff --git a/tox.ini b/tox.ini index 475e4fb..343bcf7 100644 --- a/tox.ini +++ b/tox.ini @@ -10,7 +10,7 @@ run_before = export AWS_XRAY_SDK_ENABLED=false envdir = {toxinidir}/../.venv/subhub -passenv = STRIPE_API_KEY, SUBHUB_TABLE +passenv = STRIPE_API_KEY, USER_TABLE deps = -rsubhub/test-requirements.txt .[test]