From a3d41b6c235b10f73bea80ecc0b4e35adf5b21a6 Mon Sep 17 00:00:00 2001 From: Frank Elsinga Date: Mon, 11 Mar 2024 02:32:02 +0100 Subject: [PATCH 01/12] introduced an index and made sure that the dishes are unique --- server/backend/cron/dish_name_download.go | 28 ++++++++++++---------- server/backend/migration/20240311000000.go | 20 ++++++++++++++++ server/backend/migration/migration.go | 1 + server/model/dish.go | 4 ++-- 4 files changed, 38 insertions(+), 15 deletions(-) create mode 100644 server/backend/migration/20240311000000.go diff --git a/server/backend/cron/dish_name_download.go b/server/backend/cron/dish_name_download.go index d94d315f..889ab85b 100644 --- a/server/backend/cron/dish_name_download.go +++ b/server/backend/cron/dish_name_download.go @@ -2,6 +2,7 @@ package cron import ( "encoding/json" + "errors" "fmt" "net/http" "strings" @@ -95,25 +96,26 @@ func downloadDailyDishes(c *CronService) { CafeteriaID: v.Cafeteria, } - var count int64 - var dishId int64 - if err := c.db.Model(&model.Dish{}). - Where("name = ? AND cafeteriaID = ?", dish.Name, dish.CafeteriaID). - Select("dish"). - First(&dishId). - Count(&count).Error; err != nil { - log.WithError(err).Error("Error while checking whether this is already in database") - } - if count == 0 { + var dbDish model.Dish + if err := c.db.First(&dbDish, "name = ? AND cafeteriaID = ?", dish.Name, dish.CafeteriaID).Error; err != nil && errors.Is(err, gorm.ErrRecordNotFound) { if err := c.db.Create(&dish).Error; err != nil { - log.WithError(err).Error("Error while creating new CanteenDish entry with name {}. CanteenDish won't be saved", dish.Name) + log.WithError(err).WithField("name", dish.Name).Error("Error while creating new CanteenDish entry. CanteenDish won't be saved") } addDishTagsToMapping(dish.Dish, dish.Name, c.db) - dishId = dish.Dish + dbDish = dish + } else if err != nil { + log.WithError(err).Error("Error while checking whether the dish is already in database") } + + if dbDish.Type != dish.Type { + if err := c.db.Where("dish = ?", dbDish.Dish).Updates(&dish).Error; err != nil { + log.WithError(err).WithField("from", dish.Type).WithField("to", dish.Type).Error("Error while updating dish to new type") + } + } + if weekliesWereAdded == 0 { errCreate := c.db.Create(&model.DishesOfTheWeek{ - DishID: dishId, + DishID: dbDish.Dish, Year: int32(year), Week: int32(week), Day: int32(weekDayIndex), diff --git a/server/backend/migration/20240311000000.go b/server/backend/migration/20240311000000.go new file mode 100644 index 00000000..2f4b2b20 --- /dev/null +++ b/server/backend/migration/20240311000000.go @@ -0,0 +1,20 @@ +package migration + +import ( + "github.com/go-gormigrate/gormigrate/v2" + "gorm.io/gorm" +) + +// migrate20240311000000 +// made sure that dishes have the correct indexes +func migrate20240311000000() *gormigrate.Migration { + return &gormigrate.Migration{ + ID: "20240311000000", + Migrate: func(tx *gorm.DB) error { + return tx.Raw("create unique index dish_name_cafeteriaID_uindex on dish (name, cafeteriaID)").Error + }, + Rollback: func(tx *gorm.DB) error { + return tx.Raw("drop index dish_name_cafeteriaID_uindex on dish").Error + }, + } +} diff --git a/server/backend/migration/migration.go b/server/backend/migration/migration.go index 06e368c7..3ed32523 100644 --- a/server/backend/migration/migration.go +++ b/server/backend/migration/migration.go @@ -72,6 +72,7 @@ func manualMigrate(db *gorm.DB) error { migrate20240103000000(), migrate20240207000000(), migrate20240212000000(), + migrate20240311000000(), } return gormigrate.New(db, gormigrateOptions, migrations).Migrate() } diff --git a/server/model/dish.go b/server/model/dish.go index 7ab5c4d7..72b89fa7 100644 --- a/server/model/dish.go +++ b/server/model/dish.go @@ -14,9 +14,9 @@ var ( // Dish represents one dish fin a specific cafeteria type Dish struct { Dish int64 `gorm:"primary_key;AUTO_INCREMENT;column:dish;type:int;not null;" json:"dish"` - Name string `gorm:"column:name;type:text;not null;" json:"name" ` + Name string `gorm:"column:name;type:text;not null;uniqueIndex:dish_name_cafeteriaID_uindex" json:"name" ` Type string `gorm:"column:type;type:text;not null;" json:"type" ` - CafeteriaID int64 `gorm:"column:cafeteriaID;foreignKey:cafeteria;type:int;not null;" json:"cafeteriaID"` + CafeteriaID int64 `gorm:"column:cafeteriaID;foreignKey:cafeteria;type:int;not null;uniqueIndex:dish_name_cafeteriaID_uindex" json:"cafeteriaID"` } // TableName sets the insert table name for this struct type From 0fe8302e535922d6ba16d89c26b2d53661be2283 Mon Sep 17 00:00:00 2001 From: Frank Elsinga Date: Tue, 12 Mar 2024 21:30:05 +0100 Subject: [PATCH 02/12] Update server/backend/migration/20240311000000.go --- server/backend/migration/20240311000000.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/backend/migration/20240311000000.go b/server/backend/migration/20240311000000.go index 2f4b2b20..dd082ef9 100644 --- a/server/backend/migration/20240311000000.go +++ b/server/backend/migration/20240311000000.go @@ -11,10 +11,10 @@ func migrate20240311000000() *gormigrate.Migration { return &gormigrate.Migration{ ID: "20240311000000", Migrate: func(tx *gorm.DB) error { - return tx.Raw("create unique index dish_name_cafeteriaID_uindex on dish (name, cafeteriaID)").Error + return tx.Exec("create unique index dish_name_cafeteriaID_uindex on dish (name, cafeteriaID)").Error }, Rollback: func(tx *gorm.DB) error { - return tx.Raw("drop index dish_name_cafeteriaID_uindex on dish").Error + return tx.Exec("drop index dish_name_cafeteriaID_uindex on dish").Error }, } } From ad9f390d9a8edfbfb5457a1474bd48daede003f1 Mon Sep 17 00:00:00 2001 From: Frank Elsinga Date: Wed, 13 Mar 2024 21:19:41 +0100 Subject: [PATCH 03/12] added the correct migration for `dish_ratings` FK-binding --- server/backend/migration/20240311000000.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/server/backend/migration/20240311000000.go b/server/backend/migration/20240311000000.go index dd082ef9..59cd41af 100644 --- a/server/backend/migration/20240311000000.go +++ b/server/backend/migration/20240311000000.go @@ -7,13 +7,35 @@ import ( // migrate20240311000000 // made sure that dishes have the correct indexes +// changed how `dish_ratings` is bound to `dish` func migrate20240311000000() *gormigrate.Migration { return &gormigrate.Migration{ ID: "20240311000000", Migrate: func(tx *gorm.DB) error { + // make sure that dish_ratings are FK-bound to dishes + if err := tx.Exec(`alter table dish_rating + add constraint dish_rating_dish_dish_fk + foreign key (dishID) references dish (dish) + on delete cascade;`).Error; err != nil { + return err + } + // because dishes already have a cafeteria, storing this again is not necessary + if err := tx.Exec(`alter table dish_rating drop column cafeteriaID`).Error; err != nil { + return err + } + // uniqueness return tx.Exec("create unique index dish_name_cafeteriaID_uindex on dish (name, cafeteriaID)").Error }, Rollback: func(tx *gorm.DB) error { + // make sure that dish_ratings are FK-bound to dishes + if err := tx.Exec(`alter table dish_rating drop constraint dish_rating_dish_dish_fk`).Error; err != nil { + return err + } + // because dishes already have a cafeteria, storing this agiain is not nessesary + if err := tx.Exec(`alter table dish_rating add column cafeteriaID int not null`).Error; err != nil { + return err + } + // uniqueness return tx.Exec("drop index dish_name_cafeteriaID_uindex on dish").Error }, } From 5e4360458afd04b5b0b3ea6ba162a520603b22b2 Mon Sep 17 00:00:00 2001 From: Frank Elsinga Date: Wed, 13 Mar 2024 21:29:15 +0100 Subject: [PATCH 04/12] modified the models.DishRating struct as well with the appropriate change --- server/backend/cafeteria.go | 11 +++++------ server/backend/migration/20240311000000.go | 19 ++++++++++--------- server/model/dish_rating.go | 14 +++++++------- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/server/backend/cafeteria.go b/server/backend/cafeteria.go index 6c91c030..9f447a96 100644 --- a/server/backend/cafeteria.go +++ b/server/backend/cafeteria.go @@ -409,12 +409,11 @@ func (s *CampusServer) CreateDishRating(ctx context.Context, input *pb.CreateDis resPath := imageWrapper(input.Image, "dishes", dishInMensa.Dish) rating := model.DishRating{ - Comment: input.Comment, - CafeteriaID: cafeteriaID, - DishID: dishInMensa.Dish, - Points: input.Points, - Timestamp: time.Now(), - Image: resPath, + Comment: input.Comment, + DishID: dishInMensa.Dish, + Points: input.Points, + Timestamp: time.Now(), + Image: resPath, } if err := tx.Create(&rating).Error; err != nil { log.WithError(err).Error("while creating a new dishInMensa rating.") diff --git a/server/backend/migration/20240311000000.go b/server/backend/migration/20240311000000.go index 59cd41af..48139ea8 100644 --- a/server/backend/migration/20240311000000.go +++ b/server/backend/migration/20240311000000.go @@ -7,32 +7,33 @@ import ( // migrate20240311000000 // made sure that dishes have the correct indexes -// changed how `dish_ratings` is bound to `dish` +// changed how `dish_rating` is bound to `dish` func migrate20240311000000() *gormigrate.Migration { return &gormigrate.Migration{ ID: "20240311000000", Migrate: func(tx *gorm.DB) error { - // make sure that dish_ratings are FK-bound to dishes + // make sure that dish_rating is FK-bound to dish if err := tx.Exec(`alter table dish_rating - add constraint dish_rating_dish_dish_fk - foreign key (dishID) references dish (dish) - on delete cascade;`).Error; err != nil { + add constraint dish_rating_dish_dish_fk + foreign key (dishID) references dish (dish) + on update cascade + on delete cascade;`).Error; err != nil { return err } // because dishes already have a cafeteria, storing this again is not necessary - if err := tx.Exec(`alter table dish_rating drop column cafeteriaID`).Error; err != nil { + if err := tx.Exec("alter table dish_rating drop column cafeteriaID").Error; err != nil { return err } // uniqueness return tx.Exec("create unique index dish_name_cafeteriaID_uindex on dish (name, cafeteriaID)").Error }, Rollback: func(tx *gorm.DB) error { - // make sure that dish_ratings are FK-bound to dishes - if err := tx.Exec(`alter table dish_rating drop constraint dish_rating_dish_dish_fk`).Error; err != nil { + // make sure that dish_rating is FK-bound to dishes + if err := tx.Exec("alter table dish_rating drop constraint dish_rating_dish_dish_fk").Error; err != nil { return err } // because dishes already have a cafeteria, storing this agiain is not nessesary - if err := tx.Exec(`alter table dish_rating add column cafeteriaID int not null`).Error; err != nil { + if err := tx.Exec("alter table dish_rating add column cafeteriaID int not null").Error; err != nil { return err } // uniqueness diff --git a/server/model/dish_rating.go b/server/model/dish_rating.go index 7b8e7f93..2a7fb85d 100644 --- a/server/model/dish_rating.go +++ b/server/model/dish_rating.go @@ -5,13 +5,13 @@ import ( ) type DishRating struct { - DishRating int64 `gorm:"primary_key;AUTO_INCREMENT;column:dishRating;type:int;not null;" json:"dishRating"` - Points int32 `gorm:"column:points;type:int;not null;" json:"points"` - CafeteriaID int64 `gorm:"column:cafeteriaID;foreignKey:cafeteria;type:int;not null;" json:"cafeteriaID"` - DishID int64 `gorm:"column:dishID;foreignKey:dish;type:int;not null;" json:"dishID"` - Comment string `gorm:"column:comment;type:text;" json:"comment"` - Timestamp time.Time `gorm:"column:timestamp;type:timestamp;not null;" json:"timestamp"` - Image string `gorm:"column:image;type:text;" json:"image"` + DishRating int64 `gorm:"primary_key;AUTO_INCREMENT;column:dishRating;type:int;not null;" json:"dishRating"` + Points int32 `gorm:"column:points;type:int;not null;" json:"points"` + DishID int64 `gorm:"column:dishID;type:int;not null;" json:"dishID"` + Dish Dish `gorm:"foreignKey:dishID;references:dish;constraint:OnUpdate:CASCADE,OnDelete:CASCADE;"` + Comment string `gorm:"column:comment;type:text;" json:"comment"` + Timestamp time.Time `gorm:"column:timestamp;type:timestamp;not null;" json:"timestamp"` + Image string `gorm:"column:image;type:text;" json:"image"` } // TableName sets the insert table name for this struct type From 9b2e6e0be1144bf914a63330dfc1d393551a117a Mon Sep 17 00:00:00 2001 From: Frank Elsinga Date: Wed, 13 Mar 2024 21:29:49 +0100 Subject: [PATCH 05/12] linting fixes --- server/backend/migration/20240311000000.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/backend/migration/20240311000000.go b/server/backend/migration/20240311000000.go index 48139ea8..6f421530 100644 --- a/server/backend/migration/20240311000000.go +++ b/server/backend/migration/20240311000000.go @@ -16,7 +16,7 @@ func migrate20240311000000() *gormigrate.Migration { if err := tx.Exec(`alter table dish_rating add constraint dish_rating_dish_dish_fk foreign key (dishID) references dish (dish) - on update cascade + on update cascade on delete cascade;`).Error; err != nil { return err } From fa657946bee3ac42df68c9d746ba22086378122e Mon Sep 17 00:00:00 2001 From: Frank Elsinga Date: Wed, 13 Mar 2024 21:32:34 +0100 Subject: [PATCH 06/12] tested a different way of formatting the migration --- server/backend/migration/20240311000000.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/backend/migration/20240311000000.go b/server/backend/migration/20240311000000.go index 6f421530..add5f9a9 100644 --- a/server/backend/migration/20240311000000.go +++ b/server/backend/migration/20240311000000.go @@ -16,8 +16,7 @@ func migrate20240311000000() *gormigrate.Migration { if err := tx.Exec(`alter table dish_rating add constraint dish_rating_dish_dish_fk foreign key (dishID) references dish (dish) - on update cascade - on delete cascade;`).Error; err != nil { + on update cascade on delete cascade`).Error; err != nil { return err } // because dishes already have a cafeteria, storing this again is not necessary From e0ad186eb20a2501fb1bea6ead189347da4006e0 Mon Sep 17 00:00:00 2001 From: Frank Elsinga Date: Wed, 13 Mar 2024 21:38:54 +0100 Subject: [PATCH 07/12] why does CI not like this??? --- server/backend/migration/20240311000000.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/server/backend/migration/20240311000000.go b/server/backend/migration/20240311000000.go index add5f9a9..31271b9b 100644 --- a/server/backend/migration/20240311000000.go +++ b/server/backend/migration/20240311000000.go @@ -13,10 +13,7 @@ func migrate20240311000000() *gormigrate.Migration { ID: "20240311000000", Migrate: func(tx *gorm.DB) error { // make sure that dish_rating is FK-bound to dish - if err := tx.Exec(`alter table dish_rating - add constraint dish_rating_dish_dish_fk - foreign key (dishID) references dish (dish) - on update cascade on delete cascade`).Error; err != nil { + if err := tx.Exec("alter table dish_rating add constraint dish_rating_dish_dish_fk foreign key (dishID) references dish (dish) on update cascade on delete cascade").Error; err != nil { return err } // because dishes already have a cafeteria, storing this again is not necessary From 121b2d59c9ba59506d4c2162e9d69940fa750d82 Mon Sep 17 00:00:00 2001 From: Frank Elsinga Date: Wed, 13 Mar 2024 21:59:50 +0100 Subject: [PATCH 08/12] fixed the typo in the `20240312000000` name --- server/backend/migration/20240312000000.go | 6 +++--- server/backend/migration/migration.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/backend/migration/20240312000000.go b/server/backend/migration/20240312000000.go index 2148d684..80a8d70e 100644 --- a/server/backend/migration/20240312000000.go +++ b/server/backend/migration/20240312000000.go @@ -5,11 +5,11 @@ import ( "gorm.io/gorm" ) -// migrate20240212000000 +// migrate20240312000000 // implemented a basic variant of spam protection -func migrate20240212000000() *gormigrate.Migration { +func migrate20240312000000() *gormigrate.Migration { return &gormigrate.Migration{ - ID: "20240212000000", + ID: "20240312000000", Migrate: func(tx *gorm.DB) error { if err := tx.Exec("alter table feedback modify email_id text charset utf8mb3 not null").Error; err != nil { return err diff --git a/server/backend/migration/migration.go b/server/backend/migration/migration.go index 3ed32523..5fbf5291 100644 --- a/server/backend/migration/migration.go +++ b/server/backend/migration/migration.go @@ -71,8 +71,8 @@ func manualMigrate(db *gorm.DB) error { migrate20240102000000(), migrate20240103000000(), migrate20240207000000(), - migrate20240212000000(), migrate20240311000000(), + migrate20240312000000(), } return gormigrate.New(db, gormigrateOptions, migrations).Migrate() } From acf5570069fbd597d0cefa1f65a333cc5e4c50eb Mon Sep 17 00:00:00 2001 From: Frank Elsinga Date: Wed, 13 Mar 2024 22:04:09 +0100 Subject: [PATCH 09/12] tested a different migration path --- server/backend/migration/20240311000000.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/server/backend/migration/20240311000000.go b/server/backend/migration/20240311000000.go index 31271b9b..0855f9a1 100644 --- a/server/backend/migration/20240311000000.go +++ b/server/backend/migration/20240311000000.go @@ -13,6 +13,9 @@ func migrate20240311000000() *gormigrate.Migration { ID: "20240311000000", Migrate: func(tx *gorm.DB) error { // make sure that dish_rating is FK-bound to dish + if err := tx.Exec("alter table dish_rating modify dishID int").Error; err != nil { + return err + } if err := tx.Exec("alter table dish_rating add constraint dish_rating_dish_dish_fk foreign key (dishID) references dish (dish) on update cascade on delete cascade").Error; err != nil { return err } @@ -28,6 +31,9 @@ func migrate20240311000000() *gormigrate.Migration { if err := tx.Exec("alter table dish_rating drop constraint dish_rating_dish_dish_fk").Error; err != nil { return err } + if err := tx.Exec("alter table dish_rating modify dishID int not null").Error; err != nil { + return err + } // because dishes already have a cafeteria, storing this agiain is not nessesary if err := tx.Exec("alter table dish_rating add column cafeteriaID int not null").Error; err != nil { return err From 95a4f80dcfae496e873b161873e823feba792069 Mon Sep 17 00:00:00 2001 From: Frank Elsinga Date: Wed, 13 Mar 2024 22:28:09 +0100 Subject: [PATCH 10/12] different test --- server/backend/migration/20240311000000.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/backend/migration/20240311000000.go b/server/backend/migration/20240311000000.go index 0855f9a1..fd3f46f6 100644 --- a/server/backend/migration/20240311000000.go +++ b/server/backend/migration/20240311000000.go @@ -13,7 +13,7 @@ func migrate20240311000000() *gormigrate.Migration { ID: "20240311000000", Migrate: func(tx *gorm.DB) error { // make sure that dish_rating is FK-bound to dish - if err := tx.Exec("alter table dish_rating modify dishID int").Error; err != nil { + if err := tx.Exec("alter table dish modify dish int not null primary key auto_increment").Error; err != nil { return err } if err := tx.Exec("alter table dish_rating add constraint dish_rating_dish_dish_fk foreign key (dishID) references dish (dish) on update cascade on delete cascade").Error; err != nil { @@ -31,7 +31,7 @@ func migrate20240311000000() *gormigrate.Migration { if err := tx.Exec("alter table dish_rating drop constraint dish_rating_dish_dish_fk").Error; err != nil { return err } - if err := tx.Exec("alter table dish_rating modify dishID int not null").Error; err != nil { + if err := tx.Exec("alter table dish_rating modify dish int primary key auto_increment").Error; err != nil { return err } // because dishes already have a cafeteria, storing this agiain is not nessesary From 4d67b1847e30950717b6c9560150b4b8259cf931 Mon Sep 17 00:00:00 2001 From: Frank Elsinga Date: Wed, 13 Mar 2024 22:32:41 +0100 Subject: [PATCH 11/12] different test --- server/backend/migration/20240311000000.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/backend/migration/20240311000000.go b/server/backend/migration/20240311000000.go index fd3f46f6..45f5602d 100644 --- a/server/backend/migration/20240311000000.go +++ b/server/backend/migration/20240311000000.go @@ -13,7 +13,7 @@ func migrate20240311000000() *gormigrate.Migration { ID: "20240311000000", Migrate: func(tx *gorm.DB) error { // make sure that dish_rating is FK-bound to dish - if err := tx.Exec("alter table dish modify dish int not null primary key auto_increment").Error; err != nil { + if err := tx.Exec("alter table dish modify dish int not null auto_increment").Error; err != nil { return err } if err := tx.Exec("alter table dish_rating add constraint dish_rating_dish_dish_fk foreign key (dishID) references dish (dish) on update cascade on delete cascade").Error; err != nil { @@ -31,7 +31,7 @@ func migrate20240311000000() *gormigrate.Migration { if err := tx.Exec("alter table dish_rating drop constraint dish_rating_dish_dish_fk").Error; err != nil { return err } - if err := tx.Exec("alter table dish_rating modify dish int primary key auto_increment").Error; err != nil { + if err := tx.Exec("alter table dish modify dish int auto_increment").Error; err != nil { return err } // because dishes already have a cafeteria, storing this agiain is not nessesary From f064ce50615f21b1c0b8a12f7b38fe1a25b15f28 Mon Sep 17 00:00:00 2001 From: Frank Elsinga Date: Wed, 13 Mar 2024 22:35:30 +0100 Subject: [PATCH 12/12] different test --- server/backend/migration/20240311000000.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/backend/migration/20240311000000.go b/server/backend/migration/20240311000000.go index 45f5602d..0855f9a1 100644 --- a/server/backend/migration/20240311000000.go +++ b/server/backend/migration/20240311000000.go @@ -13,7 +13,7 @@ func migrate20240311000000() *gormigrate.Migration { ID: "20240311000000", Migrate: func(tx *gorm.DB) error { // make sure that dish_rating is FK-bound to dish - if err := tx.Exec("alter table dish modify dish int not null auto_increment").Error; err != nil { + if err := tx.Exec("alter table dish_rating modify dishID int").Error; err != nil { return err } if err := tx.Exec("alter table dish_rating add constraint dish_rating_dish_dish_fk foreign key (dishID) references dish (dish) on update cascade on delete cascade").Error; err != nil { @@ -31,7 +31,7 @@ func migrate20240311000000() *gormigrate.Migration { if err := tx.Exec("alter table dish_rating drop constraint dish_rating_dish_dish_fk").Error; err != nil { return err } - if err := tx.Exec("alter table dish modify dish int auto_increment").Error; err != nil { + if err := tx.Exec("alter table dish_rating modify dishID int not null").Error; err != nil { return err } // because dishes already have a cafeteria, storing this agiain is not nessesary