Skip to content

⚡ Optimize security profile lookup to O(1) #117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
SeCuReDmE-main-dev merged 2 commits into from
Apr 28, 2026
Merged

⚡ Optimize security profile lookup to O(1) #117

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e57c63f
⚡ optimize security profile lookup to O(1) using Map
SeCuReDmE-main-dev Apr 28, 2026
ab4cce0
Update ReaAaS-N-frontend/src/services/securityProfileCatalog.ts
SeCuReDmE-main-dev Apr 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions ReaAaS-N-frontend/src/services/securityProfileCatalog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,17 @@ export const SECURITY_PROFILE_CATALOG: SecurityProfile[] = [

export const DEFAULT_SECURITY_PROFILE_ID: SecurityProfileId = 'general';

const SECURITY_PROFILE_MAP = new Map<string, SecurityProfile>(
SECURITY_PROFILE_CATALOG.map((profile) => [profile.id, profile])
);
Comment on lines +112 to +114

Copilot AI Apr 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SECURITY_PROFILE_MAP is built once from SECURITY_PROFILE_CATALOG, but SECURITY_PROFILE_CATALOG is exported as a mutable SecurityProfile[]. If any consumer mutates the exported catalog at runtime, lookups via getSecurityProfile/isSecurityProfileId will silently diverge from the catalog. Consider making the catalog immutable (e.g., ReadonlyArray and/or freezing) or otherwise ensuring the exported list cannot be mutated after module init.

Copilot uses AI. Check for mistakes.
Comment on lines +112 to +114

Copilot AI Apr 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The map is typed as Map<string, SecurityProfile>, which discards the fact that the only valid keys are SecurityProfileId values. Using Map<SecurityProfileId, SecurityProfile> (and narrowing/casting only at the boundary where you accept arbitrary strings) improves type-safety and keeps the isSecurityProfileId type guard aligned with the data source.

Copilot uses AI. Check for mistakes.

@SeCuReDmE-main-dev SeCuReDmE-main-dev Apr 28, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

@google-labs-jules google-labs-jules Bot Apr 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I apologize, but I am unable to access the external GitHub discussion link due to network restrictions in my environment. Could you please copy the relevant feedback directly into this thread?


export function getSecurityProfile(profileId?: string): SecurityProfile {
return (
SECURITY_PROFILE_CATALOG.find((profile) => profile.id === profileId) ??
SECURITY_PROFILE_CATALOG.find((profile) => profile.id === DEFAULT_SECURITY_PROFILE_ID)!
(profileId ? SECURITY_PROFILE_MAP.get(profileId) : undefined) ??
SECURITY_PROFILE_MAP.get(DEFAULT_SECURITY_PROFILE_ID)!
);
Comment on lines 116 to 120

Copilot AI Apr 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change alters lookup semantics and adds a new invariant (the map must always include DEFAULT_SECURITY_PROFILE_ID). There are existing Vitest tests for other service catalogs (e.g., algorithmCatalog.test.ts), but none for securityProfileCatalog. Adding a small unit test suite for getSecurityProfile (valid/invalid/undefined inputs + default fallback) and isSecurityProfileId would help prevent regressions.

Copilot uses AI. Check for mistakes.

@SeCuReDmE-main-dev SeCuReDmE-main-dev Apr 28, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

}
Comment on lines +112 to 121

@gemini-code-assist gemini-code-assist Bot Apr 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current implementation performs a Map lookup for the default security profile on every call to getSecurityProfile. Additionally, the use of the non-null assertion operator (!) on line 119 assumes the default ID always exists in the catalog, which could lead to a runtime crash if the catalog is misconfigured.

Consider pre-resolving the default profile at the module level. This improves performance by avoiding redundant lookups and ensures that any configuration issues (like a missing default profile) are caught during module initialization rather than at runtime.

const SECURITY_PROFILE_MAP = new Map<string, SecurityProfile>(
  SECURITY_PROFILE_CATALOG.map((profile) => [profile.id, profile])
);

const DEFAULT_PROFILE = SECURITY_PROFILE_MAP.get(DEFAULT_SECURITY_PROFILE_ID)!;

export function getSecurityProfile(profileId?: string): SecurityProfile {
  return (profileId ? SECURITY_PROFILE_MAP.get(profileId) : undefined) ?? DEFAULT_PROFILE;
}


export function isSecurityProfileId(profileId: string): profileId is SecurityProfileId {
return SECURITY_PROFILE_CATALOG.some((profile) => profile.id === profileId);
return SECURITY_PROFILE_MAP.has(profileId);
}
Loading