Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

PIN 5558 - Get eservice descriptor #1218

Merged
merged 3 commits into from
Nov 27, 2024
+47 −4
Merged

PIN 5558 - Get eservice descriptor #1218

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3a5b6e1
add attribute check on delegate and delegator
MalpenZibo Nov 12, 2024
eb1452f
fix review comments
MalpenZibo Nov 21, 2024
3d2fb19
fix bug in case of missing delegation
MalpenZibo Nov 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions packages/backend-for-frontend/src/api/catalogApiConverter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export function toBffCatalogDescriptorEService(
descriptor: catalogApi.EServiceDescriptor,
producerTenant: tenantApi.Tenant,
agreement: agreementApi.Agreement | undefined,
requesterTenant: tenantApi.Tenant
requesterTenants: tenantApi.Tenant[]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment below: since isMine should probably consider only the requester, while hasCertifiedAttributes should consider the requester and the consumer delegators, we can split the two params here

Suggested change
requesterTenants: tenantApi.Tenant[]
requesterTenant: tenantApi.Tenant,
consumerDelegators: tenantApi.Tenant[]

): bffApi.CatalogDescriptorEService {
const activeDescriptor = getLatestActiveDescriptor(eservice);
return {
Expand All @@ -107,8 +107,12 @@ export function toBffCatalogDescriptorEService(
technology: eservice.technology,
descriptors: getNotDraftDescriptor(eservice).map(toCompactDescriptor),
agreement: agreement && toBffCompactAgreement(agreement, eservice),
isMine: isRequesterEserviceProducer(requesterTenant.id, eservice),
hasCertifiedAttributes: hasCertifiedAttributes(descriptor, requesterTenant),
isMine: requesterTenants.some((t) =>
isRequesterEserviceProducer(t.id, eservice)
),
hasCertifiedAttributes: requesterTenants.some((t) =>
hasCertifiedAttributes(descriptor, t)
),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure isMine shall consider the delegators, because it's about the requester being the producer of the eservice, the one that can manage it/edit it, while if I am the delegate of a consumer delegation I want to consume the eservice. Should this maybe be set to true in the case the requester is the delegate of a producer delegation? (to discuss with the "Capofila" team, probably, as a further task).

I would also add a comment to explain why hasCertifiedAttributes checks the delegators, it's hard to understand because the name is misleading now. If you think it makes sense, let's also add the same comment as the API spec's description for the field.

Suggested change
isMine: requesterTenants.some((t) =>
isRequesterEserviceProducer(t.id, eservice)
),
hasCertifiedAttributes: requesterTenants.some((t) =>
hasCertifiedAttributes(descriptor, t)
),
isMine: isRequesterEserviceProducer(requesterTenant.id, eservice),
hasCertifiedAttributes: [requesterTenant, ...consumerDelegators].some((t) =>
hasCertifiedAttributes(descriptor, t)
/* True in case:
- the requester has the certified attributes required to consume the eservice, or
- the requester is the delegated consumer for the eservice and the delegator has the certified attributes required to consume the eservice */
),

isSubscribed: isAgreementSubscribed(agreement),
activeDescriptor: activeDescriptor
? toCompactDescriptor(activeDescriptor)
Expand Down
22 changes: 15 additions & 7 deletions packages/backend-for-frontend/src/services/catalogService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ import {
assertNotDelegatedEservice,
assertRequesterIsProducer,
} from "./validators.js";
import {
getAllDelegations,
getTenantsFromDelegation,
} from "./delegationService.js";

export type CatalogService = ReturnType<typeof catalogServiceBuilder>;

Expand Down Expand Up @@ -528,12 +532,16 @@ export function catalogServiceBuilder(
descriptor
);

const requesterTenant = await tenantProcessClient.tenant.getTenant({
headers,
params: {
id: requesterId,
},
});
const requesterTenants = await getTenantsFromDelegation(
ecamellini marked this conversation as resolved.
Show resolved Hide resolved
tenantProcessClient,
await getAllDelegations(delegationProcessClient, headers, {
delegateIds: [requesterId],
delegationStates: ["ACTIVE"],
MalpenZibo marked this conversation as resolved.
Show resolved Hide resolved
kind: "DELEGATED_CONSUMER",
MalpenZibo marked this conversation as resolved.
Show resolved Hide resolved
eserviceIds: [eserviceId],
}),
headers
);
const producerTenant = await tenantProcessClient.tenant.getTenant({
headers,
params: {
Expand Down Expand Up @@ -571,7 +579,7 @@ export function catalogServiceBuilder(
descriptor,
producerTenant,
agreement,
requesterTenant
Array.from(requesterTenants, ([, tenant]) => tenant)
MalpenZibo marked this conversation as resolved.
Show resolved Hide resolved
),
};
},
Expand Down