Skip to content

Commit

Permalink
Fix GSISK_agreementTimestamp in token-generation-readmodel-checker (#…
Browse files Browse the repository at this point in the history
…1371)

Co-authored-by: Roberto Taglioni <[email protected]>
  • Loading branch information
shuyec and taglioni-r authored Jan 15, 2025
1 parent 07db1bc commit fb779cc
Show file tree
Hide file tree
Showing 6 changed files with 157 additions and 33 deletions.
29 changes: 16 additions & 13 deletions packages/agreement-platformstate-writer/src/consumerServiceV1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { DynamoDBClient } from "@aws-sdk/client-dynamodb";
import { Logger } from "pagopa-interop-commons";
import {
readAgreementEntry,
updateAgreementStateInPlatformStatesEntry,
updateAgreementStateInPlatformStatesEntryV1,
agreementStateToItemState,
updateAgreementStateOnTokenGenStates,
writeAgreementEntry,
Expand Down Expand Up @@ -65,7 +65,7 @@ export async function handleMessageV1(
agreementState.rejected,
() => {
logger.info(
`Skipping processing of entry ${agreement.id}. Reason: state ${agreement.state}`
`Skipping processing of agreement ${agreement.id}. Reason: state ${agreement.state}`
);
return Promise.resolve();
}
Expand All @@ -86,6 +86,7 @@ export async function handleMessageV1(
agreementState.missingCertifiedAttributes,
agreementState.pending,
agreementState.rejected,
// eslint-disable-next-line sonarjs/no-identical-functions
() => {
logger.info(
`Skipping processing of agreement ${agreement.id}. Reason: state ${agreement.state}`
Expand Down Expand Up @@ -140,17 +141,18 @@ const handleActivationOrSuspension = async (
if (existingAgreementEntry) {
if (existingAgreementEntry.version > incomingVersion) {
logger.info(
`Skipping processing of entry ${existingAgreementEntry}. Reason: a more recent entry already exists`
`Skipping processing of entry ${primaryKey}. Reason: a more recent entry already exists`
);
return Promise.resolve();
} else {
await updateAgreementStateInPlatformStatesEntry(
await updateAgreementStateInPlatformStatesEntryV1({
dynamoDBClient,
primaryKey,
agreementStateToItemState(agreement.state),
incomingVersion,
logger
);
state: agreementStateToItemState(agreement.state),
timestamp: agreementTimestamp,
version: incomingVersion,
logger,
});
}
} else {
if (agreement.stamps.activation === undefined) {
Expand Down Expand Up @@ -245,13 +247,14 @@ const handleUpgrade = async (
);
return Promise.resolve();
} else {
await updateAgreementStateInPlatformStatesEntry(
await updateAgreementStateInPlatformStatesEntryV1({
dynamoDBClient,
primaryKey,
agreementStateToItemState(agreement.state),
msgVersion,
logger
);
state: agreementStateToItemState(agreement.state),
timestamp: agreementTimestamp,
version: msgVersion,
logger,
});
}
} else {
if (agreement.stamps.upgrade === undefined) {
Expand Down
10 changes: 5 additions & 5 deletions packages/agreement-platformstate-writer/src/consumerServiceV2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
agreementStateToItemState,
deleteAgreementEntry,
readAgreementEntry,
updateAgreementStateInPlatformStatesEntry,
updateAgreementStateInPlatformStatesEntryV2,
updateAgreementStateOnTokenGenStates,
writeAgreementEntry,
isLatestAgreement,
Expand Down Expand Up @@ -46,11 +46,11 @@ export async function handleMessageV2(
if (existingAgreementEntry) {
if (existingAgreementEntry.version > msg.version) {
logger.info(
`Skipping processing of entry ${existingAgreementEntry}. Reason: a more recent entry already exists`
`Skipping processing of entry ${primaryKey}. Reason: a more recent entry already exists`
);
return Promise.resolve();
} else {
await updateAgreementStateInPlatformStatesEntry(
await updateAgreementStateInPlatformStatesEntryV2(
dynamoDBClient,
primaryKey,
agreementStateToItemState(agreement.state),
Expand Down Expand Up @@ -111,7 +111,7 @@ export async function handleMessageV2(
);
return Promise.resolve();
} else {
await updateAgreementStateInPlatformStatesEntry(
await updateAgreementStateInPlatformStatesEntryV2(
dynamoDBClient,
primaryKey,
agreementStateToItemState(agreement.state),
Expand Down Expand Up @@ -175,7 +175,7 @@ export async function handleMessageV2(
);
return Promise.resolve();
} else {
await updateAgreementStateInPlatformStatesEntry(
await updateAgreementStateInPlatformStatesEntryV2(
dynamoDBClient,
primaryKey,
agreementStateToItemState(agreement.state),
Expand Down
54 changes: 53 additions & 1 deletion packages/agreement-platformstate-writer/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,59 @@ export const deleteAgreementEntry = async (
logger.info(`Platform-states. Deleted agreement entry ${primaryKey}`);
};

export const updateAgreementStateInPlatformStatesEntry = async (
// This function differs from its V2 implementation, because there could be some timestamps missing for V1 agreements
export const updateAgreementStateInPlatformStatesEntryV1 = async ({
dynamoDBClient,
primaryKey,
state,
timestamp,
version,
logger,
}: {
dynamoDBClient: DynamoDBClient;
primaryKey: PlatformStatesAgreementPK;
state: ItemState;
timestamp: string;
version: number;
logger: Logger;
}): Promise<void> => {
const input: UpdateItemInput = {
ConditionExpression: "attribute_exists(PK)",
Key: {
PK: {
S: primaryKey,
},
},
ExpressionAttributeValues: {
":newState": {
S: state,
},
":newTimestamp": {
S: timestamp,
},
":newVersion": {
N: version.toString(),
},
":newUpdatedAt": {
S: new Date().toISOString(),
},
},
ExpressionAttributeNames: {
"#state": "state",
},
UpdateExpression:
"SET #state = :newState, GSISK_agreementTimestamp = :newTimestamp, version = :newVersion, updatedAt = :newUpdatedAt",
TableName: config.tokenGenerationReadModelTableNamePlatform,
ReturnValues: "NONE",
};
const command = new UpdateItemCommand(input);
await dynamoDBClient.send(command);
logger.info(
`Platform-states. Updated agreement state in entry ${primaryKey}`
);
};

export const updateAgreementStateInPlatformStatesEntryV2 = async (
dynamoDBClient: DynamoDBClient,
primaryKey: PlatformStatesAgreementPK,
state: ItemState,
Expand Down
76 changes: 72 additions & 4 deletions packages/agreement-platformstate-writer/test/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,15 @@ import {
import { genericLogger } from "pagopa-interop-commons";
import { z } from "zod";
import {
updateAgreementStateInPlatformStatesEntry,
updateAgreementStateInPlatformStatesEntryV2,
readAgreementEntry,
writeAgreementEntry,
deleteAgreementEntry,
agreementStateToItemState,
updateAgreementStateOnTokenGenStates,
updateAgreementStateAndDescriptorInfoOnTokenGenStates,
isLatestAgreement,
updateAgreementStateInPlatformStatesEntryV1,
} from "../src/utils.js";
import { dynamoDBClient } from "./utils.js";

Expand All @@ -67,13 +68,80 @@ describe("utils", async () => {
vi.useRealTimers();
});

describe("updateAgreementStateInPlatformStatesEntry", async () => {
describe("updateAgreementStateInPlatformStatesEntryV1", async () => {
it("should throw error if previous entry doesn't exist", async () => {
const primaryKey = makePlatformStatesAgreementPK(
generateId<AgreementId>()
);
const timestamp = new Date().toISOString();
expect(
updateAgreementStateInPlatformStatesEntry(
updateAgreementStateInPlatformStatesEntryV1({
dynamoDBClient,
primaryKey,
state: itemState.active,
timestamp,
version: 1,
logger: genericLogger,
})
).rejects.toThrowError(ConditionalCheckFailedException);
const agreementEntry = await readAgreementEntry(
primaryKey,
dynamoDBClient
);
expect(agreementEntry).toBeUndefined();
});

it("should update state if previous entry exists", async () => {
const primaryKey = makePlatformStatesAgreementPK(
generateId<AgreementId>()
);

const sixHoursAgo = new Date();
sixHoursAgo.setHours(sixHoursAgo.getHours() - 6);

const currentDate = new Date();

const previousAgreementStateEntry: PlatformStatesAgreementEntry = {
...getMockPlatformStatesAgreementEntry(primaryKey),
GSISK_agreementTimestamp: sixHoursAgo.toISOString(),
};
expect(
await readAgreementEntry(primaryKey, dynamoDBClient)
).toBeUndefined();
await writeAgreementEntry(
previousAgreementStateEntry,
dynamoDBClient,
genericLogger
);
await updateAgreementStateInPlatformStatesEntryV1({
dynamoDBClient,
primaryKey,
state: itemState.active,
timestamp: currentDate.toISOString(),
version: 2,
logger: genericLogger,
});

const result = await readAgreementEntry(primaryKey, dynamoDBClient);
const expectedAgreementEntry: PlatformStatesAgreementEntry = {
...previousAgreementStateEntry,
state: itemState.active,
GSISK_agreementTimestamp: currentDate.toISOString(),
version: 2,
updatedAt: new Date().toISOString(),
};

expect(result).toEqual(expectedAgreementEntry);
});
});

describe("updateAgreementStateInPlatformStatesEntryV2", async () => {
it("should throw error if previous entry doesn't exist", async () => {
const primaryKey = makePlatformStatesAgreementPK(
generateId<AgreementId>()
);
expect(
updateAgreementStateInPlatformStatesEntryV2(
dynamoDBClient,
primaryKey,
itemState.active,
Expand Down Expand Up @@ -102,7 +170,7 @@ describe("utils", async () => {
dynamoDBClient,
genericLogger
);
await updateAgreementStateInPlatformStatesEntry(
await updateAgreementStateInPlatformStatesEntryV2(
dynamoDBClient,
primaryKey,
itemState.active,
Expand Down
10 changes: 4 additions & 6 deletions packages/catalog-platformstate-writer/src/consumerServiceV2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,10 @@ export async function handleMessageV2(
previousDescriptor.state !== descriptorState.archived
) {
logger.info(
`Skipping processing of descriptor ${
previousDescriptor?.id
} (the previous descriptor). Reason: ${
!previousDescriptor
? "entry doesn't exists"
: "state is not archived"
`Skipping processing of previous descriptor${
previousDescriptor
? ` ${previousDescriptor.id}. Reason: state ${previousDescriptor.state} is not archived`
: ". Reason: there is only one"
}`
);
return Promise.resolve();
Expand Down
11 changes: 7 additions & 4 deletions packages/token-generation-readmodel-checker/src/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -451,16 +451,14 @@ export async function compareReadModelAgreementsWithPlatformStates({
consumerId: agreement.consumerId,
eserviceId: agreement.eserviceId,
}),
GSISK_agreementTimestamp:
agreement.stamps.activation?.when.toISOString() ||
agreement.createdAt.toISOString(),
GSISK_agreementTimestamp: extractAgreementTimestamp(agreement),
agreementDescriptorId: agreement.descriptorId,
};

const objectsDiff = diff(
ComparisonPlatformStatesAgreementEntry.parse(platformStatesEntry),
expectedPlatformStatesAgreementEntry,
{ sort: true, excludeKeys: ["GSISK_agreementTimestamp"] }
{ sort: true }
);
if (objectsDiff) {
differencesCount++;
Expand Down Expand Up @@ -888,3 +886,8 @@ export const descriptorStateToItemState = (state: DescriptorState): ItemState =>
state === descriptorState.published || state === descriptorState.deprecated
? itemState.active
: itemState.inactive;

export const extractAgreementTimestamp = (agreement: Agreement): string =>
agreement.stamps.upgrade?.when.toISOString() ||
agreement.stamps.activation?.when.toISOString() ||
agreement.createdAt.toISOString();

0 comments on commit fb779cc

Please sign in to comment.