Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions src/sentry/utils/appleconnect/appstore_connect.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ def _get_next_page(response_json: Mapping[str, Any]) -> Optional[str]:

def _get_appstore_info_paged(
session: Session, credentials: AppConnectCredentials, url: str
) -> Generator[Any, None, None]:
) -> Generator[JSONData, None, None]:
"""Iterates through all the pages from a paged response.

App Store Connect responses shares the general format:
Expand Down Expand Up @@ -266,7 +266,11 @@ def get_multiple_related(self, data: JSONData, relation: str) -> Optional[List[J


def get_build_info(
session: Session, credentials: AppConnectCredentials, app_id: str
session: Session,
credentials: AppConnectCredentials,
app_id: str,
*,
include_expired: bool = False,
) -> List[BuildInfo]:
"""Returns the build infos for an application.

Expand All @@ -289,16 +293,16 @@ def get_build_info(
"&limit=200"
# include related AppStore/PreRelease versions with the response as well as
# buildBundles which contains metadata on the debug resources (dSYMs)
"&include=appStoreVersion,preReleaseVersion,buildBundles"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: why the reordering here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's silly:

  • Apple re-orders these in this order in the self and next links in API responses
  • The tests sets up responses with the URLs from self and next from the recorded responses.
  • responses is smart enough to understand query parameters can be re-ordered when mapping requests to responses, but it does not know these arguments to a parameter are equivalent.

So the simplest thing was to align this with how apple builds the response

"&include=preReleaseVersion,appStoreVersion,buildBundles"
# fetch the maximum number of build bundles
"&limit[buildBundles]=50"
# sort newer releases first
"&sort=-uploadedDate"
# only include valid builds
"&filter[processingState]=VALID"
# and builds that have not expired yet
"&filter[expired]=false"
)
if not include_expired:
url += "&filter[expired]=false"
pages = _get_appstore_info_paged(session, credentials, url)
build_info = []

Expand Down
1 change: 1 addition & 0 deletions tests/sentry/utils/appleconnect/pages_data.json

Large diffs are not rendered by default.

194 changes: 194 additions & 0 deletions tests/sentry/utils/appleconnect/test_appstore_connect.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,203 @@
"""Tests for using the App Store Connect API.

Some of these tests will try to use the live API requests if you provide a ``apikey.json``
file with credentials. See the ``api_credentials`` fixture for details.
"""

import pathlib
import textwrap
import urllib.parse
from typing import Optional

import pytest
import requests
import responses as responses_mod # type: ignore

from sentry.lang.native.appconnect import NoDsymUrl
from sentry.utils import json
from sentry.utils.appleconnect import appstore_connect


class TestListBuilds:
# mypy just can't cope with this function:
# - The request fixture type is private: _pytest.fixtures.FixtureRequest.
# - mypy has no idea that `pytest.skip()` raises and exception and terminates control flow.
@pytest.fixture(scope="session", params=["live", "responses"]) # type: ignore
def api_credentials(self, request) -> appstore_connect.AppConnectCredentials: # type: ignore
"""An App Store Connect API key in the form of AppConnectCredentials.

If ``apikey.json`` is present in the current directory it will load the credentials
from this json for the ``live`` param, the format should look like this:

```json
{
"key_id": "AAAAAAAAAA",
"issuer_id": "abcdef01-2345-6789-abcd-ef0123456789",
"private_key": "-----BEGIN PRIVATE KEY-----\na_bunch_of_\n_separated_base64\n-----END PRIVATE KEY-----\n"
}
```

For the ``responses`` param a fake value is returned with a key_id="AAAAAAAAAA".
"""
if request.param == "live":
here = pathlib.Path(__file__).parent
keyfile = here / "apikey.json"
try:
data = json.loads(keyfile.read_text(encoding="utf-8"))
except FileNotFoundError:
pytest.skip("No API key available for live tests")
else:
return appstore_connect.AppConnectCredentials(
key_id=data["key_id"], issuer_id=data["issuer_id"], key=data["private_key"]
)
else:
# NOTE: This key has been generated outside of App Store Connect for the sole
# purpose of being used in this test. The key is not associated with any account
# on App Store Connect, and therefore is unable to access any live data.
return appstore_connect.AppConnectCredentials(
key_id="AAAAAAAAAA",
issuer_id="12345678-abcd-abcd-abcd-1234567890ab",
key=(
textwrap.dedent(
"""
-----BEGIN EC PRIVATE KEY-----
MHcCAQEEILd+RopXKDeu4wvj01ydqDp9goiI2KroiY4wgrMKz4j4oAoGCCqGSM49
AwEHoUQDQgAEe0GpznJGxz5cLukKBneiXlbPEEZRvqaKmpdd5Es+KQW0RK/9WmXK
J9b/VBtFOSMiVav8iev+Kr/xPqcoor6Mpw==
-----END EC PRIVATE KEY-----
"""
)
),
)

