Skip to content

Maintenance: Mark private members as readonly in idempotency package #4253

@dreamorosi

Description

@dreamorosi

Summary

The packages/idempotency package contains 8 MAJOR severity code quality issues identified by SonarQube where class members are never reassigned after initialization but are not marked as readonly. These issues affect core idempotency functionality across multiple files:

  1. IdempotencyHandler.ts (3 issues): #saveSuccessfulResult (line 422), #saveInProgressOrReturnExistingResult (line 359), #deleteInProgressRecord (line 340)
  2. DynamoDBPersistenceLayer.ts (6 issues): client (line 53), dataAttr (line 54), expiryAttr (line 55), keyAttr (line 57), sortKeyAttr (line 58), staticPkValue (line 59), statusAttr (line 60), tableName (line 61)
  3. BasePersistenceLayer.ts (1 issue): envVarsService (line 31)
  4. IdempotencyRecord.ts (1 issue): status (line 45)
  5. EnvironmentVariablesService.ts (2 issues): idempotencyDisabledVariable (line 23), functionNameVariable (line 22)

All of these are members that are initialized once and never modified, making them ideal candidates for the readonly modifier.

Why is this needed?

  1. Code Quality: Improves code clarity by explicitly marking immutable members as readonly
  2. SonarQube Compliance: Resolves 8 MAJOR severity issues (typescript:S2933)
  3. Type Safety: Prevents accidental reassignment of these critical configuration and state members
  4. Developer Intent: Makes the immutable nature of these members explicit to future maintainers
  5. Best Practices: Follows TypeScript best practices for class member declarations

The Idempotency package is a critical utility for preventing duplicate Lambda function executions, so maintaining high code quality standards here is particularly important for reliability.

Solution

Important

The following changes are included as reference to help you understand the refactoring. Before implementing, please make sure to check the codebase and ensure that they make sense and they are exhaustive.

1. Fix IdempotencyHandler.ts (3 members)

Current code:

class IdempotencyHandler {
  // ...
  #deleteInProgressRecord: (record: IdempotencyRecord) => Promise<void>;
  // ...
  #saveInProgressOrReturnExistingResult: (record: IdempotencyRecord, remainingTimeInMillis?: number) => Promise<IdempotencyRecord>;
  // ...
  #saveSuccessfulResult: (record: IdempotencyRecord, result: unknown, remainingTimeInMillis?: number) => Promise<void>;
  // ...
}

Improved code:

class IdempotencyHandler {
  // ...
  readonly #deleteInProgressRecord: (record: IdempotencyRecord) => Promise<void>;
  // ...
  readonly #saveInProgressOrReturnExistingResult: (record: IdempotencyRecord, remainingTimeInMillis?: number) => Promise<IdempotencyRecord>;
  // ...
  readonly #saveSuccessfulResult: (record: IdempotencyRecord, result: unknown, remainingTimeInMillis?: number) => Promise<void>;
  // ...
}

2. Fix DynamoDBPersistenceLayer.ts (6 members)

Current code:

class DynamoDBPersistenceLayer extends BasePersistenceLayer {
  // ...
  client: DynamoDBClient;
  dataAttr: string;
  expiryAttr: string;
  keyAttr: string;
  sortKeyAttr: string;
  staticPkValue: string;
  statusAttr: string;
  tableName: string;
  // ...
}

Improved code:

class DynamoDBPersistenceLayer extends BasePersistenceLayer {
  // ...
  readonly client: DynamoDBClient;
  readonly dataAttr: string;
  readonly expiryAttr: string;
  readonly keyAttr: string;
  readonly sortKeyAttr: string;
  readonly staticPkValue: string;
  readonly statusAttr: string;
  readonly tableName: string;
  // ...
}

3. Fix BasePersistenceLayer.ts (1 member)

Current code:

abstract class BasePersistenceLayer {
  // ...
  envVarsService: EnvironmentVariablesService;
  // ...
}

Improved code:

abstract class BasePersistenceLayer {
  // ...
  readonly envVarsService: EnvironmentVariablesService;
  // ...
}

4. Fix IdempotencyRecord.ts (1 member)

Current code:

class IdempotencyRecord {
  // ...
  status: IdempotencyRecordStatus;
  // ...
}

Improved code:

class IdempotencyRecord {
  // ...
  readonly status: IdempotencyRecordStatus;
  // ...
}

5. Fix EnvironmentVariablesService.ts (2 members)

Current code:

class EnvironmentVariablesService extends CommonEnvironmentVariablesService {
  // ...
  functionNameVariable: string;
  idempotencyDisabledVariable: string;
  // ...
}

Improved code:

class EnvironmentVariablesService extends CommonEnvironmentVariablesService {
  // ...
  readonly functionNameVariable: string;
  readonly idempotencyDisabledVariable: string;
  // ...
}

These changes:

  • Make the immutable nature of these members explicit
  • Prevent accidental reassignment in future code changes
  • Follow TypeScript best practices for class design
  • Improve code readability and maintainability

Implementation Details

  1. Files to modify:

    • packages/idempotency/src/IdempotencyHandler.ts (lines 340, 359, 422)
    • packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts (lines 53, 54, 55, 57, 58, 59, 60, 61)
    • packages/idempotency/src/persistence/BasePersistenceLayer.ts (line 31)
    • packages/idempotency/src/persistence/IdempotencyRecord.ts (line 45)
    • packages/idempotency/src/config/EnvironmentVariablesService.ts (lines 22, 23)
  2. Testing:

    • Ensure all existing tests continue to pass
    • Run type checking to verify no TypeScript errors are introduced
    • Verify that the readonly modifier doesn't break any existing functionality
  3. Validation:

    • Run npm run test:unit -w packages/idempotency to ensure no regressions
    • Run npm run lint -w packages/idempotency to verify code style compliance
    • Run npm run build -ws to verify TypeScript compilation
    • Run SonarQube analysis to confirm issues are resolved

Additional Context

This issue was identified as part of a SonarQube code quality review. The Idempotency package is a critical utility for preventing duplicate Lambda function executions, making code quality improvements here particularly valuable for reliability and maintainability.

These changes are purely declarative improvements that should not change any functionality or behavior - they simply make the immutable nature of these class members explicit.

SonarQube Issue Keys:

Acknowledgment

Future readers

Please react with 👍 and your use case to help us understand customer demand.

Metadata

Metadata

Assignees

No one assigned

    Labels

    confirmedThe scope is clear, ready for implementationgood-first-issueSomething that is suitable for those who want to start contributinghelp-wantedWe would really appreciate some support from community for this oneidempotencyThis item relates to the Idempotency Utility

    Type

    No type

    Projects

    Status

    Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions