fix(ENGKNOW-2746): Always use common command substitution in sql. #73
Conversation
gmagnu
commented
Oct 15, 2025
gmagnu
commented
- Always use common command substitution in sql.
- Consolidate command substitution logic.
…solidate command substitution logic.
Junit Tests - Summary4 329 tests ±0 4 164 ✅ ±0 11m 16s ⏱️ +16s Results for commit e0e219c. ± Comparison against base commit c18b082. This pull request removes 2 and adds 2 tests. Note that renamed tests count towards both. |
rickbowman-dev63
left a comment
rickbowman-dev63
left a comment
There was a problem hiding this comment.
I had a few comments and questions
| public static final String KEY_PROJECT_ID = "project_id"; // Project id | ||
| public static final String KEY_DB_PROJECT_ID = "project-id"; // project id |
There was a problem hiding this comment.
Good question, each version has been used in different parts of GOR. Ofcourse it would be good do use either version but that means tracking down the use of the other and change it.
| public static final String KEY_PROJECT = "project"; // Project name | ||
| public static final String KEY_PROJECT_ID = "project_id"; // Project id | ||
| public static final String KEY_DB_PROJECT_ID = "project-id"; // project id | ||
| public static final String KEY_ORGANIZATION_ID = "organization_id"; // Organization name | ||
| public static final String KEY_DB_ORGANIZATION_ID = "organization-id"; // organization id |
There was a problem hiding this comment.
Do you need the comment? It seems redundant with the variable name.
There was a problem hiding this comment.
Yes, it is as the comments look now.
| var key = entry.getKey(); | ||
| var value = entry.getValue(); | ||
| if (value != null) { | ||
| command = command.replace("#{" + key + "}", value.toString()); |
There was a problem hiding this comment.
Do we need to update the GOR documentation for this? Or is it a known feature in GOR query language?
There was a problem hiding this comment.
I believe it is already documented for SQL commands, but it could be documented better. We could also pick a format (project_id vs project-id), but that is a big project as we need to track town all queries using this (including sql link files).
| map.put(KEY_REQUEST_ID, session.getRequestId()); | ||
|
|
||
| return updateMapFromProjectContext(session.getProjectContext(), map); | ||
| //updateMapFromSecurityContext(session.getProjectContext().getFileReader().getSecurityContext(), map); |
| import java.util.Map; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| // TODO: Should be synced with CommandSubstitutions. |
There was a problem hiding this comment.
I don't know if I understand what this comment was suggesting, but did these changes accomplish that?
| return query; | ||
| } | ||
|
|
||
| public static String[] splitResourceHints(String query, String validStart) { |
There was a problem hiding this comment.
I moved those into the CommandSubstitution class.
…solidate command substitution logic.
gmagnu
deleted the
ENGKNOW-2760-norsql-add-project-id-support-like-we-have-for-db-links
branch