@pytest.fixture(scope="session") # type: ignore
def app_id(self) -> str:
"""The Sentry Cocoa Swift example app."""
return "1549832463"

def write_paged_build_response(
self, api_credentials: appstore_connect.AppConnectCredentials, app_id: str
) -> None:
"""Use this function to create the ``pages_data.jons`` fixture data.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: s/jons/json


NOTE: this function is purposefully dead code, it shows how to re-create the fixture
data when it needs updating.

The mocked_list_builds_api needs to load this data from a file, this can be used to
generate the file.
"""
session = requests.Session()
url = (
"v1/builds"
f"?filter[app]={app_id}"
"&limit=200"
"&include=appStoreVersion,preReleaseVersion,buildBundles"
"&limit[buildBundles]=50"
"&sort=-uploadedDate"
"&filter[processingState]=VALID"
)
pages = list(appstore_connect._get_appstore_info_paged(session, api_credentials, url))
assert pages

module_dir = pathlib.Path(__file__).parent
pages_file = module_dir / "pages_data.json"
pages_file.write_text(json.dumps(pages))

@pytest.fixture # type: ignore
def mocked_list_builds_api(
self, api_credentials: appstore_connect.AppConnectCredentials
) -> Optional[responses_mod.RequestsMock]:
"""Optionally mocks the App Store Connect list builds API.

This fixture piggybacks on the ``api_credentials`` fixture's parametrisation and if
it is the fake credentials it will mock out the responses to the list build URLs
with our pre-configured data. Otherwise it does nothing.
"""
if api_credentials.key_id == "AAAAAAAAAA":
here = pathlib.Path(__file__).parent
saved_responses_filename = here / "pages_data.json"
saved_pages = json.loads(saved_responses_filename.read_text())
with responses_mod.RequestsMock() as r:
for page in saved_pages:
r.add(
method="GET",
url=urllib.parse.unquote(page["links"]["self"]),
json=page,
)
yield r
else:
yield None

def test_get_build_info(
self,
mocked_list_builds_api: Optional[responses_mod.RequestsMock],
api_credentials: appstore_connect.AppConnectCredentials,
app_id: str,
) -> None:
session = requests.Session()

# Be sure to consume the entire ``builds`` iterator, otherwise the
# responses.RequestsMock will be unhappy that not all calls were made.
builds = list(
appstore_connect.get_build_info(session, api_credentials, app_id, include_expired=True)
)
build = builds[0]

assert build.app_id == app_id
assert build.platform == "IOS"
assert build.version
assert build.build_number
assert build.uploaded_date

def test_dsyms_needed(
self,
mocked_list_builds_api: Optional[responses_mod.RequestsMock],
api_credentials: appstore_connect.AppConnectCredentials,
app_id: str,
) -> None:
session = requests.Session()

# Be sure to consume the entire ``builds`` iterator, otherwise the
# responses.RequestsMock will be unhappy that not all calls were made.
builds = list(
appstore_connect.get_build_info(session, api_credentials, app_id, include_expired=True)
)

for build in builds:
if build.build_number == "332":
break
else:
pytest.fail("Build 332 not found")

assert build.build_number == "332"
assert isinstance(build.dsym_url, str)
assert build.dsym_url.startswith("http://iosapps.itunes.apple.com/itunes-assets/")
assert "accessKey=" in build.dsym_url

def test_no_dsyms_needed(
self,
mocked_list_builds_api: Optional[responses_mod.RequestsMock],
api_credentials: appstore_connect.AppConnectCredentials,
app_id: str,
) -> None:
session = requests.Session()

# Be sure to consume the entire ``builds`` iterator, otherwise the
# responses.RequestsMock will be unhappy that not all calls were made.
builds = list(
appstore_connect.get_build_info(session, api_credentials, app_id, include_expired=True)
)

for build in builds:
if build.build_number == "333":
break
else:
pytest.fail("Build 333 not found")

assert build.build_number == "333"
assert build.dsym_url is appstore_connect.NoDsymUrl.NOT_NEEDED


class TestGetDsymUrl:
def test_none_bundles(self) -> None:
assert appstore_connect._get_dsym_url(None) is NoDsymUrl.NOT_NEEDED
Expand Down