Skip to content

Commit 7b0bb79

Browse files
britstamergify[bot]Milena-CzierlinskiMagnus-Kuhnjkoenig134
authored
Add CanCreateRelationshipUseCase and handling of expired RelationshipTemplates (#212)
* Fix: throw error when responding to Request of expired RelationshipTemplate * fix: throw error when try to create Relationship with expired RelationshipTemplate * test: add first version of test * feat: adjust auxiliary function for RelationshipTemplate * feat: adjust createTemplate testUtil function * fix: return error instead of throwing error * test: temporary changes for tests * fix: test by using auxiliary function * test: shortened test duration * chore: comment out code more consistently * chore: make default expirationDateTime recognizable * chore: add error for delay function * feat: LocalRequest is expired when source RelationshipTemplate is expired * test: add test for CreateRelationshipUseCase * chore: remove comments * test: add tests for updateStatusBasedOnTemplateExpiration function * chore: adjust isExpired function for reasons of consistency * fix: ensure to throw error only if no other is more relevant * chore: adjust test naming to be more appropriate * fix: ensure more precisely to throw error only if no other is more relevant * feat: throw error only for onNewRelationship * fix: prettier * chore: renaming variables and use $in operator * feat: implement canCreate route for Relationships * chore: use CanCreateRelationshipUseCase in other use cases * chore: use CanCreateRelationshipUseCase in more other use cases * chore: rename error message relationshipAlreadyExists * refactor: naming of response variable of CanCreateRelationshipUseCase * fix: do not use CanCreateRelationshipUseCase in other UseCases * feat: add noRequestToAccept error for CanCreateRelationshipUseCase * refactor: do not expose the term LocalRequest in error messages * feat: expose CanCreateRelationshipUseCase to facade * test: should not create Relationship with CreateRelationshipUseCase if RelationshipTemplateContent used * feat: use CreateRelationshipUseCase only for ArbitraryRelationshipTemplateContent * fix: missing imports * fix: unused import * chore: build schema of CanCreateRelationshipUseCase * refactor: choose more appropriate variable name * chore: reduce CanCreateRelationshipUseCase to split pull requests * chore: reduce CanCreateRelationshipUseCase to split pull requests * fix: error message of CreateRelationship use case * feat: update expiry of Requests also within CreateRelationshipUseCase * fix: error message used for contradicting cases * feat: only allow Request which expires after RelationshipTemplate for new Relationships * test: request for new Relationship cannot expire after RelationshipTemplate * fix: tests for expiration date of Requests * refactor: rename to error for Relationships instead of RelationshipTemplates * chore: remove unused Runtime error * refactor: throw only Runtime errors in use cases * refactor: add function for getting existing Relationships * refactor: reuse CreateRelationshipRequest for CanCreateUseCase * fix: incorrect use of recordNotFound error * feat: define canSendRelationship method in Controller * fix: failed Result for CanAcceptIncomingRequest and CanRejectIncomingRequest use cases * feat: add canCreateRelationship Backbone service * fix: value not used for sendRelationship method * fix: value not used for sendRelationship method * feat: use canCreate route of Backbone * fix: request type of canCreateRelationship Backbone route * fix: canCreateRelationship Backbone route * fix: return value of canCreateRelationship Backbone route * refactor: use variable for peerAddress * chore: do not use imprecise Backbone result * fix: return value of canSendRelationship method * feat: use canSendRelationship method in corresponding use case * chore: remove unneccessary type conversion * feat: incorporate RelationshipTemplate expiry in canSendRelationship method * refactor: CanCreateRelationshipUseCase * feat: taking rejected Requests into account * refactor: return code and message instead of error to avoid misunderstandings * refactor: take rejected Requests at a lower level into account * chore: simply code because cases handled at a lower level * feat: handle expired RelationshipTemplate error appropriately * test: does not change status of rejected Request when RelationshipTemplate expires * fix: tests * chore: update Backbone version * test: automatic update of Request of expired RelationshipTemplate * test: CanCreateRelationshipUseCase * fix: LocalRequest test * refactor: rename temporary error message * chore: remove todo comments * feat: apply Request expiry update in GetIncomingRequest(s)UseCase * test: update Request status to expired when querying Requests if RelationshipTemplate expired * chore: remove unused Runtime error relationshipCurrentlyExists * chore: use imported sleep function instead of own delay function * feat: add minimum of type validation for creationContent of CanCreateRelationshipUseCase * feat: integrate comments * fix: adjust test * feat: generate expiration date for Request in onNewRelationship property if none is set * refactor: use elvis operator * test: remove assertIsCanCreateRelationshipFailureResponse * feat: update expirationDate of Request if RelationshipTemplate expires * test: set expiresAt of Request if none provided * fix: clone template content in tests * feat: remove 10 second tolerance * refactor: move validation into controller * refactor: getExistingRelationship * feat: add min to CoreDate * refactor: remove updates from create rel and get request use cases * feat/refactor: update after received, update during get * refactor: adapt local request * refactor: rename incoming requests getter * refactor: remove now unnecessary validation * fix: remove debugging stuff from tests * fix: consumption * fix: runtime tests * test: reduce sleep duration after removing tolerance * test/fix: add tests and fix revealed errors * test: typos and status settings * chore: remove unused package * test: sleep longer * refactor/test: getOutgoingRequestWithUpdateExpiry and use it * refactor: try-catch in sendRelationship * fix: adapt the tests to the refactoring * fix: more adaptations * feat: canCreate never throws * feat: canCreate with error codes * refactor: use regex, debugging cleanup * fix: variable access * fix: unfound typescript-ioc module * fix: unused caught error * fix: unknown transportServices1 variable * fix: failing test of DeciderModule * fix: shady instanceOf check * refactor: add empty line * refactor: switch some lines in tests * test: proper variable naming * refactor: remove redundant test * refactor: rephrase of test name to give more context * fix: copy-paste error in tests * chore: bump backbone * refactor: remove unused code from request controller * test: emphazise tests expecting errors * refactor: re-simplify naming * fix: do not get the template for every incoming request query * refactor: de-dupe code * refactor: remove empty line again * refactor: distinguish success and failure response of CanCreateRelationshipUseCase * fix: use correct CanCreateRelationshipResonse type in tests * refactor: remove unused expiration code * refactor: simplify * fix: satisfy compiler --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Milena Czierlinski <[email protected]> Co-authored-by: mkuhn <[email protected]> Co-authored-by: Magnus Kuhn <[email protected]> Co-authored-by: Julian König <[email protected]>
1 parent 9876cf3 commit 7b0bb79

File tree

26 files changed

+741
-118
lines changed

26 files changed

+741
-118
lines changed

.dev/compose.backbone.env

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
BACKBONE_VERSION=6.10.0
1+
BACKBONE_VERSION=6.13.2

packages/consumption/src/consumption/ConsumptionController.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,8 @@ export class ConsumptionController {
124124
this,
125125
this.transport.eventBus,
126126
this.accountController.identity,
127-
this.accountController.relationships
127+
this.accountController.relationships,
128+
this.accountController.relationshipTemplates
128129
).init();
129130

130131
const notificationItemProcessorRegistry = new NotificationItemProcessorRegistry(this, this.getDefaultNotificationItemProcessors());

packages/consumption/src/modules/requests/incoming/IncomingRequestsController.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ export class IncomingRequestsController extends ConsumptionBaseController {
3939
private readonly identity: { address: CoreAddress },
4040
private readonly relationshipResolver: {
4141
getRelationshipToIdentity(id: CoreAddress): Promise<Relationship | undefined>;
42+
getExistingRelationshipToIdentity(id: CoreAddress): Promise<Relationship | undefined>;
43+
},
44+
private readonly relationshipTemplateResolver: {
45+
getRelationshipTemplate(id: CoreId): Promise<RelationshipTemplate | undefined>;
4246
}
4347
) {
4448
super(ConsumptionControllerName.RequestsController, parent);
@@ -60,6 +64,11 @@ export class IncomingRequestsController extends ConsumptionBaseController {
6064
statusLog: []
6165
});
6266

67+
if (!(await this.relationshipResolver.getExistingRelationshipToIdentity(CoreAddress.from(infoFromSource.peer))) && infoFromSource.expiresAt) {
68+
request.content.expiresAt = CoreDate.min(infoFromSource.expiresAt, request.content.expiresAt);
69+
request.updateStatusBasedOnExpiration();
70+
}
71+
6372
await this.localRequests.create(request);
6473

6574
this.eventBus.publish(new IncomingRequestReceivedEvent(this.identity.address.toString(), request));
@@ -95,7 +104,8 @@ export class IncomingRequestsController extends ConsumptionBaseController {
95104
source: {
96105
reference: template.id,
97106
type: "RelationshipTemplate"
98-
}
107+
},
108+
expiresAt: template.cache!.expiresAt
99109
};
100110
}
101111

