Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MM-33077] Add migrations for OAuthApps schema #17209

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
92 changes: 92 additions & 0 deletions db/migrations/bindata.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

46 changes: 46 additions & 0 deletions db/migrations/mysql/000046_create_oauth_apps.down.sql
@@ -0,0 +1,46 @@
SET @preparedStatement = (SELECT IF(
(
SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS
WHERE table_name = 'OAuthApps'
AND table_schema = DATABASE()
AND column_name = 'IconURL'
) > 0,
'ALTER TABLE OAuthApps DROP COLUMN IconURL;',
'SELECT 1'
));

PREPARE alterIfExists FROM @preparedStatement;
EXECUTE alterIfExists;
DEALLOCATE PREPARE alterIfExists;

SET @preparedStatement = (SELECT IF(
(
SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS
WHERE table_name = 'OAuthApps'
AND table_schema = DATABASE()
AND column_name = 'IsTrusted'
) > 0,
'ALTER TABLE OAuthApps DROP COLUMN IsTrusted;',
'SELECT 1'
));

PREPARE alterIfExists FROM @preparedStatement;
EXECUTE alterIfExists;
DEALLOCATE PREPARE alterIfExists;

SET @preparedStatement = (SELECT IF(
(
SELECT COUNT(*) FROM INFORMATION_SCHEMA.STATISTICS
WHERE table_name = 'OAuthApps'
AND table_schema = DATABASE()
AND index_name = 'idx_oauthapps_creator_id'
) > 0,
'DROP INDEX idx_oauthapps_creator_id ON OAuthApps;',
'SELECT 1'
));

PREPARE removeIndexIfExists FROM @preparedStatement;
EXECUTE removeIndexIfExists;
DEALLOCATE PREPARE removeIndexIfExists;

