feat: add preLoadedConnections array option to Jackson options - #4074
feat: add preLoadedConnections array option to Jackson options#4074Vansh98789 wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughJackson now accepts typed in-memory SAML and OIDC connections. Initialization routes these connections through shared creation logic and retains support for path-based preloaded connections. ChangesPreloaded connection support
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Jackson
participant ConnectionWriter
participant OIDCorSAMLCreation
Jackson->>ConnectionWriter: passes each preLoadedConnections item
ConnectionWriter->>OIDCorSAMLCreation: selects OIDC or SAML creation
OIDCorSAMLCreation-->>ConnectionWriter: creates the connection
ConnectionWriter-->>Jackson: logs the loaded connection
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
npm/src/typings.ts (1)
69-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the exported
Connectiontype in the path loader.
npm/src/loadConnection.tsstill declares the same four-member union as a privateconnectiontype.npm/src/index.tsnow uses the exportedConnectiontype. Keep one union definition so path-based and in-memory loading cannot drift when a connection variant changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@npm/src/typings.ts` around lines 69 - 73, Update the path loader in loadConnection.ts to import and reuse the exported Connection type from typings.ts instead of declaring its own private four-member union. Remove the duplicate connection type definition while preserving the existing path-based loading behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@npm/src/typings.ts`:
- Around line 69-73: Update the path loader in loadConnection.ts to import and
reuse the exported Connection type from typings.ts instead of declaring its own
private four-member union. Remove the duplicate connection type definition while
preserving the existing path-based loading behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4a0266d3-59de-45a1-b81e-ff2239c10d7b
📒 Files selected for processing (2)
npm/src/index.tsnpm/src/typings.ts
|
Thanks @Vansh98789, the team will review your PR soon. |
What does this PR do?
Adds a new
preLoadedConnectionsoption toJacksonOptionthat accepts an in-memory array of SAML/OIDC connections, as an alternative to the existing path-basedpreLoadedConnectionoption.The current
preLoadedConnectionoption loads a directory path and uses dynamicimport()(inloadConnection.ts) to read.jsconnection files. This breaks in Next.js 15 + Turbopack environments where dynamic imports from runtime paths are not supported.This PR:
Connectiontype union intypings.tspreLoadedConnections?: Connection[]toJacksonOptionConnectionAPIController— no dynamic imports or file system accesspreLoadedConnectionflow untouched for backward compatibilitywriteConnectionhelper between both pathsFixes #3644
Type of Change
How Should This Be Tested?
preLoadedConnectionscontaining a SAML connection (withrawMetadata) and/or an OIDC connection (withoidcDiscoveryUrl.connectionAPIController.getConnections({ tenant, product }).preLoadedConnectionoption still works.Example
Tests
Checklist
Summary by CodeRabbit