From beb2f8ee66a0c27a84467881fea992b5370131e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Luis=20Segura=20Lucas?= Date: Tue, 19 Oct 2021 17:55:17 +0200 Subject: [PATCH 1/4] Update rule_id field in advisor_ratings --- ...mig_0020_add_user_id_to_advisor_ratings.go | 79 +++++++++++++++++++ migration/migrations.go | 1 + storage/rating.go | 11 +-- 3 files changed, 86 insertions(+), 5 deletions(-) create mode 100644 migration/mig_0020_add_user_id_to_advisor_ratings.go diff --git a/migration/mig_0020_add_user_id_to_advisor_ratings.go b/migration/mig_0020_add_user_id_to_advisor_ratings.go new file mode 100644 index 00000000..64e1e880 --- /dev/null +++ b/migration/mig_0020_add_user_id_to_advisor_ratings.go @@ -0,0 +1,79 @@ +/* +Copyright © 2020 Red Hat, Inc. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package migration + +import ( + "database/sql" + + "github.com/RedHatInsights/insights-results-aggregator/types" +) + +var mig0020ModifyAdvisorRatingsTable = Migration{ + StepUp: func(tx *sql.Tx, driver types.DBDriver) error { + // common code for renaming column and adding a new one + _, err := tx.Exec(` + ALTER TABLE advisor_ratings RENAME COLUMN rule_id TO rule_fqdn; + ALTER TABLE advisor_ratings ADD COLUMN rule_id VARCHAR NOT NULL DEFAULT '.'; + `) + + if err != nil { + return err + } + + if driver == types.DBDriverSQLite3 { + // Update new column with values from older ones + // Sqlite uses || to concatenate strings ???? + _, err = tx.Exec(` + UPDATE advisor_ratings SET rule_id = rule_fqdn || '|' || error_key; + `) + + return err + } + + // only postgres + // Rename rule_id to rule_fqdn + _, err = tx.Exec(` + UPDATE advisor_ratings SET rule_id = CONCAT(rule_fqdn, '|', error_key); + `) + + return err + }, + StepDown: func(tx *sql.Tx, driver types.DBDriver) error { + // Remove the rule_id column + if driver == types.DBDriverPostgres { + _, err := tx.Exec(` + ALTER TABLE advisor_ratings DROP COLUMN IF EXISTS rule_id; + `) + if err != nil { + return err + } + } else if driver == types.DBDriverSQLite3 { + // sqlite, why so serious? + _, err := tx.Exec(` + CREATE TABLE advisor_ratings_temp AS SELECT user_id, org_id, rule_fqdn, error_key, rated_at, last_updated_at, rating FROM advisor_ratings; + DROP TABLE advisor_ratings; + ALTER TABLE advisor_ratings_temp RENAME TO advisor_ratings; + `) + if err != nil { + return err + } + } + + // Rename rule_fqdn back to rule_id + _, err := tx.Exec(` + ALTER TABLE advisor_ratings RENAME COLUMN rule_fqdn TO rule_id; + `) + return err + }, +} diff --git a/migration/migrations.go b/migration/migrations.go index f678aeea..ea110c6e 100644 --- a/migration/migrations.go +++ b/migration/migrations.go @@ -36,4 +36,5 @@ var migrations = []Migration{ mig0017AddSystemWideRuleDisableTable, mig0018AddRatingsTable, mig0019ModifyRecommendationTable, + mig0020ModifyAdvisorRatingsTable, } diff --git a/storage/rating.go b/storage/rating.go index a34d37f6..5472da57 100644 --- a/storage/rating.go +++ b/storage/rating.go @@ -27,16 +27,16 @@ import ( func (storage *DBStorage) RateOnRule( userID types.UserID, orgID types.OrgID, - ruleID types.RuleID, + ruleFqdn types.RuleID, errorKey types.ErrorKey, rating types.UserVote, ) error { query := ` INSERT INTO advisor_ratings - (user_id, org_id, rule_id, error_key, rated_at, last_updated_at, rating) + (user_id, org_id, rule_fqdn, error_key, rated_at, last_updated_at, rating, rule_id) VALUES - ($1, $2, $3, $4, $5, $6, $7) - ON CONFLICT (user_id, org_id, rule_id, error_key) DO UPDATE SET + ($1, $2, $3, $4, $5, $6, $7, $8) + ON CONFLICT (user_id, org_id, rule_fqdn, error_key) DO UPDATE SET last_updated_at = $6, rating = $7 ` statement, err := storage.connection.Prepare(query) @@ -52,7 +52,8 @@ func (storage *DBStorage) RateOnRule( }() now := time.Now() - _, err = statement.Exec(userID, orgID, ruleID, errorKey, now, now, rating) + ruleID := string(ruleFqdn) + "|" + string(errorKey) + _, err = statement.Exec(userID, orgID, ruleFqdn, errorKey, now, now, rating, ruleID) err = types.ConvertDBError(err, nil) if err != nil { log.Error().Err(err).Msg("RateOnRule") From c8af0d961faf04f50e938a72c41ad63d633717a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Luis=20Segura=20Lucas?= Date: Wed, 20 Oct 2021 11:37:09 +0200 Subject: [PATCH 2/4] Enhance script to generate docs --- config-devel.toml | 4 ++-- generate_db_schema_doc.sh | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/config-devel.toml b/config-devel.toml index 97a9eb67..5bea9e58 100644 --- a/config-devel.toml +++ b/config-devel.toml @@ -22,8 +22,8 @@ org_allowlist_file = "org_allowlist.csv" [storage] db_driver = "postgres" -pg_username = "user" -pg_password = "password" +pg_username = "postgres" +pg_password = "postgres" pg_host = "localhost" pg_port = 5432 pg_db_name = "aggregator" diff --git a/generate_db_schema_doc.sh b/generate_db_schema_doc.sh index eeea6e18..6dbe8749 100755 --- a/generate_db_schema_doc.sh +++ b/generate_db_schema_doc.sh @@ -16,6 +16,13 @@ # Script to generate documentation with DB schema description +function cleanUpAndExit { + if [[ $DB_KEEP_RUNNING == 0 ]]; then + podman stop $DB_POD_NAME + fi + exit $1 +} + # Settings for database to be documented # (can be taken from config.toml, config-devel.toml etc.) DB_ADDRESS=localhost @@ -23,7 +30,35 @@ DB_LOGIN=postgres DB_PASSWORD=postgres DB_NAME=aggregator +DB_POD_NAME=postgres_aggregator +DB_KEEP_RUNNING=1 + # Generate the documentation OUTPUT_DIR=docs/db-description-3 +# Launch local postgres from scratch +podman run --name=$DB_POD_NAME --rm --network=host -e POSTGRES_PASSWORD=$DB_PASSWORD -e POSTGRES_USER=$DB_LOGIN -e POSTGRES_DB=$DB_NAME -d postgres:10 + +if [[ $? != 0 ]]; then + echo "Cannot launch postgress database locally" + cleanUpAndExit 1 +fi + +# Run migration to latest +make # compiles aggregator +if [[ $? != 0 ]]; then + echo "Cannot build aggregator" + cleanUpAndExit 2 +fi + +sleep 5 +INSIGHTS_RESULTS_AGGREGATOR_CONFIG_FILE=./config-devel.toml ./insights-results-aggregator migration latest + +if [[ $? != 0 ]]; then + echo "Failure generating latest migration" + cleanUpAndExit 3 +fi + java -jar schemaspy-6.1.0.jar -cp . -t pgsql -u ${DB_LOGIN} -p ${DB_PASSWORD} -host ${DB_ADDRESS} -s public -o ${OUTPUT_DIR} -db ${DB_NAME} -dp postgresql-42.2.20.jre7.jar + +cleanUpAndExit $? \ No newline at end of file From d591e8fab1a90c02e4bd4f31d57cd8618d200ceb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Luis=20Segura=20Lucas?= Date: Wed, 20 Oct 2021 11:39:58 +0200 Subject: [PATCH 3/4] Updating DB schema --- docs/db-description/aggregator.public.xml | 28 +- docs/db-description/columns.html | 484 ++++++++++++++-------- docs/db-description/deletionOrder.txt | 1 + docs/db-description/index.html | 19 +- docs/db-description/info-html.txt | 2 +- docs/db-description/insertionOrder.txt | 1 + docs/db-description/orphans.html | 19 +- 7 files changed, 361 insertions(+), 193 deletions(-) diff --git a/docs/db-description/aggregator.public.xml b/docs/db-description/aggregator.public.xml index 54d02735..34ad916e 100644 --- a/docs/db-description/aggregator.public.xml +++ b/docs/db-description/aggregator.public.xml @@ -4,19 +4,20 @@ - + + - + - +
@@ -106,6 +107,8 @@ + + @@ -139,6 +142,25 @@ + + + + + + + + + + + + + + + + + + +
diff --git a/docs/db-description/columns.html b/docs/db-description/columns.html index 001110c7..a91fd813 100644 --- a/docs/db-description/columns.html +++ b/docs/db-description/columns.html @@ -105,39 +105,39 @@

Columns

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/generate_db_schema_doc.sh b/generate_db_schema_doc.sh index 6dbe8749..f75c56b0 100755 --- a/generate_db_schema_doc.sh +++ b/generate_db_schema_doc.sh @@ -37,28 +37,24 @@ DB_KEEP_RUNNING=1 OUTPUT_DIR=docs/db-description-3 # Launch local postgres from scratch -podman run --name=$DB_POD_NAME --rm --network=host -e POSTGRES_PASSWORD=$DB_PASSWORD -e POSTGRES_USER=$DB_LOGIN -e POSTGRES_DB=$DB_NAME -d postgres:10 - -if [[ $? != 0 ]]; then +if ! podman run --name=$DB_POD_NAME --rm --network=host -e POSTGRES_PASSWORD=$DB_PASSWORD -e POSTGRES_USER=$DB_LOGIN -e POSTGRES_DB=$DB_NAME -d postgres:10; then echo "Cannot launch postgress database locally" cleanUpAndExit 1 fi # Run migration to latest -make # compiles aggregator -if [[ $? != 0 ]]; then +if ! make; then echo "Cannot build aggregator" cleanUpAndExit 2 fi sleep 5 -INSIGHTS_RESULTS_AGGREGATOR_CONFIG_FILE=./config-devel.toml ./insights-results-aggregator migration latest +export INSIGHTS_RESULTS_AGGREGATOR_CONFIG_FILE=./config-devel.toml -if [[ $? != 0 ]]; then +if ! ./insights-results-aggregator migration latest; then echo "Failure generating latest migration" cleanUpAndExit 3 fi java -jar schemaspy-6.1.0.jar -cp . -t pgsql -u ${DB_LOGIN} -p ${DB_PASSWORD} -host ${DB_ADDRESS} -s public -o ${OUTPUT_DIR} -db ${DB_NAME} -dp postgresql-42.2.20.jre7.jar - -cleanUpAndExit $? \ No newline at end of file +cleanUpAndExit $? diff --git a/migration/mig_0020_add_user_id_to_advisor_ratings.go b/migration/mig_0020_add_user_id_to_advisor_ratings.go index 64e1e880..ec60633d 100644 --- a/migration/mig_0020_add_user_id_to_advisor_ratings.go +++ b/migration/mig_0020_add_user_id_to_advisor_ratings.go @@ -38,16 +38,17 @@ var mig0020ModifyAdvisorRatingsTable = Migration{ UPDATE advisor_ratings SET rule_id = rule_fqdn || '|' || error_key; `) + return err + } else if driver == types.DBDriverPostgres { + // only postgres + // Rename rule_id to rule_fqdn + _, err = tx.Exec(` + UPDATE advisor_ratings SET rule_id = CONCAT(rule_fqdn, '|', error_key); + `) return err } - // only postgres - // Rename rule_id to rule_fqdn - _, err = tx.Exec(` - UPDATE advisor_ratings SET rule_id = CONCAT(rule_fqdn, '|', error_key); - `) - - return err + return nil }, StepDown: func(tx *sql.Tx, driver types.DBDriver) error { // Remove the rule_id column