Removes connection types

CHANGELOG.md

@@ -5,7 +5,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 ## [upcoming]
 
 ### Changed
-* Separates read-only logic from logic that requires a transaction
+* Uses RAII to represent operations that require an "edit"
 
 ## [4.1.0] - 2019-07-10
 ### Removed

mozapkpublisher/check_rollout.py

@@ -17,9 +17,9 @@
 logger = logging.getLogger(__name__)
 
 
-def check_rollout(google_play, days):
+def check_rollout(edit, days):
     """Check if package_name has a release on staged rollout for too long"""
-    track_status = google_play.get_track_status(track='production')
+    track_status = edit.get_track_status(track='production')
     releases = track_status['releases']
     for release in releases:
         if release['status'] == 'inProgress':

mozapkpublisher/common/googleplay.py

@@ -43,32 +43,6 @@ def add_general_google_play_arguments(parser):
 --service-account and --credentials must still be provided (you can just fill them with random string and file).''')
 
 
-class GooglePlayConnection:
-    def __init__(self, edit_resource):
-        self.edit_resource = edit_resource
-
-    @staticmethod
-    def open(service_account, credentials_file_path):
-        # Create an httplib2.Http object to handle our HTTP requests an
-        # authorize it with the Credentials. Note that the first parameter,
-        # service_account_name, is the Email address created for the Service
-        # account. It must be the email address associated with the key that
-        # was created.
-        scope = 'https://www.googleapis.com/auth/androidpublisher'
-        credentials = ServiceAccountCredentials.from_p12_keyfile(
-            service_account,
-            credentials_file_path,
-            scopes=scope
-        )
-        http = httplib2.Http()
-        http = credentials.authorize(http)
-
-        service = build(serviceName='androidpublisher', version='v3', http=http,
-                        cache_discovery=False)
-
-        return GooglePlayConnection(service.edits())
-
-
 class _ExecuteDummy:
     def __init__(self, return_value):
         self._return_value = return_value
@@ -77,35 +51,6 @@ def execute(self):
         return self._return_value
 
 
-class MockGooglePlayConnection:
-    def __init__(self):
-        edit_resource_mock = MagicMock()
-
-        edit_resource_mock.insert = lambda *args, **kwargs: _ExecuteDummy(
-            {'id': 'fake-transaction-id'})
-        edit_resource_mock.commit = lambda *args, **kwargs: _ExecuteDummy(None)
-
-        apks_mock = MagicMock()
-        apks_mock.upload = lambda *args, **kwargs: _ExecuteDummy(
-            {'versionCode': 'fake-version-code'})
-        edit_resource_mock.apks = lambda *args, **kwargs: apks_mock
-
-        update_mock = MagicMock()
-        update_mock.update = lambda *args, **kwargs: _ExecuteDummy('fake-update-response')
-        edit_resource_mock.tracks = lambda *args, **kwargs: update_mock
-        edit_resource_mock.listings = lambda *args, **kwargs: update_mock
-        edit_resource_mock.apklistings = lambda *args, **kwargs: update_mock
-        self.edit_resource = edit_resource_mock
-
-
-def connection_for_options(contact_google_play, service_account, credentials_file):
-    if contact_google_play:
-        return GooglePlayConnection.open(service_account, credentials_file.name)
-    else:
-        logger.warning('Not a single request to Google Play will be made, since `contact_google_play` was set')
-        return MockGooglePlayConnection()
-
-
 class GooglePlayEdit:
     """Represents an "edit" to an app on the Google Play store
 
@@ -201,9 +146,8 @@ def update_whats_new(self, language, apk_version_code, whats_new):
 
 
 @contextmanager
-def edit(contact_google_play, service_account, google_play_credentials_file, package_name, *, commit):
-    connection = connection_for_options(contact_google_play, service_account, google_play_credentials_file)
-    edit_resource = connection.edit_resource
+def edit(contact_google_play, service_account, credentials_file_name, package_name, *, commit):
+    edit_resource = edit_resource_for_options(contact_google_play, service_account, credentials_file_name)
     edit_id = edit_resource.insert(body={}, packageName=package_name).execute()['id']
     google_play = GooglePlayEdit(edit_resource, edit_id, package_name)
     yield google_play
@@ -214,3 +158,44 @@ def edit(contact_google_play, service_account, google_play_credentials_file, pac
     else:
         edit_resource.validate()
         logger.warning('Transaction not committed, since `commit` was `False`')
+
+
+def edit_resource_for_options(contact_google_play, service_account, credentials_file_name):
+    if contact_google_play:
+        # Create an httplib2.Http object to handle our HTTP requests an
+        # authorize it with the Credentials. Note that the first parameter,
+        # service_account_name, is the Email address created for the Service
+        # account. It must be the email address associated with the key that
+        # was created.
+        scope = 'https://www.googleapis.com/auth/androidpublisher'
+        credentials = ServiceAccountCredentials.from_p12_keyfile(
+            service_account,
+            credentials_file_name,
+            scopes=scope
+        )
+        http = httplib2.Http()
+        http = credentials.authorize(http)
+
+        service = build(serviceName='androidpublisher', version='v3', http=http,
+                        cache_discovery=False)
+
+        return service.edits()
+    else:
+        logger.warning('Not a single request to Google Play will be made, since `contact_google_play` was set')
+        edit_resource_mock = MagicMock()
+
+        edit_resource_mock.insert = lambda *args, **kwargs: _ExecuteDummy(
+            {'id': 'fake-transaction-id'})
+        edit_resource_mock.commit = lambda *args, **kwargs: _ExecuteDummy(None)
+
+        apks_mock = MagicMock()
+        apks_mock.upload = lambda *args, **kwargs: _ExecuteDummy(
+            {'versionCode': 'fake-version-code'})
+        edit_resource_mock.apks = lambda *args, **kwargs: apks_mock
+
+        update_mock = MagicMock()
+        update_mock.update = lambda *args, **kwargs: _ExecuteDummy('fake-update-response')
+        edit_resource_mock.tracks = lambda *args, **kwargs: update_mock
+        edit_resource_mock.listings = lambda *args, **kwargs: update_mock
+        edit_resource_mock.apklistings = lambda *args, **kwargs: update_mock
+        return edit_resource_mock

mozapkpublisher/push_apk.py

@@ -6,7 +6,7 @@
 from mozapkpublisher.common import googleplay, main_logging
 from mozapkpublisher.common.apk import add_apk_checks_arguments, extract_and_check_apks_metadata
 from mozapkpublisher.common.exceptions import WrongArgumentGiven
-from mozapkpublisher.common.googleplay import connection_for_options
+from mozapkpublisher.common.googleplay import edit_resource_for_options
 
 logger = logging.getLogger(__name__)
 
@@ -67,7 +67,7 @@ def push_apk(
     # by package name here.
     split_apk_metadata = _split_apk_metadata_per_package_name(apks_metadata_per_paths)
     for (package_name, apks_metadata) in split_apk_metadata.items():
-        with googleplay.edit(contact_google_play, service_account, google_play_credentials_file,
+        with googleplay.edit(contact_google_play, service_account, google_play_credentials_file.name,
                              package_name, commit=commit) as edit:
             for path, metadata in apks_metadata_per_paths.items():
                 edit.upload_apk(path)

mozapkpublisher/test/common/test_googleplay.py

@@ -12,8 +12,7 @@
 from mozapkpublisher.common import googleplay
 from mozapkpublisher.common.exceptions import WrongArgumentGiven
 from mozapkpublisher.common.googleplay import add_general_google_play_arguments, \
-    GooglePlayEdit, MockGooglePlayConnection, GooglePlayConnection, \
-    connection_for_options
+    GooglePlayEdit, edit_resource_for_options
 from mozapkpublisher.test import does_not_raise
 
 
@@ -30,19 +29,9 @@ def test_add_general_google_play_arguments():
     assert config.service_account == 'dummy@dummy'
 
 
-@patch.object(googleplay, 'MockGooglePlayConnection')
-def test_connection_for_options_contact(mock):
-    connection_for_options(False, '', MagicMock)
-    mock.assert_called_with()
-
-
-@patch.object(googleplay.ServiceAccountCredentials, 'from_p12_keyfile')
-@patch.object(googleplay, 'build')
-def test_google_play_connection(mock_build, _):
-    mock_service = MagicMock()
-    mock_service.edits.return_value = 'edit resource'
-    mock_build.return_value = mock_service
-    assert GooglePlayConnection.open('service_account', 'file').edit_resource == 'edit resource'
+def test_edit_resource_for_options_do_not_contact():
+    edit_resource = edit_resource_for_options(False, '', MagicMock)
+    assert isinstance(edit_resource, MagicMock)
 
 
 @pytest.fixture
@@ -55,24 +44,20 @@ def edit_resource_mock():
     return edit_resource
 
 
-@patch.object(googleplay, 'MockGooglePlayConnection')
-def test_google_play_edit_no_commit_transaction(google_play_connection_constructor):
-    connection = MockGooglePlayConnection()
+@patch.object(googleplay, 'edit_resource_for_options')
+def test_google_play_edit_no_commit_transaction(edit_resource_for_options_):
     mock_edits_resource = MagicMock()
-    connection.edit_resource = mock_edits_resource
-    google_play_connection_constructor.return_value = connection
+    edit_resource_for_options_.return_value = mock_edits_resource
     with googleplay.edit(False, None, None, 'package.name', commit=False) as _:
         pass
 
     mock_edits_resource.commit.assert_not_called()
 
 
-@patch.object(googleplay, 'MockGooglePlayConnection')
-def test_google_play_edit_commit_transaction(google_play_connection_constructor):
-    connection = MockGooglePlayConnection()
+@patch.object(googleplay, 'edit_resource_for_options')
+def test_google_play_edit_commit_transaction(edit_resource_for_options_):
     mock_edits_resource = MagicMock()
-    connection.edit_resource = mock_edits_resource
-    google_play_connection_constructor.return_value = connection
+    edit_resource_for_options_.return_value = mock_edits_resource
     with googleplay.edit(False, None, None, 'package.name', commit=True) as _:
         pass
 

mozapkpublisher/update_apk_description.py

@@ -5,23 +5,23 @@
 
 from argparse import ArgumentParser
 from mozapkpublisher.common import googleplay, store_l10n
-from mozapkpublisher.common.googleplay import connection_for_options
+from mozapkpublisher.common.googleplay import edit_resource_for_options
 
 logger = logging.getLogger(__name__)
 
 
 def update_apk_description(package_name, force_locale, commit, service_account, google_play_credentials_file,
                            contact_google_play):
-    with googleplay.edit(contact_google_play, service_account, google_play_credentials_file,
+    with googleplay.edit(contact_google_play, service_account, google_play_credentials_file.name,
                          package_name, commit=commit) as edit:
         moz_locales = [force_locale] if force_locale else None
         l10n_strings = store_l10n.get_translations_per_google_play_locale_code(package_name, moz_locales)
         create_or_update_listings(edit, l10n_strings)
 
 
-def create_or_update_listings(google_play, l10n_strings):
+def create_or_update_listings(edit, l10n_strings):
     for google_play_locale_code, translation in l10n_strings.items():
-        google_play.update_listings(
+        edit.update_listings(
             google_play_locale_code,
             full_description=translation['long_desc'],
             short_description=translation['short_desc'],