From d285905826150506dc6809e11808d7aa92d49b3b Mon Sep 17 00:00:00 2001 From: zeripath Date: Sun, 13 Mar 2022 04:02:19 +0000 Subject: [PATCH] Update the webauthn_credential_id_sequence in Postgres (#19048) (#19060) Backport #19048 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 SELECT setval('webauthn_credential_id_seq', COALESCE((SELECT MAX(id)+1 FROM `webauthn_credential`), 1), false) ``` which will fix the bad sequence. Fix #19012 Signed-off-by: Andrew Thornton Co-authored-by: 6543 <6543@obermui.de> --- 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 d249465453..1248d41432 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 dafe355fe2..f32fae77ba 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 }