Use INT-based foreign key references instead of UUID-based references across DB schema#1282
Use INT-based foreign key references instead of UUID-based references across DB schema#1282KaveeshaPiumini wants to merge 1 commit intoasgardeo:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the runtime DB schema and flow execution persistence logic to use integer primary keys for FLOW_CONTEXT and FLOW_USER_DATA, and switches the relationship between them to reference FLOW_CONTEXT.ID instead of the previous (FLOW_ID, DEPLOYMENT_ID) composite key.
Changes:
- Added
IDidentity/autoincrement primary keys toFLOW_CONTEXTandFLOW_USER_DATA(Postgres + SQLite) and replaced the old composite PKs withUNIQUE (FLOW_ID, DEPLOYMENT_ID). - Updated
FLOW_USER_DATAto referenceFLOW_CONTEXTvia a newFLOW_CONTEXT_IDFK, and updated SQL queries to join/insert using this key. - Extended the Go DB model and row parsing to handle the new
FLOW_CONTEXT.IDvalue.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/dbscripts/runtimedb/postgres.sql | Adds integer PKs + new FK from FLOW_USER_DATA to FLOW_CONTEXT(ID) in Postgres schema. |
| backend/dbscripts/runtimedb/sqlite.sql | Same schema refactor as Postgres for SQLite. |
| backend/internal/flow/flowexec/queries.go | Refactors insert/join queries to use FLOW_CONTEXT_ID and selects fc.ID. |
| backend/internal/flow/flowexec/model.go | Adds FlowContextID field to the combined DB model. |
| backend/internal/flow/flowexec/store.go | Parses the new id column and populates FlowContextID; adds an int parsing helper. |
| CREATE TABLE FLOW_USER_DATA ( | ||
| ID INT GENERATED ALWAYS AS IDENTITY PRIMARY KEY, | ||
| FLOW_ID VARCHAR(36) NOT NULL, | ||
| DEPLOYMENT_ID VARCHAR(255) NOT NULL, | ||
| FLOW_CONTEXT_ID INT NOT NULL, | ||
| IS_AUTHENTICATED BOOLEAN NOT NULL DEFAULT FALSE, |
There was a problem hiding this comment.
Consider adding an index on FLOW_USER_DATA(FLOW_CONTEXT_ID, DEPLOYMENT_ID) (or at least FLOW_CONTEXT_ID). Queries now join FLOW_CONTEXT.ID -> FLOW_USER_DATA.FLOW_CONTEXT_ID, but the schema only creates an index on DEPLOYMENT_ID, which can cause full scans of FLOW_USER_DATA for a deployment during joins as data grows.
| UPDATED_AT TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, | ||
| PRIMARY KEY (FLOW_ID, DEPLOYMENT_ID), | ||
| FOREIGN KEY (FLOW_ID, DEPLOYMENT_ID) REFERENCES FLOW_CONTEXT(FLOW_ID, DEPLOYMENT_ID) ON DELETE CASCADE | ||
| UNIQUE (FLOW_ID, DEPLOYMENT_ID), | ||
| FOREIGN KEY (FLOW_CONTEXT_ID) REFERENCES FLOW_CONTEXT(ID) ON DELETE CASCADE |
There was a problem hiding this comment.
The FK on FLOW_USER_DATA only references FLOW_CONTEXT(ID) and does not constrain DEPLOYMENT_ID to match between the two tables. That means the DB cannot enforce deployment isolation for this relationship, and a mismatched row could be cascaded/deleted across deployments. Consider using a composite FK (FLOW_CONTEXT_ID, DEPLOYMENT_ID) referencing FLOW_CONTEXT(ID, DEPLOYMENT_ID) (with a supporting UNIQUE on FLOW_CONTEXT(ID, DEPLOYMENT_ID)) to enforce isolation at the schema level.
| CREATE TABLE FLOW_USER_DATA ( | ||
| ID INTEGER PRIMARY KEY AUTOINCREMENT, | ||
| FLOW_ID VARCHAR(36) NOT NULL, | ||
| DEPLOYMENT_ID VARCHAR(255) NOT NULL, | ||
| FLOW_CONTEXT_ID INTEGER NOT NULL, | ||
| IS_AUTHENTICATED BOOLEAN NOT NULL DEFAULT 0, |
There was a problem hiding this comment.
Consider adding an index on FLOW_USER_DATA(FLOW_CONTEXT_ID, DEPLOYMENT_ID) (or at least FLOW_CONTEXT_ID). Queries now join FLOW_CONTEXT.ID -> FLOW_USER_DATA.FLOW_CONTEXT_ID, but the schema only creates an index on DEPLOYMENT_ID, which can cause full scans of FLOW_USER_DATA for a deployment during joins as data grows.
| UPDATED_AT TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, | ||
| PRIMARY KEY (FLOW_ID, DEPLOYMENT_ID), | ||
| FOREIGN KEY (FLOW_ID, DEPLOYMENT_ID) REFERENCES FLOW_CONTEXT(FLOW_ID, DEPLOYMENT_ID) ON DELETE CASCADE | ||
| UNIQUE (FLOW_ID, DEPLOYMENT_ID), | ||
| FOREIGN KEY (FLOW_CONTEXT_ID) REFERENCES FLOW_CONTEXT(ID) ON DELETE CASCADE |
There was a problem hiding this comment.
The FK on FLOW_USER_DATA only references FLOW_CONTEXT(ID) and does not constrain DEPLOYMENT_ID to match between the two tables. That means the DB cannot enforce deployment isolation for this relationship, and a mismatched row could be cascaded/deleted across deployments. Consider using a composite FK (FLOW_CONTEXT_ID, DEPLOYMENT_ID) referencing FLOW_CONTEXT(ID, DEPLOYMENT_ID) (with a supporting UNIQUE on FLOW_CONTEXT(ID, DEPLOYMENT_ID)) to enforce isolation at the schema level.
| func (s *flowStore) parseInt64(value interface{}) (int64, error) { | ||
| if value == nil { | ||
| return 0, errors.New("value is nil") | ||
| } |
There was a problem hiding this comment.
parseInt64 returns very generic errors ("value is nil" / "failed to parse value as int64"), which makes debugging result-set issues harder. Consider including the column name and the actual encountered type/value in the error so callers can quickly identify which field failed to parse.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1282 +/- ##
==========================================
+ Coverage 89.68% 89.69% +0.01%
==========================================
Files 642 642
Lines 42417 42426 +9
Branches 2425 2425
==========================================
+ Hits 38042 38056 +14
Misses 2377 2377
+ Partials 1998 1993 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
KaveeshaPiumini
force-pushed
the
mosip
branch
from
4663180 to
aa6e6cc
Compare
KaveeshaPiumini
force-pushed
the
mosip
branch
from
aa6e6cc to
7715f5b
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSurrogate identity primary keys (ID) were added to FLOW_CONTEXT and FLOW_USER_DATA in Postgres and SQLite schemas. Composite primary keys were converted to UNIQUE constraints. FLOW_USER_DATA now stores FLOW_CONTEXT_ID FK. Go model, queries, and store code were updated to emit and consume the new ID-based relationships. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Service
participant DB
Client->>Service: Create Flow Context (flow_id, deployment_id, ...)
Service->>DB: INSERT INTO FLOW_CONTEXT (...) RETURNING ID
DB-->>Service: new FLOW_CONTEXT.ID
Client->>Service: Create Flow User Data (flow_id, ..., deployment_id)
Service->>DB: INSERT INTO FLOW_USER_DATA (FLOW_CONTEXT_ID, ...) VALUES ((SELECT ID FROM FLOW_CONTEXT WHERE FLOW_ID=$1 AND DEPLOYMENT_ID=$N), ...)
DB-->>Service: INSERT OK
Client->>Service: Get Flow Context with User Data (flow_id, deployment_id)
Service->>DB: SELECT fc.ID, fc..., fud... FROM FLOW_CONTEXT fc LEFT JOIN FLOW_USER_DATA fud ON fc.ID = fud.FLOW_CONTEXT_ID WHERE fc.FLOW_ID=$1 AND fc.DEPLOYMENT_ID=$2
DB-->>Service: result rows
Service-->>Client: composed response (includes FlowContextID)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
KaveeshaPiumini
force-pushed
the
mosip
branch
from
7715f5b to
78340c3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/dbscripts/runtimedb/sqlite.sql (1)
56-70:⚠️ Potential issue | 🟠 MajorEnforce deployment consistency in FLOW_USER_DATA’s FK.
FLOW_CONTEXT_ID alone doesn’t guarantee the same DEPLOYMENT_ID, so a wrong ID could cross-link tenants. Consider a composite FK.
🔧 Suggested schema tweak
CREATE TABLE FLOW_CONTEXT ( ID INTEGER PRIMARY KEY AUTOINCREMENT, FLOW_ID VARCHAR(36) NOT NULL, DEPLOYMENT_ID VARCHAR(255) NOT NULL, APP_ID VARCHAR(36) NOT NULL, VERBOSE BOOLEAN NOT NULL DEFAULT 0, CURRENT_NODE_ID VARCHAR(50), CURRENT_ACTION VARCHAR(50), GRAPH_ID VARCHAR(50) NOT NULL, RUNTIME_DATA TEXT, EXECUTION_HISTORY TEXT, CREATED_AT TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, UPDATED_AT TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, - UNIQUE (FLOW_ID, DEPLOYMENT_ID) + UNIQUE (FLOW_ID, DEPLOYMENT_ID), + UNIQUE (ID, DEPLOYMENT_ID) ); CREATE TABLE FLOW_USER_DATA ( ID INTEGER PRIMARY KEY AUTOINCREMENT, FLOW_ID VARCHAR(36) NOT NULL, DEPLOYMENT_ID VARCHAR(255) NOT NULL, FLOW_CONTEXT_ID INTEGER NOT NULL, IS_AUTHENTICATED BOOLEAN NOT NULL DEFAULT 0, USER_ID VARCHAR(36), OU_ID VARCHAR(36), USER_TYPE VARCHAR(50), USER_INPUTS TEXT, USER_ATTRIBUTES TEXT, CREATED_AT TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, UPDATED_AT TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, UNIQUE (FLOW_ID, DEPLOYMENT_ID), - FOREIGN KEY (FLOW_CONTEXT_ID) REFERENCES FLOW_CONTEXT(ID) ON DELETE CASCADE + FOREIGN KEY (FLOW_CONTEXT_ID, DEPLOYMENT_ID) REFERENCES FLOW_CONTEXT(ID, DEPLOYMENT_ID) ON DELETE CASCADE );backend/dbscripts/runtimedb/postgres.sql (1)
56-70:⚠️ Potential issue | 🟠 MajorEnforce deployment consistency in FLOW_USER_DATA’s FK.
FLOW_CONTEXT_ID alone doesn’t guarantee the same DEPLOYMENT_ID, so a wrong ID could cross-link tenants. Consider a composite FK.
🔧 Suggested schema tweak
CREATE TABLE FLOW_CONTEXT ( ID INT GENERATED ALWAYS AS IDENTITY PRIMARY KEY, FLOW_ID VARCHAR(36) NOT NULL, DEPLOYMENT_ID VARCHAR(255) NOT NULL, APP_ID VARCHAR(36) NOT NULL, "VERBOSE" BOOLEAN NOT NULL DEFAULT FALSE, CURRENT_NODE_ID VARCHAR(50), CURRENT_ACTION VARCHAR(50), GRAPH_ID VARCHAR(50) NOT NULL, RUNTIME_DATA JSONB, EXECUTION_HISTORY JSONB, CREATED_AT TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, UPDATED_AT TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, - UNIQUE (FLOW_ID, DEPLOYMENT_ID) + UNIQUE (FLOW_ID, DEPLOYMENT_ID), + UNIQUE (ID, DEPLOYMENT_ID) ); CREATE TABLE FLOW_USER_DATA ( ID INT GENERATED ALWAYS AS IDENTITY PRIMARY KEY, FLOW_ID VARCHAR(36) NOT NULL, DEPLOYMENT_ID VARCHAR(255) NOT NULL, FLOW_CONTEXT_ID INT NOT NULL, IS_AUTHENTICATED BOOLEAN NOT NULL DEFAULT FALSE, USER_ID VARCHAR(36), OU_ID VARCHAR(36), USER_TYPE VARCHAR(50), USER_INPUTS JSONB, USER_ATTRIBUTES JSONB, CREATED_AT TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, UPDATED_AT TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, UNIQUE (FLOW_ID, DEPLOYMENT_ID), - FOREIGN KEY (FLOW_CONTEXT_ID) REFERENCES FLOW_CONTEXT(ID) ON DELETE CASCADE + FOREIGN KEY (FLOW_CONTEXT_ID, DEPLOYMENT_ID) REFERENCES FLOW_CONTEXT(ID, DEPLOYMENT_ID) ON DELETE CASCADE );
🤖 Fix all issues with AI agents
In `@backend/dbscripts/userdb/postgres.sql`:
- Around line 55-61: The GROUP_MEMBER_REFERENCE schema allows cross-deployment
matches because the FOREIGN KEY only references GROUP_REF_ID; add a
DEPLOYMENT_ID column (same type as the GROUP table's DEPLOYMENT_ID) to the table
(NOT NULL), include DEPLOYMENT_ID in the composite foreign key (FOREIGN KEY
(GROUP_REF_ID, DEPLOYMENT_ID) REFERENCES "GROUP" (ID, DEPLOYMENT_ID) ON DELETE
CASCADE), and ensure matching types and an index/constraint exist on the
referenced "GROUP"(ID, DEPLOYMENT_ID) pair so the composite FK enforces
deployment alignment; update any INSERTs/DDL that populate this table to provide
DEPLOYMENT_ID.
In `@backend/dbscripts/userdb/sqlite.sql`:
- Around line 55-61: Add a DEPLOYMENT_ID column (NOT NULL) to the
GROUP_MEMBER_REFERENCE row definition and change the foreign key so it validates
both GROUP_REF_ID and DEPLOYMENT_ID against the "GROUP" table (i.e., make the FK
reference ("ID","DEPLOYMENT_ID") on "GROUP" instead of only GROUP_REF_ID) to
prevent cross‑deployment links; update any indexes/constraints (and
insert/insert statements) that assume the old FK shape to include DEPLOYMENT_ID
accordingly.
In `@backend/internal/flow/flowexec/store.go`:
- Around line 264-279: The parseInt64 method on flowStore must be expanded to
handle the range of numeric types database drivers may return (not just
int64/int); update parseInt64 to use a type switch similar to parseBool to
accept int32, int16, uint64/uint32/uint16, float64 (cast via int64), []byte and
string (parse with strconv.ParseInt) and any sql.RawBytes-equivalent, converting
each to int64 and returning an error on overflow or parse failure; reference the
parseInt64 function name and flowStore receiver when locating and replacing the
implementation.
| GROUP_REF_ID INT NOT NULL, | ||
| GROUP_ID VARCHAR(36) NOT NULL, | ||
| MEMBER_TYPE VARCHAR(7) NOT NULL, | ||
| MEMBER_ID VARCHAR(36) NOT NULL, | ||
| CREATED_AT TIMESTAMPTZ DEFAULT NOW(), | ||
| UPDATED_AT TIMESTAMPTZ DEFAULT NOW(), | ||
| FOREIGN KEY (GROUP_ID, DEPLOYMENT_ID) REFERENCES "GROUP" (GROUP_ID, DEPLOYMENT_ID) ON DELETE CASCADE | ||
| FOREIGN KEY (GROUP_REF_ID) REFERENCES "GROUP" (ID) ON DELETE CASCADE |
There was a problem hiding this comment.
Add DEPLOYMENT_ID to the GROUP_MEMBER_REFERENCE FK to prevent cross‑deployment links.
The FK now only validates GROUP_REF_ID, so a wrong ID from another deployment would still satisfy referential integrity. Consider enforcing deployment alignment at the DB level.
🔧 Suggested schema tweak
CREATE TABLE "GROUP" (
ID INT GENERATED ALWAYS AS IDENTITY PRIMARY KEY,
DEPLOYMENT_ID VARCHAR(255) NOT NULL,
GROUP_ID VARCHAR(36) NOT NULL,
OU_ID VARCHAR(36) NOT NULL,
NAME VARCHAR(50) NOT NULL,
DESCRIPTION VARCHAR(255),
CREATED_AT TIMESTAMPTZ DEFAULT NOW(),
UPDATED_AT TIMESTAMPTZ DEFAULT NOW(),
- UNIQUE (GROUP_ID, DEPLOYMENT_ID)
+ UNIQUE (GROUP_ID, DEPLOYMENT_ID),
+ UNIQUE (ID, DEPLOYMENT_ID)
);
CREATE TABLE GROUP_MEMBER_REFERENCE (
ID INT GENERATED ALWAYS AS IDENTITY PRIMARY KEY,
DEPLOYMENT_ID VARCHAR(255) NOT NULL,
GROUP_REF_ID INT NOT NULL,
GROUP_ID VARCHAR(36) NOT NULL,
MEMBER_TYPE VARCHAR(7) NOT NULL,
MEMBER_ID VARCHAR(36) NOT NULL,
CREATED_AT TIMESTAMPTZ DEFAULT NOW(),
UPDATED_AT TIMESTAMPTZ DEFAULT NOW(),
- FOREIGN KEY (GROUP_REF_ID) REFERENCES "GROUP" (ID) ON DELETE CASCADE
+ FOREIGN KEY (GROUP_REF_ID, DEPLOYMENT_ID) REFERENCES "GROUP" (ID, DEPLOYMENT_ID) ON DELETE CASCADE
);🤖 Prompt for AI Agents
In `@backend/dbscripts/userdb/postgres.sql` around lines 55 - 61, The
GROUP_MEMBER_REFERENCE schema allows cross-deployment matches because the
FOREIGN KEY only references GROUP_REF_ID; add a DEPLOYMENT_ID column (same type
as the GROUP table's DEPLOYMENT_ID) to the table (NOT NULL), include
DEPLOYMENT_ID in the composite foreign key (FOREIGN KEY (GROUP_REF_ID,
DEPLOYMENT_ID) REFERENCES "GROUP" (ID, DEPLOYMENT_ID) ON DELETE CASCADE), and
ensure matching types and an index/constraint exist on the referenced
"GROUP"(ID, DEPLOYMENT_ID) pair so the composite FK enforces deployment
alignment; update any INSERTs/DDL that populate this table to provide
DEPLOYMENT_ID.
backend/dbscripts/userdb/sqlite.sql
Outdated
| GROUP_REF_ID INTEGER NOT NULL, | ||
| GROUP_ID VARCHAR(36) NOT NULL, | ||
| MEMBER_TYPE VARCHAR(7) NOT NULL, | ||
| MEMBER_ID VARCHAR(36) NOT NULL, | ||
| CREATED_AT TEXT DEFAULT (datetime('now')), | ||
| UPDATED_AT TEXT DEFAULT (datetime('now')), | ||
| FOREIGN KEY (GROUP_ID, DEPLOYMENT_ID) REFERENCES "GROUP" (GROUP_ID, DEPLOYMENT_ID) ON DELETE CASCADE | ||
| FOREIGN KEY (GROUP_REF_ID) REFERENCES "GROUP" (ID) ON DELETE CASCADE |
There was a problem hiding this comment.
Add DEPLOYMENT_ID to the GROUP_MEMBER_REFERENCE FK to prevent cross‑deployment links.
The FK now only validates GROUP_REF_ID, so a wrong ID from another deployment would still satisfy referential integrity. Consider enforcing deployment alignment at the DB level.
🔧 Suggested schema tweak
CREATE TABLE "GROUP" (
ID INTEGER PRIMARY KEY AUTOINCREMENT,
DEPLOYMENT_ID VARCHAR(255) NOT NULL,
GROUP_ID VARCHAR(36) NOT NULL,
OU_ID VARCHAR(36) NOT NULL,
NAME VARCHAR(50) NOT NULL,
DESCRIPTION VARCHAR(255),
CREATED_AT TEXT DEFAULT (datetime('now')),
UPDATED_AT TEXT DEFAULT (datetime('now')),
- UNIQUE (GROUP_ID, DEPLOYMENT_ID)
+ UNIQUE (GROUP_ID, DEPLOYMENT_ID),
+ UNIQUE (ID, DEPLOYMENT_ID)
);
CREATE TABLE GROUP_MEMBER_REFERENCE (
ID INTEGER PRIMARY KEY AUTOINCREMENT,
DEPLOYMENT_ID VARCHAR(255) NOT NULL,
GROUP_REF_ID INTEGER NOT NULL,
GROUP_ID VARCHAR(36) NOT NULL,
MEMBER_TYPE VARCHAR(7) NOT NULL,
MEMBER_ID VARCHAR(36) NOT NULL,
CREATED_AT TEXT DEFAULT (datetime('now')),
UPDATED_AT TEXT DEFAULT (datetime('now')),
- FOREIGN KEY (GROUP_REF_ID) REFERENCES "GROUP" (ID) ON DELETE CASCADE
+ FOREIGN KEY (GROUP_REF_ID, DEPLOYMENT_ID) REFERENCES "GROUP" (ID, DEPLOYMENT_ID) ON DELETE CASCADE
);🤖 Prompt for AI Agents
In `@backend/dbscripts/userdb/sqlite.sql` around lines 55 - 61, Add a
DEPLOYMENT_ID column (NOT NULL) to the GROUP_MEMBER_REFERENCE row definition and
change the foreign key so it validates both GROUP_REF_ID and DEPLOYMENT_ID
against the "GROUP" table (i.e., make the FK reference ("ID","DEPLOYMENT_ID") on
"GROUP" instead of only GROUP_REF_ID) to prevent cross‑deployment links; update
any indexes/constraints (and insert/insert statements) that assume the old FK
shape to include DEPLOYMENT_ID accordingly.
| // parseInt64 safely parses an int64 field from the database row with type conversion support | ||
| func (s *flowStore) parseInt64(value interface{}) (int64, error) { | ||
| if value == nil { | ||
| return 0, errors.New("value is nil") | ||
| } | ||
|
|
||
| if intVal, ok := value.(int64); ok { | ||
| return intVal, nil | ||
| } | ||
|
|
||
| if intVal, ok := value.(int); ok { | ||
| return int64(intVal), nil | ||
| } | ||
|
|
||
| return 0, errors.New("failed to parse value as int64") | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate the parseInt64 function and understand its usage
rg -n "parseInt64" backend/internal/flow/flowexec/store.go -B2 -A10Repository: asgardeo/thunder
Length of output: 885
🏁 Script executed:
# Find GetFlowContext to see how parseInt64 is used
rg -n "GetFlowContext" backend/internal/flow/flowexec/ -B2 -A5Repository: asgardeo/thunder
Length of output: 8056
🏁 Script executed:
# Inspect the DBClient implementation to understand row-mapping
cat -n backend/internal/system/database/client.go | head -100Repository: asgardeo/thunder
Length of output: 134
🏁 Script executed:
# Check the database provider to see what types are returned
rg -n "Query\|Scan" backend/internal/system/database/provider/ -B2 -A3 | head -50Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Look for similar type conversion patterns in the codebase
rg -n "interface\{\}" backend/internal/ -A5 | grep -E "int64|int32|string|byte" | head -30Repository: asgardeo/thunder
Length of output: 5048
🏁 Script executed:
# Find the database client implementation
find backend/internal/system/database -type f -name "*.go" | head -20Repository: asgardeo/thunder
Length of output: 856
🏁 Script executed:
# Look for the Query method implementation
rg -n "func.*Query" backend/internal/system/database -A10Repository: asgardeo/thunder
Length of output: 47385
🏁 Script executed:
# Check what database providers are available
ls -la backend/internal/system/database/Repository: asgardeo/thunder
Length of output: 399
🏁 Script executed:
# Get the full QueryContext implementation
sed -n '72,132p' backend/internal/system/database/provider/dbclient.goRepository: asgardeo/thunder
Length of output: 1582
🏁 Script executed:
# Look for the row scanning logic - how map[string]interface{} is populated
rg -n "Scan\|scanRows\|mapRow" backend/internal/system/database -A5Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Check how other stores handle database type conversions
rg -n "interface\{\}" backend/internal/ -B3 -A5 | grep -E "\.(int|string|byte)" | head -20Repository: asgardeo/thunder
Length of output: 2710
🏁 Script executed:
# Check what database drivers are configured
rg -n "pq\|sqlite\|mysql" backend/internal/system/database -B2 -A2Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Look at the flow context table schema or how it's initialized
rg -n "CREATE TABLE\|FLOW_CONTEXT" backend/internal/flow -B2 -A5Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Check imports in dbclient and related files to see what drivers are used
head -30 backend/internal/system/database/provider/dbclient.goRepository: asgardeo/thunder
Length of output: 1070
🏁 Script executed:
# Check if there are any type conversion utilities already in the codebase
rg -n "ParseInt\|strconv.Parse" backend/internal/ -B2 -A2 | head -40Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Look for how other stores parse numeric values from database rows
rg -n "ok := value\.\(int" backend/internal/ -B3 -A3Repository: asgardeo/thunder
Length of output: 1701
🏁 Script executed:
# Check the actual database initialization to see which drivers are supported
find backend -name "*.go" -type f -exec grep -l "pq.Open\|sqlite.Open\|mysql.Open" {} \;Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Search for documentation or tests about driver behavior
rg -n "driver.Value\|database/sql" backend/internal/system/database -B2 -A3 | head -50Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Check what other type conversion patterns exist in the codebase
rg -n "int32\|int64.*interface" backend/internal/ -B3 -A3Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Look for database driver imports in the codebase
rg -n "github.com/lib/pq\|github.com/mattn/go-sqlite3" backend/ -B2 -A2Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Check the go.mod to see what database drivers are dependencies
grep -E "pq|sqlite|mysql" backend/go.modRepository: asgardeo/thunder
Length of output: 114
🏁 Script executed:
# Check the database provider initialization to see which drivers are configured
rg -n "NewDB\|dbType" backend/internal/system/database -B3 -A5 | head -60Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Look at actual test data to see what types are returned
cat -n backend/internal/system/database/provider/dbclient_test.go | sed -n '67,85p'Repository: asgardeo/thunder
Length of output: 744
🏁 Script executed:
# Check lib/pq documentation behavior in tests
rg -n "int32\|int\|int64" backend/internal/system/database/provider/dbclient_test.go -B2 -A2Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Look at how the flow context query is defined and what columns it selects
rg -n "QueryGetFlowContextWithUserData\|SELECT.*fc.ID" backend/internal/flow/flowexec/ -A15 | head -40Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Check if there are any test cases that show actual driver behavior
rg -n "AddRow\|MockDBClient" backend/internal/flow/flowexec/ -B2 -A3Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Check the database schema or migrations to see the column types
find backend -name "*migration*" -o -name "*schema*" | head -10Repository: asgardeo/thunder
Length of output: 166
🏁 Script executed:
# Look for database initialization or schema definitions
rg -n "CREATE TABLE.*FLOW" backend/ -B2 -A10Repository: asgardeo/thunder
Length of output: 8114
🏁 Script executed:
# Check if there are any actual test failures or issues reported
rg -n "parseInt64" backend/internal/flow/flowexec/store_test.go -B5 -A10Repository: asgardeo/thunder
Length of output: 144
🏁 Script executed:
# Look at the actual Go sql package documentation behavior by checking how other stores handle this
rg -n "Scan\|parseInt\|parse.*int" backend/internal/ -B3 -A3 | grep -v test | head -50Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Look for any test failures or issues with parseInt64 or type conversions
rg -n "test.*parseInt64\|TestParse.*Int" backend/internal/flow/ -B2 -A5Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Check if there are integration tests that might reveal driver behavior
find backend -name "*integration*test*.go" -o -name "*e2e*test*.go" | head -5Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Look at the modernc.org/sqlite driver documentation in go.mod
cat -n backend/go.mod | grep -A5 -B5 "sqlite"Repository: asgardeo/thunder
Length of output: 470
🏁 Script executed:
# Search for any existing issues or comments about type handling in database queries
rg -n "type.*int\|driver.*Value" backend/internal/system/database/provider/ -B2 -A3Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Check what actual database drivers return based on Go database/sql documentation
# Look at any comments or documentation in the code about type expectations
rg -n "int64\|int32\|byte.*value" backend/internal/flow/flowexec/store.go -B2 -A2Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Check if there are any runtime test assertions about types
rg -n "reflect.TypeOf\|%T\|fmt.Printf" backend/internal/system/database/ -B2 -A3Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Verify what actual types are returned by checking how similar functions handle this
rg -n "func.*parse.*interface" backend/internal/ -B2 -A8 | head -60Repository: asgardeo/thunder
Length of output: 4332
🏁 Script executed:
# Check the Go database/sql documentation and verify what types lib/pq and modernc.org/sqlite return
# by looking at actual test assertions
rg -n "int64\|int32" backend/internal/system/database/provider/dbclient_test.go -B3 -A3Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Look at the full flow context query to understand what data types are expected
sed -n '70,95p' backend/internal/flow/flowexec/queries.goRepository: asgardeo/thunder
Length of output: 726
🏁 Script executed:
# Check if there are any error cases or logs that might indicate type conversion issues
rg -n "failed to parse\|type assertion" backend/internal/flow/flowexec/ -B2 -A2Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Verify the behavior of parseBool to understand how the codebase handles multiple types
sed -n '287,310p' backend/internal/userschema/store.goRepository: asgardeo/thunder
Length of output: 593
🏁 Script executed:
# Verify the modernc.org/sqlite driver behavior by checking for any related issues or workarounds
rg -n "modernc\|sqlite" backend/ -B2 -A2 | grep -i "int\|type\|cast" | head -20Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Check if there are any other similar conversion functions that handle int types
rg -n "func.*parse.*int\|func.*parse.*Int" backend/internal/ -B2 -A10 | head -80Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Verify if the current implementation breaks anywhere by checking for related error handling
rg -n "failed to parse.*int\|type.*assertion" backend/internal/flow/flowexec/store.goRepository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Check the Go database/sql and driver documentation by looking at actual column type handling
rg -n "ColumnTypes\|DatabaseTypeName" backend/internal/ -B2 -A3Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Verify what types modernc.org/sqlite actually returns by checking any existing test data
rg -n "AddRow.*int\|AddRow.*byte\|AddRow.*string" backend/internal/system/database/provider/dbclient_test.go -B2 -A2Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Check the Go documentation in comments about database/sql return types
head -100 backend/internal/system/database/provider/dbclient.go | grep -A10 -B10 "Scan\|interface"Repository: asgardeo/thunder
Length of output: 2718
🏁 Script executed:
# Final check - look at how the buildFlowContextFromResultRow is tested
find backend/internal/flow -name "*test*.go" -exec grep -l "buildFlowContextFromResultRow\|parseInt64" {} \;Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Let's understand what modernc.org/sqlite version 1.37.0 returns for INTEGER columns
# Check if there are any comments or documentation in the codebase
rg -n "sqlite.*int\|modernc.*int" backend/ -B2 -A2Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Verify the issue by checking Go database/sql documentation behavior
# Look for any existing conversion patterns that show awareness of type variability
rg -n "switch.*value\|type.*value" backend/internal/flow/flowexec/store.go -B2 -A8Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Check the full parseInt64 and parseBool to compare implementation patterns
sed -n '264,279p' backend/internal/flow/flowexec/store.go && echo "---" && sed -n '287,305p' backend/internal/userschema/store.goRepository: asgardeo/thunder
Length of output: 1009
🏁 Script executed:
# Final verification - check if modernc.org/sqlite documentation or issues mention type returns
rg -n "TODO\|FIXME\|BUG\|XXX" backend/internal/flow/flowexec/store.go -B2 -A2Repository: asgardeo/thunder
Length of output: 42
🏁 Script executed:
# Verify if there are any actual test assertions or issues that would manifest
rg -n "reflect.DeepEqual\|assert.Equal.*int" backend/internal/flow/ -B2 -A2Repository: asgardeo/thunder
Length of output: 42
Harden parseInt64 to handle multiple driver return types for INT columns.
The current implementation only handles int64 and int, but database/sql drivers may return different types depending on the driver implementation. PostgreSQL (lib/pq) typically returns int64, but SQLite (modernc.org/sqlite) may return other numeric types. This inconsistency with the robust type handling already present in parseBool can cause failures in GetFlowContext depending on which driver returns the value.
🛠️ Suggested fix
import (
"errors"
"fmt"
+ "strconv"
"github.com/asgardeo/thunder/internal/system/config"
dbmodel "github.com/asgardeo/thunder/internal/system/database/model"
"github.com/asgardeo/thunder/internal/system/database/provider"
)
@@
func (s *flowStore) parseInt64(value interface{}) (int64, error) {
if value == nil {
return 0, errors.New("value is nil")
}
- if intVal, ok := value.(int64); ok {
- return intVal, nil
- }
-
- if intVal, ok := value.(int); ok {
- return int64(intVal), nil
- }
-
- return 0, errors.New("failed to parse value as int64")
+ switch v := value.(type) {
+ case int64:
+ return v, nil
+ case int32:
+ return int64(v), nil
+ case int:
+ return int64(v), nil
+ case []byte:
+ return strconv.ParseInt(string(v), 10, 64)
+ case string:
+ return strconv.ParseInt(v, 10, 64)
+ default:
+ return 0, fmt.Errorf("failed to parse value as int64 from %T", value)
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // parseInt64 safely parses an int64 field from the database row with type conversion support | |
| func (s *flowStore) parseInt64(value interface{}) (int64, error) { | |
| if value == nil { | |
| return 0, errors.New("value is nil") | |
| } | |
| if intVal, ok := value.(int64); ok { | |
| return intVal, nil | |
| } | |
| if intVal, ok := value.(int); ok { | |
| return int64(intVal), nil | |
| } | |
| return 0, errors.New("failed to parse value as int64") | |
| } | |
| // parseInt64 safely parses an int64 field from the database row with type conversion support | |
| func (s *flowStore) parseInt64(value interface{}) (int64, error) { | |
| if value == nil { | |
| return 0, errors.New("value is nil") | |
| } | |
| switch v := value.(type) { | |
| case int64: | |
| return v, nil | |
| case int32: | |
| return int64(v), nil | |
| case int: | |
| return int64(v), nil | |
| case []byte: | |
| return strconv.ParseInt(string(v), 10, 64) | |
| case string: | |
| return strconv.ParseInt(v, 10, 64) | |
| default: | |
| return 0, fmt.Errorf("failed to parse value as int64 from %T", value) | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@backend/internal/flow/flowexec/store.go` around lines 264 - 279, The
parseInt64 method on flowStore must be expanded to handle the range of numeric
types database drivers may return (not just int64/int); update parseInt64 to use
a type switch similar to parseBool to accept int32, int16, uint64/uint32/uint16,
float64 (cast via int64), []byte and string (parse with strconv.ParseInt) and
any sql.RawBytes-equivalent, converting each to int64 and returning an error on
overflow or parse failure; reference the parseInt64 function name and flowStore
receiver when locating and replacing the implementation.
846d2d7 to
d0742d6
Compare
KaveeshaPiumini
force-pushed
the
mosip
branch
from
d0742d6 to
0f79dec
Compare
1 participant
Purpose
This pull request updates the database schema and related backend code to introduce a single-column integer primary key (
ID) for theFLOW_CONTEXTandFLOW_USER_DATAtables, and refactors foreign key relationships and queries to use this new key. This change improves data integrity and simplifies joins between these tables. The backend logic is updated accordingly to support these schema changes.Database schema changes:
IDprimary key to bothFLOW_CONTEXTandFLOW_USER_DATAtables in both PostgreSQL and SQLite schemas. The previous composite primary key on(FLOW_ID, DEPLOYMENT_ID)is replaced with a unique constraint. (backend/dbscripts/runtimedb/postgres.sql[1] [2] [3];backend/dbscripts/runtimedb/sqlite.sql[4] [5] [6]FLOW_USER_DATAto reference the newIDcolumn inFLOW_CONTEXTinstead of the composite key. (backend/dbscripts/runtimedb/postgres.sql[1];backend/dbscripts/runtimedb/sqlite.sql[2]Backend code changes:
FlowContextWithUserDataDBstruct to include the newFlowContextIDfield. (backend/internal/flow/flowexec/model.gobackend/internal/flow/flowexec/model.goR108)IDfor joins and inserts, including using a subquery to retrieve theFLOW_CONTEXT.IDwhen inserting intoFLOW_USER_DATA. (backend/internal/flow/flowexec/queries.go[1] [2]IDfield, including adding a helper function to safely parse integer values from database rows. (backend/internal/flow/flowexec/store.go[1] [2] [3]Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
Refactor
Bug Fixes