-
Notifications
You must be signed in to change notification settings - Fork 165
Description
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:
- IdempotencyHandler.ts (3 issues):
#saveSuccessfulResult
(line 422),#saveInProgressOrReturnExistingResult
(line 359),#deleteInProgressRecord
(line 340) - 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) - BasePersistenceLayer.ts (1 issue):
envVarsService
(line 31) - IdempotencyRecord.ts (1 issue):
status
(line 45) - 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?
- Code Quality: Improves code clarity by explicitly marking immutable members as readonly
- SonarQube Compliance: Resolves 8 MAJOR severity issues (typescript:S2933)
- Type Safety: Prevents accidental reassignment of these critical configuration and state members
- Developer Intent: Makes the immutable nature of these members explicit to future maintainers
- 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
-
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)
-
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
-
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
- Run
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:
AZItlgaBcEUs2753LD6h
- IdempotencyHandler.ts #saveSuccessfulResultAZItlgaBcEUs2753LD6g
- IdempotencyHandler.ts #saveInProgressOrReturnExistingResultAZItlgaBcEUs2753LD6f
- IdempotencyHandler.ts #deleteInProgressRecordAZItlgZocEUs2753LD6e
- IdempotencyRecord.ts statusAZItlgZccEUs2753LD6d
- BasePersistenceLayer.ts envVarsServiceAZItlgY0cEUs2753LD6U
- DynamoDBPersistenceLayer.ts clientAZItlgY0cEUs2753LD6W
- DynamoDBPersistenceLayer.ts dataAttrAZItlgY0cEUs2753LD6X
- DynamoDBPersistenceLayer.ts expiryAttrAZItlgY0cEUs2753LD6Y
- DynamoDBPersistenceLayer.ts keyAttrAZItlgY0cEUs2753LD6Z
- DynamoDBPersistenceLayer.ts sortKeyAttrAZItlgY0cEUs2753LD6a
- DynamoDBPersistenceLayer.ts staticPkValueAZItlgY0cEUs2753LD6b
- DynamoDBPersistenceLayer.ts statusAttrAZItlgY0cEUs2753LD6c
- DynamoDBPersistenceLayer.ts tableNameAZItlgaecEUs2753LD6j
- EnvironmentVariablesService.ts idempotencyDisabledVariableAZItlgaecEUs2753LD6i
- EnvironmentVariablesService.ts functionNameVariable
Acknowledgment
- This request meets Powertools for AWS Lambda (TypeScript) Tenets
- Should this be considered in other Powertools for AWS Lambda languages? i.e. Python, Java, and .NET
Future readers
Please react with 👍 and your use case to help us understand customer demand.
Metadata
Metadata
Assignees
Labels
Type
Projects
Status