From 886b1de94914186a502ed68a6a281a15f1d8d9a3 Mon Sep 17 00:00:00 2001 From: zeripath Date: Thu, 10 Mar 2022 22:04:55 +0000 Subject: [PATCH] Update the webauthn_credential_id_sequence in Postgres (#19048) * Update the webauthn_credential_id_sequence in Postgres There is (yet) another problem with v210 in that Postgres will silently allow preset ID insertions ... but it will not update the sequence value. This PR simply adds a little step to the end of the v210 migration to update the sequence number. Users who have already migrated who find that they cannot insert new webauthn_credentials into the DB can either run: ```bash gitea doctor recreate-table webauthn_credential ``` or ```bash ./gitea doctor --run=check-db-consistency --fix ``` which will fix the bad sequence. Fix #19012 Signed-off-by: Andrew Thornton --- docs/content/doc/developers/guidelines-backend.md | 14 ++++++++++++-- models/migrations/v210.go | 6 ++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/docs/content/doc/developers/guidelines-backend.md b/docs/content/doc/developers/guidelines-backend.md index d24946545..1248d4143 100644 --- a/docs/content/doc/developers/guidelines-backend.md +++ b/docs/content/doc/developers/guidelines-backend.md @@ -42,7 +42,7 @@ To maintain understandable code and avoid circular dependencies it is important - `modules/setting`: Store all system configurations read from ini files and has been referenced by everywhere. But they should be used as function parameters when possible. - `modules/git`: Package to interactive with `Git` command line or Gogit package. - `public`: Compiled frontend files (javascript, images, css, etc.) -- `routers`: Handling of server requests. As it uses other Gitea packages to serve the request, other packages (models, modules or services) shall not depend on routers. +- `routers`: Handling of server requests. As it uses other Gitea packages to serve the request, other packages (models, modules or services) must not depend on routers. - `routers/api` Contains routers for `/api/v1` aims to handle RESTful API requests. - `routers/install` Could only respond when system is in INSTALL mode (INSTALL_LOCK=false). - `routers/private` will only be invoked by internal sub commands, especially `serv` and `hooks`. @@ -106,10 +106,20 @@ i.e. `servcies/user`, `models/repository`. Since there are some packages which use the same package name, it is possible that you find packages like `modules/user`, `models/user`, and `services/user`. When these packages are imported in one Go file, it's difficult to know which package we are using and if it's a variable name or an import name. So, we always recommend to use import aliases. To differ from package variables which are commonly in camelCase, just use **snake_case** for import aliases. i.e. `import user_service "code.gitea.io/gitea/services/user"` +### Important Gotchas + +- Never write `x.Update(exemplar)` without an explicit `WHERE` clause: + - This will cause all rows in the table to be updated with the non-zero values of the exemplar - including IDs. + - You should usually write `x.ID(id).Update(exemplar)`. +- If during a migration you are inserting into a table using `x.Insert(exemplar)` where the ID is preset: + - You will need to ``SET IDENTITY_INSERT `table` ON`` for the MSSQL variant (the migration will fail otherwise) + - However, you will also need to update the id sequence for postgres - the migration will silently pass here but later insertions will fail: + ``SELECT setval('table_name_id_seq', COALESCE((SELECT MAX(id)+1 FROM `table_name`), 1), false)`` + ### Future Tasks Currently, we are creating some refactors to do the following things: - Correct that codes which doesn't follow the rules. - There are too many files in `models`, so we are moving some of them into a sub package `models/xxx`. -- Some `modules` sub packages should be moved to `services` because they depends on `models`. \ No newline at end of file +- Some `modules` sub packages should be moved to `services` because they depend on `models`. diff --git a/models/migrations/v210.go b/models/migrations/v210.go index dafe355fe..f32fae77b 100644 --- a/models/migrations/v210.go +++ b/models/migrations/v210.go @@ -174,5 +174,11 @@ func remigrateU2FCredentials(x *xorm.Engine) error { regs = regs[:0] } + if x.Dialect().URI().DBType == schemas.POSTGRES { + if _, err := x.Exec("SELECT setval('webauthn_credential_id_seq', COALESCE((SELECT MAX(id)+1 FROM `webauthn_credential`), 1), false)"); err != nil { + return err + } + } + return nil }