Skip to content

Commit

Permalink
feat: add function to get specific app's details (#470)
Browse files Browse the repository at this point in the history
  • Loading branch information
giovanni-guidini committed May 31, 2024
1 parent 1beef84 commit 4a0347d
Show file tree
Hide file tree
Showing 9 changed files with 154 additions and 12 deletions.
5 changes: 5 additions & 0 deletions database/models/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,11 @@ class CommitNotification(CodecovBaseModel):

id_ = Column("id", types.BigInteger, primary_key=True)
commit_id = Column(types.BigInteger, ForeignKey("commits.id"))
gh_app_id = Column(
types.BigInteger,
ForeignKey("codecov_auth_githubappinstallation.id"),
nullable=True,
)
notification_type = Column(
postgresql.ENUM(Notification, values_callable=lambda x: [e.value for e in x]),
nullable=False,
Expand Down
4 changes: 4 additions & 0 deletions helpers/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ class RepositoryWithoutValidBotError(Exception):
pass


class RequestedGithubAppNotFound(Exception):
pass


class OwnerWithoutValidBotError(Exception):
pass

Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# This file is autogenerated by pip-compile with Python 3.12
# by the following command:
#
# pip-compile requirements.in
# pip-compile
#
amqp==5.2.0
# via kombu
Expand Down
39 changes: 39 additions & 0 deletions services/bots/github_apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
)
from helpers.exceptions import (
NoConfiguredAppsAvailable,
RequestedGithubAppNotFound,
)
from services.bots.types import TokenWithOwner
from services.github import get_github_integration_token
Expand Down Expand Up @@ -131,6 +132,43 @@ def get_github_app_token(
return installation_token, None


def get_specific_github_app_details(
owner: Owner, github_app_id: int, commitid: str
) -> GithubInstallationInfo:
"""Gets the GithubInstallationInfo for GithubAppInstallation with id github_app_id.
Args:
owner (Owner): the owner of the app. We look only in the apps for this owner.
github_app_id (int): the ID of the GithubAppInstallation we're looking for
commitid (str): Commit.commitid, used for logging purposes
Raises:
RequestedGithubAppNotFound - if the app is not found in the given owner raise exception.
The assumption is that we need this specific app for a reason, and if we can't find the app
it's better to just fail
"""
app: GithubAppInstallation = next(
(obj for obj in owner.github_app_installations if obj.id == github_app_id), None
)
if app is None:
log.exception(
"Can't find requested app",
extra=dict(ghapp_id=github_app_id, commitid=commitid),
)
raise RequestedGithubAppNotFound()
if not app.is_configured():
log.warning(
"Request for specific app that is not configured",
extra=dict(ghapp_id=id, commitid=commitid),
)
return GithubInstallationInfo(
id=app.id,
installation_id=app.installation_id,
app_id=app.app_id,
pem_path=app.pem_path,
)


def get_github_app_info_for_owner(
owner: Owner,
*,
Expand Down Expand Up @@ -203,6 +241,7 @@ def get_github_app_info_for_owner(
info_to_get_tokens = list(
map(
lambda obj: GithubInstallationInfo(
id=obj.id,
installation_id=obj.installation_id,
app_id=obj.app_id,
pem_path=obj.pem_path,
Expand Down
40 changes: 32 additions & 8 deletions services/bots/tests/test_bots.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,10 @@ def test_select_owner_single_installation(self, dbsession):
),
token_owner=None,
selected_installation_info=GithubInstallationInfo(
installation_id=1200, app_id=200, pem_path="pem_path"
id=installations[0].id,
installation_id=1200,
app_id=200,
pem_path="pem_path",
),
fallback_installations=[],
token_type_mapping=None,
Expand Down Expand Up @@ -210,7 +213,10 @@ def test_select_owner_multiple_installations_default_name(self, dbsession):
),
token_owner=None,
selected_installation_info=GithubInstallationInfo(
installation_id=1200, app_id=200, pem_path="pem_path"
id=installations[0].id,
installation_id=1200,
app_id=200,
pem_path="pem_path",
),
fallback_installations=[],
token_type_mapping=None,
Expand Down Expand Up @@ -250,11 +256,17 @@ def test_select_owner_multiple_installations_custom_name(self, dbsession):
),
token_owner=None,
selected_installation_info=GithubInstallationInfo(
installation_id=1300, app_id=300, pem_path="pem_path"
id=installations[1].id,
installation_id=1300,
app_id=300,
pem_path="pem_path",
),
fallback_installations=[
GithubInstallationInfo(
installation_id=1200, app_id=200, pem_path="pem_path"
id=installations[0].id,
installation_id=1200,
app_id=200,
pem_path="pem_path",
)
],
token_type_mapping=None,
Expand Down Expand Up @@ -437,7 +449,10 @@ def test_select_repo_single_installation(self, dbsession):
),
token_owner=None,
selected_installation_info=GithubInstallationInfo(
installation_id=1200, app_id=200, pem_path="pem_path"
id=installations[0].id,
installation_id=1200,
app_id=200,
pem_path="pem_path",
),
fallback_installations=[],
token_type_mapping=None,
Expand Down Expand Up @@ -504,7 +519,10 @@ def test_select_repo_multiple_installations_default_name(self, dbsession):
),
token_owner=None,
selected_installation_info=GithubInstallationInfo(
installation_id=1200, app_id=200, pem_path="pem_path"
id=installations[0].id,
installation_id=1200,
app_id=200,
pem_path="pem_path",
),
fallback_installations=[],
token_type_mapping=None,
Expand Down Expand Up @@ -547,11 +565,17 @@ def test_select_repo_multiple_installations_custom_name(self, dbsession):
),
token_owner=None,
selected_installation_info=GithubInstallationInfo(
installation_id=1300, app_id=300, pem_path="pem_path"
id=installations[1].id,
installation_id=1300,
app_id=300,
pem_path="pem_path",
),
fallback_installations=[
GithubInstallationInfo(
installation_id=1200, app_id=200, pem_path="pem_path"
id=installations[0].id,
installation_id=1200,
app_id=200,
pem_path="pem_path",
)
],
token_type_mapping=None,
Expand Down
51 changes: 51 additions & 0 deletions services/bots/tests/test_github_apps.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import pytest
from shared.typings.torngit import GithubInstallationInfo

