MySQL compatibility for esignet-service #2619
Replies: 2 comments 1 reply
|
@anushasunkada can we check on this package GORM, it can resolve multiple issue stated here.
Edited: I have found another package Ent. It has upsert statement support for all rdbms except oracle/ms sql. |
|
there are 19 queries total across the 3 db packages, where 10 of them are already in sqlx format and only 1 (UpsertConsent) is a genuine cross-dialect problem (Rule D). it would be a high effort for GORM/ENT's upsert statement support for just 1 upsert statement which may be solved by a switch statement also keymanager/db is already hand-written sqlx in production, so extending that pattern thats already working fine to clientmgmt/db and consentmgmt/db would be less effort then shifting all three packages to a new ORM |
Uh oh!
There was an error while loading. Please reload this page.
Context
esignet-service(the Go rewrite under/opt/mosip/github/idp/esignet-service, not the Java module) is currently hardwired to PostgreSQL end-to-end:internal/config/db.go): builds a Postgres keyword-value DSN and configures apgxpool.Pooldirectly;cmd/esignet/main.gohardcodessqlx.NewDb(pgConn, "pgx").clientmgmt/dbandconsentmgmt/dbaresqlc-generated (sqlc.yamlhardcodesengine: "postgresql") fromquery.sqlfiles using$1placeholders,ON CONFLICT ... DO UPDATE ... EXCLUDED,RETURNING *, andIS NOT DISTINCT FROM.keymanager/db/db.gois hand-written withsqlx, also using$1-style placeholders built viafmt.Sprintfwith schema-qualified table names.clientmgmt/service.go(isDuplicateClientID,isDuplicatePublicKeyHash) andkeymanager/rotation.go(IsDuplicateUniIdent) detect unique-constraint violations by type-asserting*pgconn.PgErrorand checking SQLSTATE23505+ConstraintName— a third, independently-discovered Postgres-only dependency beyond the SQL text itself.db_scripts/mosip_esignet/anddb_upgrade_script/mosip_esignet/arepsql-driven (schema/search_path,CREATE ROLE,GRANT ... ON ALL TABLES IN SCHEMA,ALTER DEFAULT PRIVILEGES).docker-compose/docker-compose.yamlonly provisionspostgres:bookworm.ddl/*.sql) is already portable — plainvarchar/timestamp/boolean, nojsonb, arrays, or Postgres-only types — and the sqlc-generated Go models use plaindatabase/sqltypes (sql.NullString,sql.NullTime), not pgx-specific ones.clientmgmt/service.go,consentmgmt/service.go,cryptomanagerviakeymanager/db) depend only on each package'sQuerierinterface anddb.New(...)/model/*Paramsstructs — never on the sqlc-generated implementation directly. This is what makes an internal rewrite safe: the public surface (interface method signatures, struct field names/types) must stay identical so calling code needs no changes.Goal: add MySQL as a second supported database without duplicating business logic, and without hard-wiring the solution to exactly two databases — the user wants a design that keeps adding a third RDBMS cheap. Chosen direction: drop sqlc for query execution, converge
clientmgmt/dbandconsentmgmt/dbonto the same hand-writtensqlxpatternkeymanager/dbalready uses, and eliminate dialect divergence at the SQL-text and error-handling level wherever possible instead of maintaining parallel implementations per database.Approach: five portability rules, two isolated exceptions
Rule A — Placeholders are free. Write every query with
?, never$N. Convert once per query string, at construction time, viasqlx.Rebind(sqlx.BindType(driverName), query)— a standalone function, so theDBTX/TXinterfaces in all three db packages stay plaindatabase/sql-shaped (no need to switchclientmgmt/consentmgmtontosqlx.DB/sqlx.Txat all). Adding a third RDBMS needs no query-text changes, only asqlx.BindTypemapping sqlx mostly already knows.Rule B — Never use
RETURNING. Replaceclientmgmt'sINSERT ... RETURNING */UPDATE ... RETURNING *(CreateClient,UpdateClient,PatchClient) with the write followed by aSELECT ... WHERE id = ?in Go. Written once, works on every RDBMS.Rule C — No dialect-specific null-safe operators. Replace
PatchClient'supd_dtimes IS NOT DISTINCT FROM sqlc.arg(expected_upd_dtimes)with a WHERE clause built conditionally in Go:upd_dtimes IS NULLwhen the expected value is nil, elseupd_dtimes = ?. Fully portable.Rule D (isolated exception #1) — the upsert.
consentmgmt'sUpsertConsent(ON CONFLICT ... DO UPDATE ... EXCLUDED) has no unified SQL syntax across RDBMS. Fix: an application-level transactional check-then-write (SELECT ... FOR UPDATEon(client_id, psu_token), thenINSERTorUPDATE) — standard across Postgres and MySQL/InnoDB, so it needs zero per-dialect SQL.Rule E (isolated exception #2) — unique-constraint error classification.
errors.Asagainst*pgconn.PgErroronly works for Postgres. Add one small shared helper (e.g.internal/dbutil/uniqueviolation.go) exposingIsUniqueViolation(err error, names ...string) boolthat type-switches on*pgconn.PgError(SQLSTATE23505, matchConstraintName) or*mysql.MySQLError(Number == 1062, match by substring against the index name inMessage, since MySQL doesn't expose a separate constraint-name field). All three existing call sites (isDuplicateClientID,isDuplicatePublicKeyHash,IsDuplicateUniIdent) switch to calling this helper instead of hand-rolling the type assertion. This is the only other place a future RDBMS needs one newcaseadded.Net effect: adding a future RDBMS touches only the connection layer (Step 1) and the two isolated helpers (Rules D and E) — no other query text or business logic changes.
Detailed implementation steps
Step 1 —
internal/config/db.go: driver-aware connection layerDB_DRIVERto the env-var list (envOrDefault("DB_DRIVER", "postgres")), validate it's"postgres"or"mysql".resolveDBDSN/loadDB/buildPoolConfig/Openinto apostgrespath (existing code, unchanged) and a newmysqlpath:resolveMySQLDSN: builduser:password@tcp(host:port)/dbname?parseTime=true&loc=UTCfromDATABASE_HOST/PORT/NAME/USERNAME/PASSWORD(reuseenvOrDefault/DB_DBUSER_PASSWORDfallback exactly as today), or passDATABASE_URLstraight through if it already looks like amysql:///DSN string for that driver.openMySQL(dsn string, pool DBPool) (*sql.DB, func() error, error):sql.Open("mysql", dsn), thenconn.SetMaxOpenConns(pool.MaxOpenConns),SetMaxIdleConns(pool.MaxIdleConns),SetConnMaxLifetime(effectiveMaxConnLifetime(...)),SetConnMaxIdleTime(...), thenPingContextwith the samedbPingTimeout. Nopgxpool-style eagerMinConns— add a doc comment notingMaxIdleConnshere is the passive database/sql ceiling, not an eager floor (unlike the Postgres path)._ "github.com/go-sql-driver/mysql"blank import to register the driver.DB.Open()'s signature to also return the resolved driver name:func (d DB) Open() (conn *sql.DB, driverName string, closeFn func() error, err error). Dispatch to the postgres or mysql path based onDB_DRIVER, defaulting to"postgres"(preserves current behavior when unset).internal/config/db_test.go: addTestResolveMySQLDSN_*cases mirroring the existingTestResolveDBDSN_*table-driven style; keep all existing Postgres tests unchanged.Step 2 —
internal/dbutil(new package): shared portability helpersCreate
esignet-service/internal/dbutil/with two files, importable by all three db packages and byclientmgmt/keymanager:rebind.go:func Rebind(driverName, query string) string { return sqlx.Rebind(sqlx.BindType(driverName), query) }— thin wrapper so call sites don't importsqlxdirectly just for this.uniqueviolation.go:func IsUniqueViolation(err error, names ...string) boolimplementing Rule E —errors.Asagainst*pgconn.PgError(code23505,ConstraintNameinnames) first, then*mysql.MySQLError(Number == 1062,Messagecontains any ofnames) as a second branch. Keep the existing string-match fallback fromrotation.go(strings.Contains(msg, "23505")) as a third, driver-agnostic-error branch for tests that construct plain errors.Step 3 —
internal/clientmgmt/db/: convert to hand-written sqlxquery.sql,query.sql.go's sqlc-generated body, and this package's entry insqlc.yaml. Keepschema.sqlas living documentation (or delete it too — no longer feeds codegen).db.go: keepDBTXinterface exactly as-is (still plaindatabase/sqlshaped —ExecContext/PrepareContext/QueryContext/QueryRowContext). ChangeNew(db DBTX) *QueriestoNew(db DBTX, driverName string) *Queries, pre-building each rebound query string once viadbutil.Rebind(mirrorskeymanager/db.New's pattern of building strings once in the constructor).models.go(ClientDetail) andquerier.go(Querierinterface) byte-for-byte identical — no caller-visible change.*Paramsstruct shapes from today'squery.sql.go:GetClient,GetActiveClient: trivial —$1→?, everything else unchanged.CreateClient:INSERT INTO client_detail (...) VALUES (?, ?, ...)(17 placeholders, noRETURNING); on success, runSELECT * FROM client_detail WHERE id = ?(reuse the same query string asGetClient) to build the returnedClientDetail(Rule B).UpdateClient:UPDATE client_detail SET ... WHERE id = ?(noRETURNING), then the same follow-upSELECT(Rule B).PatchClient: sameUPDATE ... WHERE id = ?pattern, but build the trailingAND upd_dtimes = ?/AND upd_dtimes IS NULLclause in Go fromarg.ExpectedUpdDtimes.Valid(Rule C) instead ofIS NOT DISTINCT FROM $15; checkRowsAffected()— 0 rows means either "not found" or "concurrent modification", matching the currentsql.ErrNoRows-from-RETURNINGsemantics the caller (clientmgmt/service.go) already expects forErrClientConflict; then the same follow-upSELECT(Rule B).clientmgmt/service.go: replaceisDuplicateClientID/isDuplicatePublicKeyHash's hand-rolledpgconn.PgErrorassertions with calls todbutil.IsUniqueViolation(err, "pk_clntdtl_id", "client_detail_pkey")anddbutil.IsUniqueViolation(err, "uk_clntdtl_public_key_hash")respectively (Rule E); drop the now-unusedpgconnimport.NewService(conn *sql.DB, ...)→ thread adriverName stringparameter through todb.New(conn, driverName).Step 4 —
internal/consentmgmt/db/: convert to hand-written sqlxquery.sql/generatedquery.sql.gobody and this package'ssqlc.yamlentry; keep or dropschema.sqlas documentation.db.gothe same way as Step 3.2 (New(db DBTX, driverName string) *Queries, pre-rebound query strings).models.go(ConsentDetail,ConsentHistory) andquerier.go(Querier) identical.*Paramsstruct shapes:GetConsent,InsertConsentHistory,DeleteConsent: trivial$N→?swap.UpsertConsent(Rule D): replace the singleON CONFLICT ... EXCLUDEDstatement with:txBeginneris a small local interface (BeginTx(ctx, *sql.TxOptions) (*sql.Tx, error)) that only*sql.DBsatisfies (not*sql.Tx), andupsertConsentTxrunsSELECT id FROM consent_detail WHERE client_id = ? AND psu_token = ? FOR UPDATE, then either the existingINSERT INTO consent_detail (...)(no row found) or anUPDATE consent_detail SET claims=?, ... WHERE client_id = ? AND psu_token = ?(row found) usingarg's fields.consentmgmt.Service.SaveRecordalready opens a*sql.Txand doesq = db.New(tx)to wrapInsertConsentHistory+UpsertConsentatomically — but only whens.db != nil, andNewService(conn *sql.DB)(the only constructormain.goactually calls) never setss.db, so today that branch is dead and both statements run as separate, non-transactional writes in production. This pre-existing gap is orthogonal to MySQL support, but thetxBeginnerdesign above happens to makeUpsertConsentsafe either way; flag it to the user as a separate finding rather than silently changingSaveRecord's behavior as a side effect of this work.NewService(conn *sql.DB)→ threaddriverName stringthrough todb.New(conn, driverName).Step 5 —
internal/keymanager/db/db.go: placeholder portability$1/$2/... literal in thefmt.Sprintf(...)-built query strings to?.New(conn TX, schema string) *QueriestoNew(conn TX, schema string, driverName string) *Queries; after building each%s.key_alias/%s.key_policy_def/%s.key_store-qualified string, pass it throughdbutil.Rebind(driverName, ...)before storing it onq.rotation.go: replaceIsDuplicateUniIdent'spgconn.PgErrorassertion withdbutil.IsUniqueViolation(err, "uni_ident_const")(Rule E), keeping the existing string-match fallback folded into the shared helper (see Step 2). Drop the now-unusedpgconnimport if nothing else in the file needs it.New) that theschemaparameter must be a MySQL database name whendriverName == "mysql"(MySQL has no schema-within-database concept;CREATE SCHEMAis a synonym forCREATE DATABASEthere), and that the connecting user needs privileges on that database.Step 6 —
cmd/esignet/main.go: wire the driver name throughpgConn, closeDB, err := appCfg.DB.Open()→conn, driverName, closeDB, err := appCfg.DB.Open().sqlxConn := sqlx.NewDb(pgConn, "pgx")→sqlxConn := sqlx.NewDb(conn, driverName)(only correct whendriverNameis a name sqlx recognizes for bind-type purposes — confirm"mysql"and"postgres"/"pgx"both resolve correctly viasqlx.BindType, registering a custom bind type viasqlx.BindDriverif needed for"pgx"specifically, mirroring whatever mapping already makes today's"pgx"call work).clientmgmt.NewService(conn, ...),consentmgmt.NewService(conn), anddb.New(conn, kmCfg.DBSchema)(keymanager) call sites to also passdriverName.Step 7 —
esignet-service/go.modAdd
github.com/go-sql-driver/mysqlas a direct dependency (go get github.com/go-sql-driver/mysql@<version already pinned in go.sum>thengo mod tidy); remove thegithub.com/mosip/esignet/... sqlctoolchain dependency ifsqlc.yaml/codegen is dropped entirely (check for a//go:generate sqlc generatedirective or Makefile target referencing it first).Step 8 — Provisioning scripts (Out of Scope)
db_scripts/mysql/mosip_esignet/mirroring the Postgres tree file-by-file:db.sql:CREATE DATABASE mosip_esignet CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;— noCREATE SCHEMA/ALTER DATABASE ... SET search_path(the database itself is the equivalent unit).role_dbuser.sql:CREATE USER 'dbuser'@'%' IDENTIFIED BY '...';in place ofCREATE ROLE ... WITH INHERIT LOGIN PASSWORD.grants.sql:GRANT ALL PRIVILEGES ON mosip_esignet.* TO 'dbuser'@'%';— noALTER DEFAULT PRIVILEGESequivalent; note in a comment that this must be re-run after adding new tables if using least-privilege grants.ddl/*.sql,dml/*.sql: copy the existing files; spot-check every column/table name against MySQL's reserved-word list (none of the current ones —key,status, etc. as table names would collide, but check column names too) and adjustvarchar/timestamplengths only if MySQL's row-size limits are exceeded (unlikely given current sizes).deploy.sh/deploy.properties,drop_db.sql/drop_role.sql: mirror using themysqlCLI instead ofpsql(mysql -u ... -p... < file.sql,-efor inline commands, no\c/psql-variable substitution — use shell variable substitution orenvsubstinstead).db_upgrade_script/mysql/mosip_esignet/sql/mirroring each versioned*_upgrade.sql/*_rollback.sqlpair (1.0.0 → 2.0.0) andupgrade.sh/upgrade.properties, translated the same way.Step 9 —
docker-compose/docker-compose.yamlAdd a
mysql:8service under a compose profile (e.g.mysql), withMYSQL_DATABASE/MYSQL_USER/MYSQL_PASSWORD/MYSQL_ROOT_PASSWORDenv vars and amysqladmin pinghealthcheck (mirroring the existingpostgresservice'spg_isreadyhealthcheck). Leave theesignetservice's env vars as-is except documenting that settingDB_DRIVER=mysql+ pointingDATABASE_*at this service switches it over; don't change the default profile (Postgres stays the default local-dev path).Step 10 — Tests
internal/config/db_test.go: add the MySQL DSN/pool-builder unit tests (Step 1.4).internal/dbutil/uniqueviolation_test.go: table-driven tests covering*pgconn.PgError,*mysql.MySQLError, and the plain-string fallback.clientmgmt/consentmgmt/keymanagerunit tests that useNewServiceWithQuerier/hand-written fakes need no changes (they target the unchangedQuerierinterfaces) — just confirmgo test ./...still passes after the rewrite.clientmgmt/consentmgmt/keymanagerdbpackage tests against both a real Postgres and a real MySQL instance (e.g. viadocker-compose --profile mysql up -d+ the new deploy script, ortestcontainers-goif introducing a test dependency is acceptable) to catch any query that type-checks but behaves differently per dialect.Critical files
esignet-service/internal/config/db.go,db_test.goesignet-service/internal/dbutil/rebind.go,uniqueviolation.go(new package)esignet-service/cmd/esignet/main.goesignet-service/internal/clientmgmt/db/{db,models,querier}.go(rewritten),service.go(error-classification + driver-name wiring)esignet-service/internal/consentmgmt/db/{db,models,querier}.go(rewritten),service.go(driver-name wiring)esignet-service/internal/keymanager/db/db.go,rotation.goesignet-service/sqlc.yaml— trim or removeesignet-service/go.moddb_scripts/mosip_esignet/**→ newdb_scripts/mysql/mosip_esignet/**sibling treedb_upgrade_script/mosip_esignet/**→ new sibling tree per versiondocker-compose/docker-compose.yamlVerification
go build ./... && go test ./...inesignet-service— should pass unchanged for callers (they depend onQuerierinterfaces, not implementations).docker-compose --profile mysql up -d mysql; run the new MySQL deploy script against it; start the service withDB_DRIVER=mysqland the MySQLDATABASE_*values; smoke-test: create an OIDC client (exercises Rule B + Rule E via a deliberate duplicate-ID attempt), patch a client (Rule C, optimistic concurrency), save/overwrite a consent record twice (Rule D upsert path), key generation and lookup via keymanager.DB_DRIVER=postgres, the default) to confirm no regression.go vet/gofmtclean; confirm removing/trimmingsqlc.yamldoesn't break any remaininggo generate/Makefile step.All reactions