Removes types from GooglePlayEdit API

mozapkpublisher/check_rollout.py

@@ -8,8 +8,9 @@
 import requests
 
 from argparse import ArgumentParser
-from mozapkpublisher.common.googleplay import add_general_google_play_arguments, \
-    GooglePlayConnection, GooglePlayEdit
+
+from mozapkpublisher.common import googleplay
+from mozapkpublisher.common.googleplay import add_general_google_play_arguments
 
 DAY = 24 * 60 * 60
 
@@ -40,11 +41,9 @@ def main():
                         type=int, default=7)
     config = parser.parse_args()
 
-    with GooglePlayEdit.transaction(GooglePlayConnection.open(
-        config.service_account,
-        config.google_play_credentials_file.name
-    ), 'org.mozilla.firefox', True) as google_play:
-        for (release, age) in check_rollout(google_play, config.days):
+    with googleplay.edit(True, config.service_account, config.google_play_credentials_file.name,
+                         'org.mozilla.firefox', commit=False) as edit:
+        for (release, age) in check_rollout(edit, config.days):
             print('fennec {} is on staged rollout at {}% but it shipped {} days ago'.format(
                   release['name'], int(release['userFraction'] * 100), int(age / DAY)))
 

mozapkpublisher/common/googleplay.py

@@ -45,10 +45,7 @@ def add_general_google_play_arguments(parser):
 
 class GooglePlayConnection:
     def __init__(self, edit_resource):
-        self._edit_resource = edit_resource
-
-    def get_edit_resource(self):
-        return self._edit_resource
+        self.edit_resource = edit_resource
 
     @staticmethod
     def open(service_account, credentials_file_path):
@@ -81,26 +78,24 @@ def execute(self):
 
 
 class MockGooglePlayConnection:
-    @staticmethod
-    def get_edit_resource():
-        edit_service_mock = MagicMock()
+    def __init__(self):
+        edit_resource_mock = MagicMock()
 
-        edit_service_mock.insert = lambda *args, **kwargs: _ExecuteDummy(
+        edit_resource_mock.insert = lambda *args, **kwargs: _ExecuteDummy(
             {'id': 'fake-transaction-id'})
-        edit_service_mock.commit = lambda *args, **kwargs: _ExecuteDummy(None)
+        edit_resource_mock.commit = lambda *args, **kwargs: _ExecuteDummy(None)
 
         apks_mock = MagicMock()
         apks_mock.upload = lambda *args, **kwargs: _ExecuteDummy(
             {'versionCode': 'fake-version-code'})
-        edit_service_mock.apks = lambda *args, **kwargs: apks_mock
+        edit_resource_mock.apks = lambda *args, **kwargs: apks_mock
 
         update_mock = MagicMock()
         update_mock.update = lambda *args, **kwargs: _ExecuteDummy('fake-update-response')
-        edit_service_mock.tracks = lambda *args, **kwargs: update_mock
-        edit_service_mock.listings = lambda *args, **kwargs: update_mock
-        edit_service_mock.apklistings = lambda *args, **kwargs: update_mock
-
-        return edit_service_mock
+        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):
@@ -204,16 +199,18 @@ def update_whats_new(self, language, apk_version_code, whats_new):
         ))
         logger.debug(u'Apk listing response: {}'.format(response))
 