@@ -392,6 +402,7 @@ export class IncomingRequestsController extends ConsumptionBaseController {
392402
if (!requestDoc) return;
393403

394404
const localRequest = LocalRequest.from(requestDoc);
405+
395406
return await this.updateRequestExpiry(localRequest);
396407
}
397408

@@ -434,4 +445,5 @@ export class IncomingRequestsController extends ConsumptionBaseController {
434445
interface InfoFromSource {
435446
peer: ICoreAddress;
436447
source: ILocalRequestSource;
448+
expiresAt?: CoreDate;
437449
}

packages/consumption/src/modules/requests/local/LocalRequest.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ export class LocalRequest extends CoreSynchronizable implements ILocalRequest {
117117
public isExpired(comparisonDate: CoreDate = CoreDate.utc()): boolean {
118118
if (!this.content.expiresAt) return false;
119119

120-
return comparisonDate.isAfter(this.content.expiresAt.add({ seconds: 10 }));
120+
return comparisonDate.isAfter(this.content.expiresAt);
121121
}
122122

123123
public updateStatusBasedOnExpiration(comparisonDate: CoreDate = CoreDate.utc()): boolean {
@@ -130,4 +130,17 @@ export class LocalRequest extends CoreSynchronizable implements ILocalRequest {
130130

131131
return false;
132132
}
133+
134+
public updateExpirationDateBasedOnTemplateExpiration(templateExpiresAt: CoreDate): boolean {
135+
if (this.source?.type !== "RelationshipTemplate") return false;
136+
137+
if (this.status === LocalRequestStatus.Completed || this.status === LocalRequestStatus.Expired) return false;
138+
139+
if (!this.isExpired()) {
140+
this.content.expiresAt = CoreDate.min(templateExpiresAt, this.content.expiresAt);
141+
return true;
142+
}
143+
144+
return false;
145+
}
133146
}

packages/consumption/test/modules/requests/IncomingRequestsController.test.ts

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { IDatabaseConnection } from "@js-soft/docdb-access-abstractions";
22
import { IRequest, IRequestItemGroup, RejectResponseItem, Request, RequestItemGroup, ResponseItem, ResponseItemGroup, ResponseItemResult } from "@nmshd/content";
33
import { CoreDate, CoreId } from "@nmshd/core-types";
4-
import { CoreIdHelper, TransportLoggerFactory } from "@nmshd/transport";
4+
import { CoreIdHelper, RelationshipTemplate, TransportLoggerFactory } from "@nmshd/transport";
55
import {
66
ConsumptionIds,
77
DecideRequestItemGroupParametersJSON,
@@ -65,6 +65,34 @@ describe("IncomingRequestsController", function () {
6565
await Then.eventHasBeenPublished(IncomingRequestReceivedEvent);
6666
});
6767

68+
test("takes the expiration date from the Template if the Request has no expiration date", async function () {
69+
const timestamp = CoreDate.utc();
70+
const incomingTemplate = TestObjectFactory.createIncomingRelationshipTemplate(timestamp);
71+
await When.iCreateAnIncomingRequestWith({ requestSourceObject: incomingTemplate });
72+
await Then.theRequestHasExpirationDate(timestamp);
73+
await Then.theRequestIsInStatus(LocalRequestStatus.Expired);
74+
});
75+
76+
test("takes the expiration date from the Template if the Request has a later expiration date", async function () {
77+
const timestamp = CoreDate.utc().add({ days: 1 });
78+
const incomingTemplate = TestObjectFactory.createIncomingRelationshipTemplate(timestamp);
79+
await When.iCreateAnIncomingRequestWith({
80+
requestSourceObject: incomingTemplate,
81+
receivedRequest: TestObjectFactory.createRequestWithOneItem({ expiresAt: timestamp.add({ days: 1 }) })
82+
});
83+
await Then.theRequestHasExpirationDate(timestamp);
84+
});
85+
86+
test("takes the expiration date from the Request if the Template has a later expiration date", async function () {
87+
const timestamp = CoreDate.utc().add({ days: 1 });
88+
const incomingTemplate = TestObjectFactory.createIncomingRelationshipTemplate(timestamp.add({ days: 1 }));
89+
await When.iCreateAnIncomingRequestWith({
90+
requestSourceObject: incomingTemplate,
91+
receivedRequest: TestObjectFactory.createRequestWithOneItem({ expiresAt: timestamp })
92+
});
93+
await Then.theRequestHasExpirationDate(timestamp);
94+
});
95+
6896
test("uses the ID of the given Request if it exists", async function () {
6997
const request = TestObjectFactory.createRequestWithOneItem({ id: await CoreIdHelper.notPrefixed.generate() });
7098

@@ -999,6 +1027,7 @@ describe("IncomingRequestsController", function () {
9991027
items: [TestRequestItem.from({ mustBeAccepted: false })]
10001028
});
10011029
const template = TestObjectFactory.createIncomingIRelationshipTemplate();
1030+
context.templateToReturnFromGetTemplate = RelationshipTemplate.from(template);
10021031

10031032
let cnsRequest = await context.incomingRequestsController.received({
10041033
receivedRequest: request,

packages/consumption/test/modules/requests/RequestsIntegrationTest.ts

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {
1414
ResponseItemResult,
1515
ResponseResult
1616
} from "@nmshd/content";
17-
import { CoreAddress, CoreId, ICoreId } from "@nmshd/core-types";
17+
import { CoreAddress, CoreDate, CoreId, ICoreId } from "@nmshd/core-types";
1818
import { CoreIdHelper, IConfigOverwrite, IMessage, IRelationshipTemplate, Message, Relationship, RelationshipTemplate, SynchronizedCollection, Transport } from "@nmshd/transport";
1919
import {
2020
ConsumptionController,
@@ -57,6 +57,8 @@ export class RequestsTestsContext {
5757
public currentIdentity: CoreAddress;
5858
public mockEventBus = new MockEventBus();
5959
public relationshipToReturnFromGetRelationshipToIdentity: Relationship | undefined;
60+
public relationshipToReturnFromGetExistingRelationshipToIdentity: Relationship | undefined;
61+
public templateToReturnFromGetTemplate: RelationshipTemplate | undefined;
6062
public consumptionController: ConsumptionController;
6163

6264
private constructor() {
@@ -110,7 +112,11 @@ export class RequestsTestsContext {
110112
address: account.accountController.identity.address
111113
},
112114
{
113-
getRelationshipToIdentity: () => Promise.resolve(context.relationshipToReturnFromGetRelationshipToIdentity)
115+
getRelationshipToIdentity: () => Promise.resolve(context.relationshipToReturnFromGetRelationshipToIdentity),
116+
getExistingRelationshipToIdentity: () => Promise.resolve(context.relationshipToReturnFromGetExistingRelationshipToIdentity)
117+
},
118+
{
119+
getRelationshipTemplate: () => Promise.resolve(context.templateToReturnFromGetTemplate)
114120
}
115121
);
116122

@@ -132,6 +138,8 @@ export class RequestsTestsContext {
132138
this.validationResult = undefined;
133139
this.actionToTry = undefined;
134140
this.relationshipToReturnFromGetRelationshipToIdentity = undefined;
141+
this.relationshipToReturnFromGetExistingRelationshipToIdentity = undefined;
142+
this.templateToReturnFromGetTemplate = undefined;
135143

136144
TestRequestItemProcessor.numberOfApplyIncomingResponseItemCalls = 0;
137145

@@ -222,6 +230,12 @@ export class RequestsGiven {
222230
requestSourceObject: params.requestSource
223231
});
224232

233+
try {
234+
this.context.templateToReturnFromGetTemplate = RelationshipTemplate.from(params.requestSource as any);
235+
} catch (_) {
236+
// the source is not a template
237+
}
238+
225239
await this.moveIncomingRequestToStatus(localRequest, params.status);
226240

227241
this.context.givenLocalRequest = localRequest;
@@ -306,11 +320,13 @@ export class RequestsGiven {
306320
}
307321

308322
private async moveOutgoingRequestToStatus(localRequest: LocalRequest, status: LocalRequestStatus) {
309-
if (localRequest.status === status) return;
323+
const updatedRequest = await this.context.outgoingRequestsController.getOutgoingRequest(localRequest.id);
324+
325+
if (updatedRequest!.status === status) return;
310326

311327
if (isStatusAAfterStatusB(status, LocalRequestStatus.Draft)) {
312328
await this.context.outgoingRequestsController.sent({
313-
requestId: localRequest.id,
329+
requestId: updatedRequest!.id,
314330
requestSourceObject: TestObjectFactory.createOutgoingIMessage(this.context.currentIdentity)
315331
});
316332
}
@@ -1039,6 +1055,12 @@ export class RequestsThen {
10391055
return Promise.resolve();
10401056
}
10411057

1058+
public theRequestHasExpirationDate(expiresAt: CoreDate): Promise<void> {
1059+
expect(this.context.localRequestAfterAction?.content.expiresAt).toStrictEqual(expiresAt);
1060+
1061+
return Promise.resolve();
1062+
}
1063+
10421064
public theRequestHasCorrectItemCount(itemCount: number): Promise<void> {
10431065
expect(this.context.localRequestAfterAction?.content.items).toHaveLength(itemCount);
10441066

@@ -1205,8 +1227,6 @@ function isStatusAAfterStatusB(a: LocalRequestStatus, b: LocalRequestStatus): bo
12051227

12061228
function getIntegerValue(status: LocalRequestStatus): number {
12071229
switch (status) {
1208-
case LocalRequestStatus.Expired:
1209-
return -1;
12101230
case LocalRequestStatus.Draft:
12111231
return 0;
12121232
case LocalRequestStatus.Open:
@@ -1215,6 +1235,8 @@ function getIntegerValue(status: LocalRequestStatus): number {
12151235
return 2;
12161236
case LocalRequestStatus.ManualDecisionRequired:
12171237
return 3;
1238+
case LocalRequestStatus.Expired:
1239+
return 4;
12181240
case LocalRequestStatus.Decided:
12191241
return 5;
12201242
case LocalRequestStatus.Completed:

packages/consumption/test/modules/requests/local/LocalRequest.test.ts

Lines changed: 70 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ describe("LocalRequest", function () {
3535
});
3636

3737
test("adds a status log entry on status change", function () {
38-
const request = TestObjectFactory.createLocalRequestWith({});
38+
const request = TestObjectFactory.createLocalRequestWith({ status: LocalRequestStatus.Draft });
3939
request.changeStatus(LocalRequestStatus.Open);
4040

4141
expect(request.statusLog).toHaveLength(1);
@@ -50,7 +50,7 @@ describe("LocalRequest", function () {
5050
});
5151

5252
describe("updateStatusBasedOnExpiration", function () {
53-
test("sets the status to expired when the request is expired", function () {
53+
test("sets the status to expired when the Request is expired", function () {
5454
const request = TestObjectFactory.createLocalRequestWith({
5555
contentProperties: {
5656
expiresAt: CoreDate.utc().subtract({ days: 1 })
@@ -61,18 +61,84 @@ describe("LocalRequest", function () {
6161
expect(request.status).toStrictEqual(LocalRequestStatus.Expired);
6262
});
6363

64-
test("does not change the status when the request is expired but already completed", function () {
64+
test("does not change the status when the Request is expired but already completed", function () {
6565
const request = TestObjectFactory.createLocalRequestWith({ status: LocalRequestStatus.Completed });
6666

6767
request.updateStatusBasedOnExpiration();
6868
expect(request.status).toStrictEqual(LocalRequestStatus.Completed);
6969
});
7070

71-
test("does not change the status when the request is expired but already expired", function () {
71+
test("does not change the status when the Request is expired but already expired", function () {
7272
const request = TestObjectFactory.createLocalRequestWith({ status: LocalRequestStatus.Expired });
7373

7474
request.updateStatusBasedOnExpiration();
7575
expect(request.status).toStrictEqual(LocalRequestStatus.Expired);
7676
});
7777
});
78+
79+
describe("updateExpirationDateBasedOnTemplateExpiration", function () {
80+
test("sets the expiration date if the Request doesn't have an expiration date", function () {
81+
const request = TestObjectFactory.createUnansweredLocalRequestBasedOnTemplateWith({});
82+
expect(request.content.expiresAt).toBeUndefined();
83+
const timestamp = CoreDate.utc().subtract({ days: 1 });
84+
85+
request.updateExpirationDateBasedOnTemplateExpiration(timestamp);
86+
expect(request.content.expiresAt).toStrictEqual(timestamp);
87+
});
88+
89+
test("sets the expiration date if the Request has already been rejected", function () {
90+
const request = TestObjectFactory.createRejectedLocalRequestBasedOnTemplateWith({});
91+
92+
const timestamp = CoreDate.utc().subtract({ days: 1 });
93+
94+
request.updateExpirationDateBasedOnTemplateExpiration(timestamp);
95+
expect(request.content.expiresAt).toStrictEqual(timestamp);
96+
});
97+
98+
test("does not change the expiration date when the Request expires before the Template", function () {
99+
const timestamp = CoreDate.utc().add({ days: 1 });
100+
const request = TestObjectFactory.createUnansweredLocalRequestBasedOnTemplateWith({
101+
contentProperties: {
102+
expiresAt: timestamp
103+
}
104+
});
105+
106+
request.updateExpirationDateBasedOnTemplateExpiration(timestamp.add({ days: 1 }));
107+
expect(request.content.expiresAt).toStrictEqual(timestamp);
108+
});
109+
110+
test("does not change the expiration date when the Request is already completed", function () {
111+
const request = TestObjectFactory.createRejectedLocalRequestBasedOnTemplateWith({
112+
status: LocalRequestStatus.Completed
113+
});
114+
115+
const timestamp = CoreDate.utc().subtract({ days: 1 });
116+
117+
request.updateExpirationDateBasedOnTemplateExpiration(timestamp);
118+
expect(request.content.expiresAt).toBeUndefined();
119+
});
120+
121+
test("does not change the expiration date when the Request is already expired", function () {
122+
const timestamp = CoreDate.utc().subtract({ days: 1 });
123+
124+
const request = TestObjectFactory.createUnansweredLocalRequestBasedOnTemplateWith({
125+
status: LocalRequestStatus.Expired,
126+
contentProperties: {
127+
expiresAt: timestamp
128+
}
129+
});
130+
131+
request.updateExpirationDateBasedOnTemplateExpiration(timestamp.subtract({ days: 1 }));
132+
expect(request.content.expiresAt).toStrictEqual(timestamp);
133+
});
134+
135+
test("does not set the expiration date when Message instead of RelationshipTemplate is used", function () {
136+
const request = TestObjectFactory.createLocalRequestWith({});
137+
138+
const timestamp = CoreDate.utc().subtract({ days: 1 });
139+
140+
request.updateExpirationDateBasedOnTemplateExpiration(timestamp);
141+
expect(request.content.expiresAt).toBeUndefined();
142+
});
143+
});
78144
});

0 commit comments

Comments
 (0)