Skip to content

Commit 919eefc

Browse files
sebbi08mergify[bot]jkoenig134Milena-Czierlinskibritsta
authored
Request can be created and sent via a message with an expiration date that has passed (#369)
* chore: respect expireation date of request when creating request and sending message * chore: white space * Update packages/consumption/test/modules/requests/OutgoingRequestsController.test.ts Co-authored-by: Julian König <33655937+jkoenig134@users.noreply.github.com> * Update packages/consumption/src/consumption/ConsumptionCoreErrors.ts Co-authored-by: Milena Czierlinski <146972016+Milena-Czierlinski@users.noreply.github.com> * chore: fix test misstake * chore: fix tests * chore: PR comments * chore: revert Date changes * chore: pr comments * chore: test * chore: fix flaky test * Update packages/consumption/test/modules/requests/OutgoingRequestsController.test.ts Co-authored-by: Milena Czierlinski <146972016+Milena-Czierlinski@users.noreply.github.com> * Update packages/consumption/test/modules/requests/OutgoingRequestsController.test.ts Co-authored-by: Milena Czierlinski <146972016+Milena-Czierlinski@users.noreply.github.com> * chore: improve test performance * chore: improve test performance * chore: PR comments * refactor: add empty lines * Uniqueness of `key` for RelationshipAttributes (#319) * refactor: preapare CreateAttributeRequestItemProcessor to add more validation * feat: add validation for CreateAttributeRequestItemProcessor * test: CreateAttributeRequestItemProcessor key validation * fix: uniqueness for key only instead of pair of owner and key * fix: failing test * test: ensure key uniqueness in attributes.test * test: ensure key uniqueness in CreateRelationshipAttributeRequestItemDVO.test * feat: use database language every database understands * test: ensure key uniqueness only for constant ownership * fix: too late clean up of Attributes * feat: ensure key uniqueness only for RelationshipAttributes that are not in deletion in any way * refactor: extract function call as auxiliary function * refactor: put afterEach block inside describe block * feat: ensure key uniqueness for Recipient within CreateAttributeRequestItemProcessor * refactor: auxiliary function can be applied to queries as well * feat: ensure key uniqueness for Sender within ReadAttributeRequestItemProcessor * feat: ensure key uniqueness only for constant value type * fix: failing tests with async canCreateOutgoingRequestItem method * feat: key uniqueness for Recipient within ReadAttributeRequestItemProcessor * feat: ensure key uniquess for empty owner as placeholder as well * refactor: simplify error message * refactor: move validation to canAccept method * refactor: standardize error messages * test: ensure key uniqueness with empty owner * test: move tests to canAccept block * fix: call accept instead of canAccept * test: refactor canAccept test with SuccessfulValidationResult * test: make tests independent of each other * feat: ensure key uniqueness for empty owner queried with ReadAttributeRequestItem * feat: ensure key uniqueness on Sender site of ProposeAttributeRequestItemProcessor * feat: ensure key uniqueness on Recipient site of ProposeAttributeRequestItemProcessor * refactor: standardize error messages * feat: prohibit Requests that create more than one RelationshipAttribute with same key * fix: title type * test: key uniqueness of incomingRequestsController * test: key uniqueness of OutgoingRequestsController * refactor: remove recursion of IncomingRequestsController * refactor: keep RequestItemGroup in mind * refactor: use auxiliary method for clearness * feat: handle empty owner in IncomingRequestsController and OutgoingRequestsController * test: ensure key uniqueness for empty owner * refactor: move validation from canDecide to canAccept * fix: failing IncomingRequestsController test * test: ensure key uniqueness with RequestItemGroups * fix: wrong DecideRequestParameters for RequestItemGroups * feat: use already known separation sequence * feat: give better validation function name * refactor: do not break existing code blocks anymore * fix: error due to incorrect merging * test: ensure key uniqueness with ProposeAttributeRequestItem * chore: descriptive values should not start with capital letter * refactor: rename error code * feat: distinguish between user mistake and deformed Request * feat: throwing instead of returning error if Request is deformed * feat: reuse key uniqueness error * feat: distinguish between deformed Requests and wrong accept parameters * test: RequestItem cannot be accepted due to incorrect params of User * fix: failing test due to void return * feat: self-explanatory variable names * refactor: more renaming of variables * refactor: unify renaming * feat: simplify error message for unknown recipient of Request * feat: incorporate some review comments * fix: failing test due to wrong error format * feat: incorporate review comment * refactor: prepare reusability of code * fix: error due to merge * feat: use JSON.stringify() instead of own method for extracting identifiers * refactor: reduce redundancy by move auxiliary functions to different file * feat: inform user what key would be redundantly in use * feat: incorporate some review comments * feat: incoporate review comments * refactor: change order of thrown errors * test: can propose and query RelationshipAttribute with same key but different value type * feat: incorporate review comments * refactor: simplify extraction of fragments of mustBeAcceptedItems * refactor: further simplification of extraction of fragments of mustBeAcceptedItems * refactor: simplify extraction of fragments of accepte items * fix: returned instead of continued * test: RequestItem cannot be accepted if key uniqueness is violated regardless of value of mustBeAccepted --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Milena Czierlinski <milena.czierlinski@js-soft.com> * chore: prettier * chore: pr comments --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Julian König <33655937+jkoenig134@users.noreply.github.com> Co-authored-by: Milena Czierlinski <146972016+Milena-Czierlinski@users.noreply.github.com> Co-authored-by: Milena Czierlinski <milena.czierlinski@js-soft.com> Co-authored-by: Britta Stallknecht <146106656+britsta@users.noreply.github.com>
1 parent 6bd9be2 commit 919eefc

File tree

6 files changed

+106
-7
lines changed

6 files changed

+106
-7
lines changed

‎packages/consumption/src/consumption/ConsumptionCoreErrors.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,10 @@ class Requests {
336336
return new CoreError("error.consumption.requests.cannotShareRequestWithYourself", "You cannot share a Request with yourself.");
337337
}
338338

339+
public cannotCreateRequestWithExpirationDateInPast() {
340+
return new CoreError("error.consumption.requests.cannotCreateRequestWithExpirationDateInPast", "You cannot create a Request with an expiration date that is in the past.");
341+
}
342+
339343
private static readonly _decideValidation = class {
340344
public invalidNumberOfItems(message: string) {
341345
return new ApplicationError("error.consumption.requests.decide.validation.invalidNumberOfItems", message);

‎packages/consumption/src/modules/requests/outgoing/OutgoingRequestsController.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ export class OutgoingRequestsController extends ConsumptionBaseController {
4646
return ValidationResult.error(ConsumptionCoreErrors.requests.cannotShareRequestWithYourself());
4747
}
4848

49+
if (parsedParams.content.expiresAt?.isBefore(CoreDate.utc())) {
50+
return ValidationResult.error(ConsumptionCoreErrors.requests.cannotCreateRequestWithExpirationDateInPast());
51+
}
52+
4953
if (parsedParams.peer) {
5054
const relationship = await this.relationshipResolver.getRelationshipToIdentity(parsedParams.peer);
5155

‎packages/consumption/test/modules/requests/OutgoingRequestsController.test.ts‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { IDatabaseConnection } from "@js-soft/docdb-access-abstractions";
2-
import { ApplicationError } from "@js-soft/ts-utils";
2+
import { ApplicationError, sleep } from "@js-soft/ts-utils";
33
import {
44
CreateAttributeRequestItem,
55
IAcceptResponseItem,
@@ -231,6 +231,24 @@ describe("OutgoingRequestsController", function () {
231231
});
232232
});
233233

234+
test("returns a validation result that contains an error for requests that are expired", async function () {
235+
const validationResult = await When.iCallCanCreateForAnOutgoingRequest({
236+
content: {
237+
expiresAt: CoreDate.utc().subtract({ days: 1 }),
238+
items: [
239+
TestRequestItem.from({
240+
mustBeAccepted: false
241+
})
242+
]
243+
}
244+
});
245+
246+
expect(validationResult).errorValidationResult({
247+
code: "error.consumption.requests.cannotCreateRequestWithExpirationDateInPast",
248+
message: "You cannot create a Request with an expiration date that is in the past."
249+
});
250+
});
251+
234252
test("returns a validation result that contains an error for requests that would lead to the creation of more than one RelationshipAttribute with the same key", async function () {
235253
const validationResult = await When.iCallCanCreateForAnOutgoingRequest({
236254
content: {
@@ -745,25 +763,27 @@ describe("OutgoingRequestsController", function () {
745763
await Given.anOutgoingRequestWith({
746764
status: LocalRequestStatus.Expired,
747765
content: {
748-
expiresAt: CoreDate.utc().subtract({ days: 1 }),
766+
expiresAt: CoreDate.utc().add({ millisecond: 100 }),
749767
items: [TestRequestItem.from({ mustBeAccepted: false })]
750768
}
751769
});
770+
await sleep(150);
752771

753772
await When.iCompleteTheOutgoingRequestWith({ responseSourceObject: incomingMessage });
754773
await Then.theRequestMovesToStatus(LocalRequestStatus.Completed);
755774
});
756775

757776
test("throws when trying to complete an expired Request with a Message that was created after the expiry date", async function () {
758-
const incomingMessage = TestObjectFactory.createIncomingIMessage(context.currentIdentity);
759777
await Given.anOutgoingRequestWith({
760778
status: LocalRequestStatus.Expired,
761779
content: {
762-
expiresAt: CoreDate.utc().subtract({ days: 1 }),
780+
expiresAt: CoreDate.utc().add({ millisecond: 100 }),
763781
items: [TestRequestItem.from({ mustBeAccepted: false })]
764782
}
765783
});
784+
await sleep(150);
766785

786+
const incomingMessage = TestObjectFactory.createIncomingIMessage(context.currentIdentity);
767787
await When.iTryToCompleteTheOutgoingRequestWith({ responseSourceObject: incomingMessage });
768788
await Then.itThrowsAnErrorWithTheErrorMessage("*Cannot complete an expired request with a response that was created before the expiration date*");
769789
});
@@ -784,10 +804,11 @@ describe("OutgoingRequestsController", function () {
784804
await Given.anOutgoingRequestWith({
785805
status: LocalRequestStatus.Draft,
786806
content: {
787-
expiresAt: CoreDate.utc().subtract({ days: 1 }),
807+
expiresAt: CoreDate.utc().add({ millisecond: 100 }),
788808
items: [TestRequestItem.from({ mustBeAccepted: false })]
789809
}
790810
});
811+
await sleep(150);
791812

792813
await When.iGetTheOutgoingRequest();
793814
await Then.theRequestIsInStatus(LocalRequestStatus.Expired);
@@ -863,10 +884,11 @@ describe("OutgoingRequestsController", function () {
863884
const outgoingRequest = await Given.anOutgoingRequestWith({
864885
status: LocalRequestStatus.Draft,
865886
content: {
866-
expiresAt: CoreDate.utc().subtract({ days: 1 }),
887+
expiresAt: CoreDate.utc().add({ millisecond: 100 }),
867888
items: [TestRequestItem.from({ mustBeAccepted: false })]
868889
}
869890
});
891+
await sleep(150);
870892

871893
await When.iGetOutgoingRequestsWithTheQuery({ id: outgoingRequest.id.toString() });
872894
await Then.theOnlyReturnedRequestIsInStatus(LocalRequestStatus.Expired);

‎packages/runtime/src/useCases/common/RuntimeErrors.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,13 @@ class Messages {
128128
);
129129
}
130130

131+
public cannotSendMessageWithExpiredRequest() {
132+
return new ApplicationError(
133+
"error.runtime.messages.cannotSendMessageWithExpiredRequest",
134+
"The Message cannot be sent as the contained Request is already expired. Please create a new Request and try again."
135+
);
136+
}
137+
131138
public peerIsInDeletion(addresses: string[]) {
132139
return new ApplicationError(
133140
"error.runtime.messages.peerIsInDeletion",

‎packages/runtime/src/useCases/transport/messages/SendMessage.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { Serializable } from "@js-soft/ts-serval";
22
import { ApplicationError, Result } from "@js-soft/ts-utils";
33
import { OutgoingRequestsController } from "@nmshd/consumption";
44
import { ArbitraryMessageContent, Mail, Notification, Request, ResponseWrapper } from "@nmshd/content";
5-
import { CoreAddress, CoreError, CoreId } from "@nmshd/core-types";
5+
import { CoreAddress, CoreDate, CoreError, CoreId } from "@nmshd/core-types";
66
import { AccountController, File, FileController, MessageController, PeerDeletionStatus, RelationshipsController, RelationshipStatus, TransportCoreErrors } from "@nmshd/transport";
77
import { Inject } from "@nmshd/typescript-ioc";
88
import _ from "lodash";
@@ -143,6 +143,10 @@ export class SendMessageUseCase extends UseCase<SendMessageRequest, MessageDTO>
143143
return RuntimeErrors.general.invalidPropertyValue("The sent Request must have the same content as the LocalRequest.");
144144
}
145145

146+
if (request.expiresAt?.isBefore(CoreDate.utc())) {
147+
return RuntimeErrors.messages.cannotSendMessageWithExpiredRequest();
148+
}
149+
146150
if (!recipient.equals(localRequest.peer)) return RuntimeErrors.general.invalidPropertyValue("The recipient does not match the Request's peer.");
147151

148152
return;

‎packages/runtime/test/transport/messages.test.ts‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { sleep } from "@js-soft/ts-utils";
12
import { ConsumptionIds } from "@nmshd/consumption";
23
import { ConsentRequestItemJSON, Notification } from "@nmshd/content";
34
import { CoreDate } from "@nmshd/core-types";
@@ -6,8 +7,10 @@ import {
67
AttributeDeletedEvent,
78
GetMessagesQuery,
89
IdentityDeletionProcessStatus,
10+
IncomingRequestReceivedEvent,
911
LocalAttributeDeletionStatus,
1012
LocalRequestDTO,
13+
LocalRequestStatus,
1114
MessageReceivedEvent,
1215
MessageSentEvent,
1316
MessageWasReadAtChangedEvent,
@@ -272,6 +275,61 @@ describe("Message errors", () => {
272275
expect(result).toBeAnError("The recipient does not match the Request's peer.", "error.runtime.validation.invalidPropertyValue");
273276
});
274277

278+
test("should throw correct error for trying to send a Message with an expired Request", async () => {
279+
const expiresAt = CoreDate.utc().add({ milliseconds: 100 }).toString();
280+
const createRequestResult = (
281+
await client1.consumption.outgoingRequests.create({
282+
content: {
283+
expiresAt,
284+
items: [requestItem]
285+
},
286+
peer: client2.address
287+
})
288+
).value;
289+
290+
await sleep(150);
291+
292+
const result = await client1.transport.messages.sendMessage({
293+
recipients: [client2.address],
294+
content: createRequestResult.content
295+
});
296+
297+
expect(result).toBeAnError(
298+
"The Message cannot be sent as the contained Request is already expired. Please create a new Request and try again.",
299+
"error.runtime.messages.cannotSendMessageWithExpiredRequest"
300+
);
301+
});
302+
303+
test("should mark a Request as expired when synced after the expiration date", async () => {
304+
const expiresAt = CoreDate.utc().add({ milliseconds: 100 }).toString();
305+
const createRequestResult = (
306+
await client1.consumption.outgoingRequests.create({
307+
content: {
308+
expiresAt,
309+
items: [requestItem]
310+
},
311+
peer: client2.address
312+
})
313+
).value;
314+
315+
const result = await client1.transport.messages.sendMessage({
316+
recipients: [client2.address],
317+
content: createRequestResult.content
318+
});
319+
320+
expect(result).toBeSuccessful();
321+
322+
await sleep(150);
323+
const client1ExpiredRequestResult = await client1.consumption.outgoingRequests.getRequest({ id: createRequestResult.id });
324+
expect(client1ExpiredRequestResult.value.status).toBe(LocalRequestStatus.Expired);
325+
326+
await client2.transport.account.syncEverything();
327+
await client2.eventBus.waitForEvent(IncomingRequestReceivedEvent, (event) => event.data.id === createRequestResult.id);
328+
329+
const client2ExpiredRequestResult = await client2.consumption.incomingRequests.getRequest({ id: createRequestResult.id });
330+
expect(client2ExpiredRequestResult.value.status).toBe(LocalRequestStatus.Expired);
331+
});
332+
275333
describe("Message errors for Relationships that are not active", () => {
276334
test("should throw correct error for trying to send a Message if there are recipients to which no Relationship exists", async () => {
277335
const result = await client1.transport.messages.sendMessage({

0 commit comments

Comments
 (0)