-    @staticmethod
-    @contextmanager
-    def transaction(connection, package_name, commit):
-        edit_resource = connection.get_edit_resource()
-        edit_id = edit_resource.insert(body={}, packageName=package_name).execute()['id']
-        google_play = GooglePlayEdit(edit_resource, edit_id, package_name)
-        yield google_play
-        if commit:
-            edit_resource.commit(editId=edit_id, packageName=package_name)
-            logger.info('Changes committed')
-            logger.debug('edit_id "{}" for "{}" has been committed'.format(edit_id, package_name))
-        else:
-            logger.warning('Transaction not committed, since `commit` was `False`')
+
+@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
+    edit_id = edit_resource.insert(body={}, packageName=package_name).execute()['id']
+    google_play = GooglePlayEdit(edit_resource, edit_id, package_name)
+    yield google_play
+    if commit:
+        edit_resource.commit(editId=edit_id, packageName=package_name)
+        logger.info('Changes committed')
+        logger.debug('edit_id "{}" for "{}" has been committed'.format(edit_id, package_name))
+    else:
+        edit_resource.validate()
+        logger.warning('Transaction not committed, since `commit` was `False`')

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 GooglePlayEdit, connection_for_options
+from mozapkpublisher.common.googleplay import connection_for_options
 
 logger = logging.getLogger(__name__)
 
@@ -63,21 +63,17 @@ def push_apk(
         skip_check_ordered_version_codes,
     )
 
-    # TODO make programmatic usage of this library provide a "GooglePlayConnection" object, rather
-    # than having to provide redundant information like "service_account" and "credentials" when
-    # "contact_google_play" is false
-    connection = connection_for_options(contact_google_play, service_account, google_play_credentials_file)
-
     # Each distinct product must be uploaded in different Google Play transaction, so we split them
     # 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 GooglePlayEdit.transaction(connection, package_name, commit) as google_play:
+        with googleplay.edit(contact_google_play, service_account, google_play_credentials_file,
+                             package_name, commit=commit) as edit:
             for path, metadata in apks_metadata_per_paths.items():
-                google_play.upload_apk(path)
+                edit.upload_apk(path)
 
             all_version_codes = _get_ordered_version_codes(apks_metadata_per_paths)
-            google_play.update_track(track, all_version_codes, rollout_percentage)
+            edit.update_track(track, all_version_codes, rollout_percentage)
 
 
 def _split_apk_metadata_per_package_name(apks_metadata_per_paths):

mozapkpublisher/test/common/test_googleplay.py

@@ -42,7 +42,7 @@ 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').get_edit_resource() == 'edit resource'
+    assert GooglePlayConnection.open('service_account', 'file').edit_resource == 'edit resource'
 
 
 @pytest.fixture
@@ -55,21 +55,25 @@ def edit_resource_mock():
     return edit_resource
 
 
-def test_google_play_edit_no_commit_transaction():
+@patch.object(googleplay, 'MockGooglePlayConnection')
+def test_google_play_edit_no_commit_transaction(google_play_connection_constructor):
     connection = MockGooglePlayConnection()
     mock_edits_resource = MagicMock()
-    connection.get_edit_resource = lambda: mock_edits_resource
-    with GooglePlayEdit.transaction(connection, 'package.name', False) as _:
+    connection.edit_resource = mock_edits_resource
+    google_play_connection_constructor.return_value = connection
+    with googleplay.edit(False, None, None, 'package.name', commit=False) as _:
         pass
 
     mock_edits_resource.commit.assert_not_called()
 
 
-def test_google_play_edit_commit_transaction():
+@patch.object(googleplay, 'MockGooglePlayConnection')
+def test_google_play_edit_commit_transaction(google_play_connection_constructor):
     connection = MockGooglePlayConnection()
     mock_edits_resource = MagicMock()
-    connection.get_edit_resource = lambda: mock_edits_resource
-    with GooglePlayEdit.transaction(connection, 'package.name', True) as _:
+    connection.edit_resource = mock_edits_resource
+    google_play_connection_constructor.return_value = connection
+    with googleplay.edit(False, None, None, 'package.name', commit=True) as _:
         pass
 
     mock_edits_resource.commit.assert_called_with(editId=ANY, packageName='package.name')

mozapkpublisher/test/test_push_apk.py

@@ -29,11 +29,11 @@
 
 
 @pytest.fixture
-def writable_google_play_mock():
+def google_play_edit_mock():
     return create_autospec(googleplay.GooglePlayEdit)
 
 
