[github apps] drop github_app_installs table as it's unused (#52136)

We are getting the list of installations directly from github API, so no
need to keep it stored in the `github_app_installs` table on our side.
Dropping the table and related stored methods.

## Test plan

Tested locally by installing an app. It still works.
This commit is contained in:
Milan Freml 2023-05-18 12:43:01 +02:00 committed by GitHub
parent fc246bb6dc
commit 38d3d0195e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 16 additions and 354 deletions

View File

@ -296,12 +296,6 @@ func newServeMux(db edb.EnterpriseDB, prefix string, cache *rcache.Cache) http.H
return
}
err = db.GitHubApps().Install(ctx, id, installationID)
if err != nil {
http.Error(w, fmt.Sprintf("Unexpected error while installing github app: %s", err.Error()), http.StatusInternalServerError)
return
}
http.Redirect(w, req, fmt.Sprintf("/site-admin/github-apps/%s?installation_id=%d", MarshalGitHubAppID(int64(app.ID)), installationID), http.StatusFound)
return
} else {

View File

@ -34,9 +34,6 @@ type MockGitHubAppsStore struct {
// GetBySlugFunc is an instance of a mock function object controlling
// the behavior of the method GetBySlug.
GetBySlugFunc *GitHubAppsStoreGetBySlugFunc
// InstallFunc is an instance of a mock function object controlling the
// behavior of the method Install.
InstallFunc *GitHubAppsStoreInstallFunc
// ListFunc is an instance of a mock function object controlling the
// behavior of the method List.
ListFunc *GitHubAppsStoreListFunc
@ -78,11 +75,6 @@ func NewMockGitHubAppsStore() *MockGitHubAppsStore {
return
},
},
InstallFunc: &GitHubAppsStoreInstallFunc{
defaultHook: func(context.Context, int, int) (r0 error) {
return
},
},
ListFunc: &GitHubAppsStoreListFunc{
defaultHook: func(context.Context) (r0 []*types.GitHubApp, r1 error) {
return
@ -130,11 +122,6 @@ func NewStrictMockGitHubAppsStore() *MockGitHubAppsStore {
panic("unexpected invocation of MockGitHubAppsStore.GetBySlug")
},
},
InstallFunc: &GitHubAppsStoreInstallFunc{
defaultHook: func(context.Context, int, int) error {
panic("unexpected invocation of MockGitHubAppsStore.Install")
},
},
ListFunc: &GitHubAppsStoreListFunc{
defaultHook: func(context.Context) ([]*types.GitHubApp, error) {
panic("unexpected invocation of MockGitHubAppsStore.List")
@ -173,9 +160,6 @@ func NewMockGitHubAppsStoreFrom(i GitHubAppsStore) *MockGitHubAppsStore {
GetBySlugFunc: &GitHubAppsStoreGetBySlugFunc{
defaultHook: i.GetBySlug,
},
InstallFunc: &GitHubAppsStoreInstallFunc{
defaultHook: i.Install,
},
ListFunc: &GitHubAppsStoreListFunc{
defaultHook: i.List,
},
@ -731,114 +715,6 @@ func (c GitHubAppsStoreGetBySlugFuncCall) Results() []interface{} {
return []interface{}{c.Result0, c.Result1}
}
// GitHubAppsStoreInstallFunc describes the behavior when the Install method
// of the parent MockGitHubAppsStore instance is invoked.
type GitHubAppsStoreInstallFunc struct {
defaultHook func(context.Context, int, int) error
hooks []func(context.Context, int, int) error
history []GitHubAppsStoreInstallFuncCall
mutex sync.Mutex
}
// Install delegates to the next hook function in the queue and stores the
// parameter and result values of this invocation.
func (m *MockGitHubAppsStore) Install(v0 context.Context, v1 int, v2 int) error {
r0 := m.InstallFunc.nextHook()(v0, v1, v2)
m.InstallFunc.appendCall(GitHubAppsStoreInstallFuncCall{v0, v1, v2, r0})
return r0
}
// SetDefaultHook sets function that is called when the Install method of
// the parent MockGitHubAppsStore instance is invoked and the hook queue is
// empty.
func (f *GitHubAppsStoreInstallFunc) SetDefaultHook(hook func(context.Context, int, int) error) {
f.defaultHook = hook
}
// PushHook adds a function to the end of hook queue. Each invocation of the
// Install method of the parent MockGitHubAppsStore instance invokes the
// hook at the front of the queue and discards it. After the queue is empty,
// the default hook function is invoked for any future action.
func (f *GitHubAppsStoreInstallFunc) PushHook(hook func(context.Context, int, int) error) {
f.mutex.Lock()
f.hooks = append(f.hooks, hook)
f.mutex.Unlock()
}
// SetDefaultReturn calls SetDefaultHook with a function that returns the
// given values.
func (f *GitHubAppsStoreInstallFunc) SetDefaultReturn(r0 error) {
f.SetDefaultHook(func(context.Context, int, int) error {
return r0
})
}
// PushReturn calls PushHook with a function that returns the given values.
func (f *GitHubAppsStoreInstallFunc) PushReturn(r0 error) {
f.PushHook(func(context.Context, int, int) error {
return r0
})
}
func (f *GitHubAppsStoreInstallFunc) nextHook() func(context.Context, int, int) error {
f.mutex.Lock()
defer f.mutex.Unlock()
if len(f.hooks) == 0 {
return f.defaultHook
}
hook := f.hooks[0]
f.hooks = f.hooks[1:]
return hook
}
func (f *GitHubAppsStoreInstallFunc) appendCall(r0 GitHubAppsStoreInstallFuncCall) {
f.mutex.Lock()
f.history = append(f.history, r0)
f.mutex.Unlock()
}
// History returns a sequence of GitHubAppsStoreInstallFuncCall objects
// describing the invocations of this function.
func (f *GitHubAppsStoreInstallFunc) History() []GitHubAppsStoreInstallFuncCall {
f.mutex.Lock()
history := make([]GitHubAppsStoreInstallFuncCall, len(f.history))
copy(history, f.history)
f.mutex.Unlock()
return history
}
// GitHubAppsStoreInstallFuncCall is an object that describes an invocation
// of method Install on an instance of MockGitHubAppsStore.
type GitHubAppsStoreInstallFuncCall struct {
// Arg0 is the value of the 1st argument passed to this method
// invocation.
Arg0 context.Context
// Arg1 is the value of the 2nd argument passed to this method
// invocation.
Arg1 int
// Arg2 is the value of the 3rd argument passed to this method
// invocation.
Arg2 int
// Result0 is the value of the 1st result returned from this method
// invocation.
Result0 error
}
// Args returns an interface slice containing the arguments of this
// invocation.
func (c GitHubAppsStoreInstallFuncCall) Args() []interface{} {
return []interface{}{c.Arg0, c.Arg1, c.Arg2}
}
// Results returns an interface slice containing the results of this
// invocation.
func (c GitHubAppsStoreInstallFuncCall) Results() []interface{} {
return []interface{}{c.Result0}
}
// GitHubAppsStoreListFunc describes the behavior when the List method of
// the parent MockGitHubAppsStore instance is invoked.
type GitHubAppsStoreListFunc struct {

View File

@ -24,9 +24,6 @@ type GitHubAppsStore interface {
// Update updates a GitHub App in the database and returns the updated struct.
Update(ctx context.Context, id int, app *types.GitHubApp) (*types.GitHubApp, error)
// Install creates a new GitHub App installation in the database.
Install(ctx context.Context, id, installationID int) error
// GetByID retrieves a GitHub App from the database by ID.
GetByID(ctx context.Context, id int) (*types.GitHubApp, error)
@ -181,17 +178,6 @@ func (s *gitHubAppsStore) Update(ctx context.Context, id int, app *types.GitHubA
return apps[0], nil
}
// Install creates a new GitHub App installation in the database.
func (s *gitHubAppsStore) Install(ctx context.Context, id, installationID int) error {
query := sqlf.Sprintf(`
INSERT INTO github_app_installs (app_id, installation_id)
VALUES (%s, %s)
ON CONFLICT DO NOTHING
RETURNING id`,
id, installationID)
return s.Exec(ctx, query)
}
func (s *gitHubAppsStore) get(ctx context.Context, where *sqlf.Query) (*types.GitHubApp, error) {
selectQuery := `SELECT
id,

View File

@ -3,7 +3,6 @@ package store
import (
"context"
"testing"
"time"
"github.com/keegancsmith/sqlf"
"github.com/stretchr/testify/require"
@ -325,55 +324,3 @@ func TestGetBySlug(t *testing.T) {
_, err = store.GetBySlug(ctx, "foo", "bar")
require.Error(t, err)
}
func TestInstallGitHubApp(t *testing.T) {
if testing.Short() {
t.Skip()
}
logger := logtest.Scoped(t)
db := database.NewDB(logger, dbtest.NewDB(logger, t))
store := &gitHubAppsStore{Store: basestore.NewWithHandle(db.Handle())}
ctx := context.Background()
app := &types.GitHubApp{
AppID: 1,
Name: "Test App",
Slug: "test-app",
ClientID: "abc123",
ClientSecret: "secret",
PrivateKey: "private-key",
Logo: "logo.png",
}
id, err := store.Create(ctx, app)
require.NoError(t, err)
installationID := 42
err = store.Install(ctx, id, installationID)
require.NoError(t, err)
var fetchedID, fetchedInstallID int
var createdAt time.Time
query := sqlf.Sprintf(`SELECT app_id, installation_id, created_at FROM github_app_installs WHERE app_id=%s AND installation_id = %s`, id, installationID)
err = store.QueryRow(ctx, query).Scan(
&fetchedID,
&fetchedInstallID,
&createdAt,
)
require.NoError(t, err)
require.NotZero(t, createdAt)
// installing with the same ID results in a noop
err = store.Install(ctx, id, installationID)
require.NoError(t, err)
var createdAt2 time.Time
err = store.QueryRow(ctx, query).Scan(
&fetchedID,
&fetchedInstallID,
&createdAt2,
)
require.NoError(t, err)
require.Equal(t, createdAt, createdAt2)
}

View File

@ -699,15 +699,6 @@
"Increment": 1,
"CycleOption": "NO"
},
{
"Name": "github_app_installs_id_seq",
"TypeName": "integer",
"StartValue": 1,
"MinimumValue": 1,
"MaximumValue": 2147483647,
"Increment": 1,
"CycleOption": "NO"
},
{
"Name": "github_apps_id_seq",
"TypeName": "integer",
@ -12207,106 +12198,6 @@
],
"Triggers": []
},
{
"Name": "github_app_installs",
"Comment": "",
"Columns": [
{
"Name": "app_id",
"Index": 2,
"TypeName": "integer",
"IsNullable": false,
"Default": "",
"CharacterMaximumLength": 0,
"IsIdentity": false,
"IdentityGeneration": "",
"IsGenerated": "NEVER",
"GenerationExpression": "",
"Comment": ""
},
{
"Name": "created_at",
"Index": 4,
"TypeName": "timestamp with time zone",
"IsNullable": false,
"Default": "now()",
"CharacterMaximumLength": 0,
"IsIdentity": false,
"IdentityGeneration": "",
"IsGenerated": "NEVER",
"GenerationExpression": "",
"Comment": ""
},
{
"Name": "id",
"Index": 1,
"TypeName": "integer",
"IsNullable": false,
"Default": "nextval('github_app_installs_id_seq'::regclass)",
"CharacterMaximumLength": 0,
"IsIdentity": false,
"IdentityGeneration": "",
"IsGenerated": "NEVER",
"GenerationExpression": "",
"Comment": ""
},
{
"Name": "installation_id",
"Index": 3,
"TypeName": "integer",
"IsNullable": false,
"Default": "",
"CharacterMaximumLength": 0,
"IsIdentity": false,
"IdentityGeneration": "",
"IsGenerated": "NEVER",
"GenerationExpression": "",
"Comment": ""
}
],
"Indexes": [
{
"Name": "github_app_installs_pkey",
"IsPrimaryKey": true,
"IsUnique": true,
"IsExclusion": false,
"IsDeferrable": false,
"IndexDefinition": "CREATE UNIQUE INDEX github_app_installs_pkey ON github_app_installs USING btree (id)",
"ConstraintType": "p",
"ConstraintDefinition": "PRIMARY KEY (id)"
},
{
"Name": "app_id_idx",
"IsPrimaryKey": false,
"IsUnique": false,
"IsExclusion": false,
"IsDeferrable": false,
"IndexDefinition": "CREATE INDEX app_id_idx ON github_app_installs USING btree (app_id)",
"ConstraintType": "",
"ConstraintDefinition": ""
},
{
"Name": "installation_id_idx",
"IsPrimaryKey": false,
"IsUnique": false,
"IsExclusion": false,
"IsDeferrable": false,
"IndexDefinition": "CREATE INDEX installation_id_idx ON github_app_installs USING btree (installation_id)",
"ConstraintType": "",
"ConstraintDefinition": ""
}
],
"Constraints": [
{
"Name": "github_app_installs_app_id_fkey",
"ConstraintType": "f",
"RefTableName": "github_apps",
"IsDeferrable": false,
"ConstraintDefinition": "FOREIGN KEY (app_id) REFERENCES github_apps(id) ON DELETE CASCADE"
}
],
"Triggers": []
},
{
"Name": "github_apps",
"Comment": "",

View File

@ -1645,23 +1645,6 @@ Referenced by:
**rollout**: Rollout only defined when flag_type is rollout. Increments of 0.01%
# Table "public.github_app_installs"
```
Column | Type | Collation | Nullable | Default
-----------------+--------------------------+-----------+----------+-------------------------------------------------
id | integer | | not null | nextval('github_app_installs_id_seq'::regclass)
app_id | integer | | not null |
installation_id | integer | | not null |
created_at | timestamp with time zone | | not null | now()
Indexes:
"github_app_installs_pkey" PRIMARY KEY, btree (id)
"app_id_idx" btree (app_id)
"installation_id_idx" btree (installation_id)
Foreign-key constraints:
"github_app_installs_app_id_fkey" FOREIGN KEY (app_id) REFERENCES github_apps(id) ON DELETE CASCADE
```
# Table "public.github_apps"
```
Column | Type | Collation | Nullable | Default
@ -1685,8 +1668,6 @@ Indexes:
"github_apps_app_id_slug_base_url_unique" UNIQUE, btree (app_id, slug, base_url)
Foreign-key constraints:
"github_apps_webhook_id_fkey" FOREIGN KEY (webhook_id) REFERENCES webhooks(id) ON DELETE SET NULL
Referenced by:
TABLE "github_app_installs" CONSTRAINT "github_app_installs_app_id_fkey" FOREIGN KEY (app_id) REFERENCES github_apps(id) ON DELETE CASCADE
```

View File

@ -967,6 +967,9 @@ go_library(
"frontend/1683913757_add_ranking_progress_columns/down.sql",
"frontend/1683913757_add_ranking_progress_columns/metadata.yaml",
"frontend/1683913757_add_ranking_progress_columns/up.sql",
"frontend/1684398004_drop_github_app_installs_table/down.sql",
"frontend/1684398004_drop_github_app_installs_table/metadata.yaml",
"frontend/1684398004_drop_github_app_installs_table/up.sql",
],
importpath = "github.com/sourcegraph/sourcegraph/migrations",
visibility = ["//visibility:public"],

View File

@ -0,0 +1,10 @@
-- This repeats migration 1682598027 that introduced the table
CREATE TABLE IF NOT EXISTS github_app_installs (
id SERIAL PRIMARY KEY,
app_id INT NOT NULL REFERENCES github_apps(id) ON DELETE CASCADE,
installation_id INT NOT NULL,
created_at TIMESTAMPTZ NOT NULL DEFAULT NOW()
);
CREATE INDEX IF NOT EXISTS installation_id_idx ON github_app_installs USING btree (installation_id);
CREATE INDEX IF NOT EXISTS app_id_idx ON github_app_installs USING btree (app_id);

View File

@ -0,0 +1,2 @@
name: drop github_app_installs table
parents: [1683913757, 1684306784, 1683924275]

View File

@ -0,0 +1 @@
DROP TABLE IF EXISTS github_app_installs;

View File

@ -2424,23 +2424,6 @@ COMMENT ON CONSTRAINT required_bool_fields ON feature_flags IS 'Checks that bool
COMMENT ON CONSTRAINT required_rollout_fields ON feature_flags IS 'Checks that rollout is set IFF flag_type = rollout';
CREATE TABLE github_app_installs (
id integer NOT NULL,
app_id integer NOT NULL,
installation_id integer NOT NULL,
created_at timestamp with time zone DEFAULT now() NOT NULL
);
CREATE SEQUENCE github_app_installs_id_seq
AS integer
START WITH 1
INCREMENT BY 1
NO MINVALUE
NO MAXVALUE
CACHE 1;
ALTER SEQUENCE github_app_installs_id_seq OWNED BY github_app_installs.id;
CREATE TABLE github_apps (
id integer NOT NULL,
app_id integer NOT NULL,
@ -4782,8 +4765,6 @@ ALTER TABLE ONLY explicit_permissions_bitbucket_projects_jobs ALTER COLUMN id SE
ALTER TABLE ONLY external_services ALTER COLUMN id SET DEFAULT nextval('external_services_id_seq'::regclass);
ALTER TABLE ONLY github_app_installs ALTER COLUMN id SET DEFAULT nextval('github_app_installs_id_seq'::regclass);
ALTER TABLE ONLY github_apps ALTER COLUMN id SET DEFAULT nextval('github_apps_id_seq'::regclass);
ALTER TABLE ONLY gitserver_relocator_jobs ALTER COLUMN id SET DEFAULT nextval('gitserver_relocator_jobs_id_seq'::regclass);
@ -5127,9 +5108,6 @@ ALTER TABLE ONLY feature_flag_overrides
ALTER TABLE ONLY feature_flags
ADD CONSTRAINT feature_flags_pkey PRIMARY KEY (flag_name);
ALTER TABLE ONLY github_app_installs
ADD CONSTRAINT github_app_installs_pkey PRIMARY KEY (id);
ALTER TABLE ONLY github_apps
ADD CONSTRAINT github_apps_pkey PRIMARY KEY (id);
@ -5430,8 +5408,6 @@ CREATE INDEX access_requests_status ON access_requests USING btree (status);
CREATE INDEX access_tokens_lookup ON access_tokens USING hash (value_sha256) WHERE (deleted_at IS NULL);
CREATE INDEX app_id_idx ON github_app_installs USING btree (app_id);
CREATE INDEX assigned_owners_file_path ON assigned_owners USING btree (file_path_id);
CREATE INDEX batch_changes_namespace_org_id ON batch_changes USING btree (namespace_org_id);
@ -5662,8 +5638,6 @@ CREATE INDEX insights_query_runner_jobs_series_id_state ON insights_query_runner
CREATE INDEX insights_query_runner_jobs_state_btree ON insights_query_runner_jobs USING btree (state);
CREATE INDEX installation_id_idx ON github_app_installs USING btree (installation_id);
CREATE UNIQUE INDEX kind_cloud_default ON external_services USING btree (kind, cloud_default) WHERE ((cloud_default = true) AND (deleted_at IS NULL));
CREATE INDEX lsif_configuration_policies_repository_id ON lsif_configuration_policies USING btree (repository_id);
@ -6257,9 +6231,6 @@ ALTER TABLE ONLY vulnerability_affected_symbols
ALTER TABLE ONLY vulnerability_matches
ADD CONSTRAINT fk_vulnerability_affected_packages FOREIGN KEY (vulnerability_affected_package_id) REFERENCES vulnerability_affected_packages(id) ON DELETE CASCADE;
ALTER TABLE ONLY github_app_installs
ADD CONSTRAINT github_app_installs_app_id_fkey FOREIGN KEY (app_id) REFERENCES github_apps(id) ON DELETE CASCADE;
ALTER TABLE ONLY github_apps
ADD CONSTRAINT github_apps_webhook_id_fkey FOREIGN KEY (webhook_id) REFERENCES webhooks(id) ON DELETE SET NULL;