from database.models.core import GithubAppInstallation
from database.tests.factories.core import OwnerFactory
from helpers.exceptions import RequestedGithubAppNotFound
from services.bots.github_apps import get_specific_github_app_details


class TestGetSpecificGithubAppDetails(object):
def _get_owner_with_apps(self, dbsession):
owner = OwnerFactory(service="github")
app_1 = GithubAppInstallation(
owner=owner,
installation_id=1200,
app_id=12,
)
app_2 = GithubAppInstallation(
owner=owner,
installation_id=1500,
app_id=15,
pem_path="some_path",
)
dbsession.add_all([owner, app_1, app_2])
dbsession.flush()
assert owner.github_app_installations == [app_1, app_2]
return owner

def test_get_specific_github_app_details(self, dbsession):
owner = self._get_owner_with_apps(dbsession)
assert get_specific_github_app_details(
owner, owner.github_app_installations[0].id, "commit_id_for_logs"
) == GithubInstallationInfo(
id=owner.github_app_installations[0].id,
installation_id=1200,
app_id=12,
pem_path=None,
)
assert get_specific_github_app_details(
owner, owner.github_app_installations[1].id, "commit_id_for_logs"
) == GithubInstallationInfo(
id=owner.github_app_installations[1].id,
installation_id=1500,
app_id=15,
pem_path="some_path",
)

def test_get_specific_github_app_not_found(self, dbsession):
owner = self._get_owner_with_apps(dbsession)
with pytest.raises(RequestedGithubAppNotFound):
get_specific_github_app_details(owner, 123456, "commit_id_for_logs")
10 changes: 9 additions & 1 deletion services/tests/test_bots.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ def test_get_repo_appropriate_bot_token_via_installation_covered_repo(
installations = get_github_app_info_for_owner(repo.owner)
assert installations == [
{
"id": installation.id,
"installation_id": 12341234,
"app_id": None,
"pem_path": None,
Expand Down Expand Up @@ -360,6 +361,7 @@ def test_get_owner_installation_id_yes_installation_yes_legacy_integration(
assert owner.github_app_installations == [installation]
assert get_github_app_info_for_owner(owner) == [
{
"id": installation.id,
"installation_id": 123456,
"app_id": None,
"pem_path": None,
Expand Down Expand Up @@ -389,11 +391,17 @@ def test_get_owner_installation_id_yes_installation_yes_legacy_integration_yes_f
assert owner.github_app_installations == [installation_0, installation_1]
assert get_github_app_info_for_owner(owner, installation_name="my_app") == [
{
"id": installation_1.id,
"installation_id": 12000,
"app_id": 1212,
"pem_path": "path",
},
{"installation_id": 123456, "app_id": None, "pem_path": None},
{
"id": installation_0.id,
"installation_id": 123456,
"app_id": None,
"pem_path": None,
},
]

def test_get_owner_installation_id_yes_installation_all_rate_limited(
Expand Down
7 changes: 6 additions & 1 deletion services/tests/test_owner_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,12 @@ def test_get_owner_provider_service_with_installation(self, dbsession, mocker):
"username": owner.username,
},
"repo": {},
"installation": {"installation_id": 1500, "pem_path": None, "app_id": None},
"installation": {
"id": installation.id,
"installation_id": 1500,
"pem_path": None,
"app_id": None,
},
"fallback_installations": [],
}
assert res.service == "github"
Expand Down
8 changes: 7 additions & 1 deletion services/tests/test_repository_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,12 +159,18 @@ def test_get_repo_provider_service_github_with_installations(
"repoid": repo.repoid,
},
"installation": {
"id": installation_1.id,
"installation_id": 1300,
"app_id": 300,
"pem_path": "path",
},
"fallback_installations": [
{"app_id": 200, "installation_id": 1200, "pem_path": None}
{
"id": installation_0.id,
"app_id": 200,
"installation_id": 1200,
"pem_path": None,
}
],
}
assert res.data == expected_data
Expand Down

0 comments on commit 4a0347d

Please sign in to comment.