-def set_up_mocks(monkeypatch_, writable_google_play_mock_):
+def set_up_mocks(monkeypatch_, google_play_edit_mock_):
     def _metadata(*args, **kwargs):
         return {
             apk_arm.name: {
@@ -75,10 +75,10 @@ def _metadata(*args, **kwargs):
         }
 
     @contextmanager
-    def fake_transaction(_, __, ___):
-        yield writable_google_play_mock_
+    def fake_edit(_, __, ___, ____, *, commit):
+        yield google_play_edit_mock_
 
-    monkeypatch_.setattr(googleplay.GooglePlayEdit, 'transaction', fake_transaction)
+    monkeypatch_.setattr(googleplay, 'edit', fake_edit)
     monkeypatch_.setattr('mozapkpublisher.push_apk.extract_and_check_apks_metadata', _metadata)
 
 
@@ -93,13 +93,13 @@ def test_invalid_rollout_percentage():
         push_apk(APKS, SERVICE_ACCOUNT, credentials, invalid_track, [], rollout_percentage=valid_percentage)
 
 
-def test_valid_rollout_percentage(writable_google_play_mock, monkeypatch):
-    set_up_mocks(monkeypatch, writable_google_play_mock)
+def test_valid_rollout_percentage(google_play_edit_mock, monkeypatch):
+    set_up_mocks(monkeypatch, google_play_edit_mock)
     valid_percentage = 50
 
     push_apk(APKS, SERVICE_ACCOUNT, credentials, 'rollout', [], rollout_percentage=valid_percentage, contact_google_play=False)
-    writable_google_play_mock.update_track.assert_called_once_with('rollout', ['0', '1'], valid_percentage)
-    writable_google_play_mock.update_track.reset_mock()
+    google_play_edit_mock.update_track.assert_called_once_with('rollout', ['0', '1'], valid_percentage)
+    google_play_edit_mock.update_track.reset_mock()
 
 
 def test_get_ordered_version_codes():
@@ -113,15 +113,15 @@ def test_get_ordered_version_codes():
     }) == ['0', '1']    # should be sorted
 
 
-def test_upload_apk(writable_google_play_mock, monkeypatch):
-    set_up_mocks(monkeypatch, writable_google_play_mock)
+def test_upload_apk(google_play_edit_mock, monkeypatch):
+    set_up_mocks(monkeypatch, google_play_edit_mock)
 
     push_apk(APKS, SERVICE_ACCOUNT, credentials, 'alpha', [], contact_google_play=False)
 
     for apk_file in (apk_arm, apk_x86):
-        writable_google_play_mock.upload_apk.assert_any_call(apk_file.name)
+        google_play_edit_mock.upload_apk.assert_any_call(apk_file.name)
 
-    writable_google_play_mock.update_track.assert_called_once_with('alpha', ['0', '1'], None)
+    google_play_edit_mock.update_track.assert_called_once_with('alpha', ['0', '1'], None)
 
 
 def test_get_distinct_package_name_apk_metadata():

mozapkpublisher/update_apk_description.py

@@ -5,18 +5,18 @@
 
 from argparse import ArgumentParser
 from mozapkpublisher.common import googleplay, store_l10n
-from mozapkpublisher.common.googleplay import connection_for_options, GooglePlayEdit
+from mozapkpublisher.common.googleplay import connection_for_options
 
 logger = logging.getLogger(__name__)
 
 
 def update_apk_description(package_name, force_locale, commit, service_account, google_play_credentials_file,
                            contact_google_play):
-    connection = connection_for_options(contact_google_play, service_account, google_play_credentials_file)
-    with GooglePlayEdit.transaction(connection, package_name, commit) as google_play:
+    with googleplay.edit(contact_google_play, service_account, google_play_credentials_file,
+                         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(google_play, l10n_strings)
+        create_or_update_listings(edit, l10n_strings)
 
 
 def create_or_update_listings(google_play, l10n_strings):