DROP TABLE IF EXISTS OAuthApps;
57 changes: 57 additions & 0 deletions db/migrations/mysql/000046_create_oauth_apps.up.sql
@@ -0,0 +1,57 @@
CREATE TABLE IF NOT EXISTS OAuthApps (
Id varchar(26) NOT NULL,
CreatorId varchar(26) DEFAULT NULL,
CreateAt bigint(20) DEFAULT NULL,
UpdateAt bigint(20) DEFAULT NULL,
ClientSecret varchar(128) DEFAULT NULL,
Name varchar(64) DEFAULT NULL,
Description text,
CallbackUrls text,
Homepage text,
PRIMARY KEY (Id)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;

SET @preparedStatement = (SELECT IF(
(
SELECT COUNT(*) FROM INFORMATION_SCHEMA.STATISTICS
WHERE table_name = 'OAuthApps'
AND table_schema = DATABASE()
AND index_name = 'idx_oauthapps_creator_id'
) > 0,
'SELECT 1',
'CREATE INDEX idx_oauthapps_creator_id ON OAuthApps(CreatorId);'
));

PREPARE createIndexIfNotExists FROM @preparedStatement;
EXECUTE createIndexIfNotExists;
DEALLOCATE PREPARE createIndexIfNotExists;

SET @preparedStatement = (SELECT IF(
(
SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS
WHERE table_name = 'OAuthApps'
AND table_schema = DATABASE()
AND column_name = 'IsTrusted'
) > 0,
'SELECT 1',
'ALTER TABLE OAuthApps ADD IsTrusted tinyint(1) DEFAULT 0;'
));

PREPARE alterIfNotExists FROM @preparedStatement;
EXECUTE alterIfNotExists;
DEALLOCATE PREPARE alterIfNotExists;

SET @preparedStatement = (SELECT IF(
(
SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS
WHERE table_name = 'OAuthApps'
AND table_schema = DATABASE()
AND column_name = 'IconURL'
) > 0,
'SELECT 1',
'ALTER TABLE OAuthApps ADD IconURL varchar(512) DEFAULT "";'
));

PREPARE alterIfNotExists FROM @preparedStatement;
EXECUTE alterIfNotExists;
DEALLOCATE PREPARE alterIfNotExists;
6 changes: 6 additions & 0 deletions db/migrations/postgres/000046_create_oauth_apps.down.sql
@@ -0,0 +1,6 @@
ALTER TABLE oauthapps DROP COLUMN IF EXISTS iconurl;
ALTER TABLE oauthapps DROP COLUMN IF EXISTS istrusted;

DROP INDEX IF EXISTS idx_oauthapps_creator_id ON oauthapps;

DROP TABLE IF EXISTS oauthapps;
16 changes: 16 additions & 0 deletions db/migrations/postgres/000046_create_oauth_apps.up.sql
@@ -0,0 +1,16 @@
CREATE TABLE IF NOT EXISTS oauthapps (
id VARCHAR(26) PRIMARY KEY,
creatorid VARCHAR(26),
createat bigint,
updateat bigint,
clientsecret VARCHAR(128),
name VARCHAR(64),
description VARCHAR(512),
callbackurls VARCHAR(1024),
homepage VARCHAR(256)
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Another general question: I am wondering if it would be simpler for the table creation code to have the most recent version of the table. I am thinking it would be the same logic wise.

Copy link
Member

Choose a reason for hiding this comment

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

But in that case we would lose the history right? And if we are going to include the down scripts with the reverse steps it wouldn't be consistent. cc @mgdelacroix


CREATE INDEX IF NOT EXISTS idx_oauthapps_creator_id ON oauthapps (creatorid);

ALTER TABLE oauthapps ADD COLUMN IF NOT EXISTS istrusted boolean DEFAULT false;
ALTER TABLE oauthapps ADD COLUMN IF NOT EXISTS iconurl VARCHAR(512) DEFAULT '';
Copy link
Contributor

Choose a reason for hiding this comment

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

In the current system we don't set defaults on creation and usually we prefer to avoid them. I am wondering if we should change these as well.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, makes sense 👍

4 changes: 2 additions & 2 deletions scripts/mattermost-mysql-5.0.sql
Expand Up @@ -491,10 +491,10 @@ CREATE TABLE `OAuthApps` (
`ClientSecret` varchar(128) DEFAULT NULL,
`Name` varchar(64) DEFAULT NULL,
`Description` text,
`IconURL` text,
`CallbackUrls` text,
`Homepage` text,
`IsTrusted` tinyint(1) DEFAULT NULL,
`IsTrusted` tinyint(1) DEFAULT '0',
`IconURL` varchar(512) DEFAULT '',
PRIMARY KEY (`Id`),
KEY `idx_oauthapps_creator_id` (`CreatorId`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;
Expand Down
5 changes: 2 additions & 3 deletions scripts/mattermost-postgresql-5.0.sql
Expand Up @@ -313,10 +313,10 @@ CREATE TABLE public.oauthapps (
clientsecret character varying(128),
name character varying(64),
description character varying(512),
iconurl character varying(512),
callbackurls character varying(1024),
homepage character varying(256),
istrusted boolean
istrusted boolean DEFAULT false,
iconurl character varying(512) DEFAULT ''::character varying
);


Expand Down Expand Up @@ -1816,4 +1816,3 @@ GRANT ALL ON SCHEMA public TO PUBLIC;
--
-- PostgreSQL database dump complete
--

6 changes: 0 additions & 6 deletions store/sqlstore/oauth_store.go
Expand Up @@ -53,12 +53,6 @@ func newSqlOAuthStore(sqlStore *SqlStore) store.OAuthStore {
return as
}

func (as SqlOAuthStore) createIndexesIfNotExists() {
as.CreateIndexIfNotExists("idx_oauthapps_creator_id", "OAuthApps", "CreatorId")
as.CreateIndexIfNotExists("idx_oauthaccessdata_user_id", "OAuthAccessData", "UserId")
as.CreateIndexIfNotExists("idx_oauthaccessdata_refresh_token", "OAuthAccessData", "RefreshToken")
Comment on lines -58 to -59
Copy link
Contributor

Choose a reason for hiding this comment

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

We are removing stuff for OAuthAccessData but the PR merely focuses on OAuthApps.

Copy link
Member

Choose a reason for hiding this comment

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

}

func (as SqlOAuthStore) SaveApp(app *model.OAuthApp) (*model.OAuthApp, error) {
if app.Id != "" {
return nil, store.NewErrInvalidInput("OAuthApp", "Id", app.Id)
Expand Down
1 change: 0 additions & 1 deletion store/sqlstore/store.go
Expand Up @@ -260,7 +260,6 @@ func New(settings model.SqlSettings, metrics einterfaces.MetricsInterface) *SqlS
store.stores.channel.(*SqlChannelStore).createIndexesIfNotExists()
store.stores.retentionPolicy.(*SqlRetentionPolicyStore).createIndexesIfNotExists()
store.stores.user.(*SqlUserStore).createIndexesIfNotExists()
store.stores.oauth.(*SqlOAuthStore).createIndexesIfNotExists()
Copy link
Contributor

Choose a reason for hiding this comment

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

This also affects other tables that are not covered in here.

Copy link
Member

Choose a reason for hiding this comment

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

Related with above.

store.stores.system.(*SqlSystemStore).createIndexesIfNotExists()
store.stores.emoji.(*SqlEmojiStore).createIndexesIfNotExists()
store.stores.fileInfo.(*SqlFileInfoStore).createIndexesIfNotExists()
Expand Down
3 changes: 0 additions & 3 deletions store/sqlstore/upgrade.go
Expand Up @@ -305,9 +305,6 @@ func upgradeDatabaseToVersion33(sqlStore *SqlStore) {
}
}

sqlStore.CreateColumnIfNotExists("OAuthApps", "IsTrusted", "tinyint(1)", "boolean", "0")
sqlStore.CreateColumnIfNotExists("OAuthApps", "IconURL", "varchar(512)", "varchar(512)", "")

sqlStore.RemoveColumnIfExists("Users", "LastActivityAt")
sqlStore.RemoveColumnIfExists("Users", "LastPingAt")

Expand Down