From ed727b051fbe1eb31a2046381dc412e5c8390ecf Mon Sep 17 00:00:00 2001 From: garciafdezpatricia Date: Tue, 23 Apr 2024 12:57:59 +0200 Subject: [PATCH] coverage and lint --- CHANGELOG.md | 3 +- src/acp/control.test.ts | 167 +++++++++++++++++++++++++++++++++++++++- src/acp/policy.test.ts | 24 +++++- src/acp/policy.ts | 6 +- 4 files changed, 189 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba04b7d2a3..d009921491 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,8 @@ The following changes are pending, and will be applied on the next major release ## Unreleased changes ### Patch changes -- Fixed #2339: Unnamed policies are now returned by `getResourcePolicyAll` if an optional argument + +- Fixed #2339: Unnamed policies are now returned by `getResourcePolicyAll` if an optional argument `{ acceptBlankNodes: true }` is specified. This additional argument makes this a non-breaking change, as the current type signature isn't changed. - `getThing` now supports Blank Node identifiers in addition to IRIs and skolems to refer to a subject. diff --git a/src/acp/control.test.ts b/src/acp/control.test.ts index 7ea666d47e..17efbf753f 100644 --- a/src/acp/control.test.ts +++ b/src/acp/control.test.ts @@ -43,8 +43,10 @@ import { } from "./control"; import { internal_createControl, + internal_getAcr, internal_getControl, internal_getControlAll, + internal_setAcr, internal_setControl, } from "./control.internal"; import { acp, rdf } from "../constants"; @@ -52,9 +54,11 @@ import type { WithServerResourceInfo } from "../interfaces"; import { getIri, getUrl, getUrlAll } from "../thing/get"; import { asIri, + asUrl, createThing, getThing, getThingAll, + removeThing, setThing, } from "../thing/thing"; import { addMockAcrTo, mockAcrFor } from "./mock"; @@ -527,6 +531,24 @@ describe("getAcrPolicyUrlAll", () => { expect(policyUrls).toEqual([]); }); + + it("does not return policies if acr does not have an anchor node", () => { + const accessControlResource = mockAcrFor("https://some.pod/resource"); + const existingControl = addUrl( + createThing({ url: getSourceUrl(accessControlResource) }), + acp.accessMembers, + "https://some.pod/policy-resource#policy", + ); + const resourceWithAcr = addMockAcrTo( + mockSolidDatasetFrom("https://some.pod/resource"), + setThing(accessControlResource, existingControl), + ); + const acr = internal_getAcr(resourceWithAcr); + const acrUrl = getSourceUrl(acr); + const updatedAcr = removeThing(acr, acrUrl); + const updatedResource = internal_setAcr(resourceWithAcr, updatedAcr); + expect(getAcrPolicyUrlAll(updatedResource)).toHaveLength(0); + }); }); describe("getMemberAcrPolicyUrlAll", () => { @@ -575,6 +597,24 @@ describe("getMemberAcrPolicyUrlAll", () => { expect(policyUrls).toEqual([]); }); + + it("does not return policies if acr does not have an anchor node", () => { + const accessControlResource = mockAcrFor("https://some.pod/resource"); + const existingControl = addUrl( + createThing({ url: getSourceUrl(accessControlResource) }), + acp.accessMembers, + "https://some.pod/policy-resource#policy", + ); + const resourceWithAcr = addMockAcrTo( + mockSolidDatasetFrom("https://some.pod/resource"), + setThing(accessControlResource, existingControl), + ); + const acr = internal_getAcr(resourceWithAcr); + const acrUrl = getSourceUrl(acr); + const updatedAcr = removeThing(acr, acrUrl); + const updatedResource = internal_setAcr(resourceWithAcr, updatedAcr); + expect(getMemberAcrPolicyUrlAll(updatedResource)).toHaveLength(0); + }); }); describe("removeAcrPolicyUrl", () => { @@ -705,6 +745,29 @@ describe("removeAcrPolicyUrl", () => { "https://some.pod/policy-resource#policy", ); }); + + it("returns the resource unchanged if acr does not have an anchor node", () => { + const accessControlResource = mockAcrFor("https://some.pod/resource"); + const existingControl = addUrl( + createThing({ url: getSourceUrl(accessControlResource) }), + acp.accessMembers, + "https://some.pod/policy-resource#policy", + ); + const resourceWithAcr = addMockAcrTo( + mockSolidDatasetFrom("https://some.pod/resource"), + setThing(accessControlResource, existingControl), + ); + const acr = internal_getAcr(resourceWithAcr); + const acrUrl = getSourceUrl(acr); + const updatedAcr = removeThing(acr, acrUrl); + const updatedResource = internal_setAcr(resourceWithAcr, updatedAcr); + expect( + removeAcrPolicyUrl( + updatedResource, + "https://some.pod/policy-resource#policy", + ), + ).toEqual(updatedResource); + }); }); describe("removeMemberAcrPolicyUrl", () => { @@ -835,6 +898,33 @@ describe("removeMemberAcrPolicyUrl", () => { "https://some.pod/policy-resource#policy", ); }); + + it("returns the resource unchanged if acr does not have an anchor node", () => { + let accessControlResource = mockAcrFor("https://some.pod/resource"); + let existingControl = createThing({ + url: getSourceUrl(accessControlResource), + }); + existingControl = addUrl( + existingControl, + acp.accessMembers, + "https://some.pod/policy-resource#policy", + ); + accessControlResource = setThing(accessControlResource, existingControl); + const resourceWithAcr = addMockAcrTo( + mockSolidDatasetFrom("https://some.pod/resource"), + accessControlResource, + ); + const acr = internal_getAcr(resourceWithAcr); + const acrUrl = getSourceUrl(acr); + const updatedAcr = removeThing(acr, acrUrl); + const updatedResource = internal_setAcr(resourceWithAcr, updatedAcr); + expect( + removeMemberAcrPolicyUrl( + updatedResource, + "https://some.pod/policy-resource#policy", + ), + ).toEqual(updatedResource); + }); }); describe("removeAcrPolicyUrlAll", () => { @@ -929,6 +1019,24 @@ describe("removeAcrPolicyUrlAll", () => { "https://some.pod/policy-resource#policy", ); }); + + it("returns the resource unchanged if acr does not have an anchor node", () => { + const accessControlResource = mockAcrFor("https://some.pod/resource"); + const existingControl = addUrl( + createThing({ url: getSourceUrl(accessControlResource) }), + acp.accessMembers, + "https://some.pod/policy-resource#policy", + ); + const resourceWithAcr = addMockAcrTo( + mockSolidDatasetFrom("https://some.pod/resource"), + setThing(accessControlResource, existingControl), + ); + const acr = internal_getAcr(resourceWithAcr); + const acrUrl = getSourceUrl(acr); + const updatedAcr = removeThing(acr, acrUrl); + const updatedResource = internal_setAcr(resourceWithAcr, updatedAcr); + expect(removeAcrPolicyUrlAll(updatedResource)).toEqual(updatedResource); + }); }); describe("removeMemberAcrPolicyUrlAll", () => { @@ -1023,6 +1131,26 @@ describe("removeMemberAcrPolicyUrlAll", () => { "https://some.pod/policy-resource#policy", ); }); + + it("returns the resource unchanged if acr does not have an anchor node", () => { + const accessControlResource = mockAcrFor("https://some.pod/resource"); + const existingControl = addUrl( + createThing({ url: getSourceUrl(accessControlResource) }), + acp.accessMembers, + "https://some.pod/policy-resource#policy", + ); + const resourceWithAcr = addMockAcrTo( + mockSolidDatasetFrom("https://some.pod/resource"), + setThing(accessControlResource, existingControl), + ); + const acr = internal_getAcr(resourceWithAcr); + const acrUrl = getSourceUrl(acr); + const updatedAcr = removeThing(acr, acrUrl); + const updatedResource = internal_setAcr(resourceWithAcr, updatedAcr); + expect(removeMemberAcrPolicyUrlAll(updatedResource)).toEqual( + updatedResource, + ); + }); }); describe("addPolicyUrl", () => { @@ -1042,8 +1170,14 @@ describe("addPolicyUrl", () => { updatedResourceWithAcr.internal_acp.acr, ); + const difference = updatedControls.filter((control) => { + return !oldControls.some((oldControl) => { + return asUrl(control) === asUrl(oldControl); + }); + })[0]; + expect(updatedControls).toHaveLength(oldControls.length + 1); - expect(getUrl(updatedControls[updatedControls.length - 1], acp.apply)).toBe( + expect(getUrl(difference, acp.apply)).toBe( "https://some.pod/policy-resource#policy", ); }); @@ -1110,10 +1244,17 @@ describe("addMemberPolicyUrl", () => { const updatedControls = getThingAll( updatedResourceWithAcr.internal_acp.acr, ); + + const difference = updatedControls.filter((control) => { + return !oldControls.some((oldControl) => { + return asUrl(control) === asUrl(oldControl); + }); + })[0]; + expect(updatedControls).toHaveLength(oldControls.length + 1); - expect( - getUrl(updatedControls[updatedControls.length - 1], acp.applyMembers), - ).toBe("https://some.pod/policy-resource#policy"); + expect(getUrl(difference, acp.applyMembers)).toBe( + "https://some.pod/policy-resource#policy", + ); }); it("does not remove existing Policy URLs", () => { @@ -1276,6 +1417,24 @@ describe("getMemberPolicyUrlAll", () => { expect(policyUrls).toEqual([]); }); + + it("returns the resource unchanged if acr does not have an anchor node", () => { + const accessControlResource = mockAcrFor("https://some.pod/resource"); + const existingControl = addUrl( + createThing({ url: getSourceUrl(accessControlResource) }), + acp.applyMembers, + "https://some.pod/policy-resource#policy", + ); + const resourceWithAcr = addMockAcrTo( + mockSolidDatasetFrom("https://some.pod/resource"), + setThing(accessControlResource, existingControl), + ); + const acr = internal_getAcr(resourceWithAcr); + const acrUrl = getSourceUrl(acr); + const updatedAcr = removeThing(acr, acrUrl); + const updatedResource = internal_setAcr(resourceWithAcr, updatedAcr); + expect(getMemberPolicyUrlAll(updatedResource)).toHaveLength(0); + }); }); describe("removePolicyUrl", () => { diff --git a/src/acp/policy.test.ts b/src/acp/policy.test.ts index 959fb7d89c..0501cb7777 100644 --- a/src/acp/policy.test.ts +++ b/src/acp/policy.test.ts @@ -37,6 +37,7 @@ import { createThing, getThing, getThingAll, + removeThing, setThing, } from "../thing/thing"; import { @@ -45,7 +46,7 @@ import { getAcrPolicyUrlAll, getPolicyUrlAll, } from "./control"; -import { internal_getAcr } from "./control.internal"; +import { internal_getAcr, internal_setAcr } from "./control.internal"; import { addMockAcrTo, mockAcrFor } from "./mock"; import { createPolicy, @@ -68,6 +69,7 @@ import { setResourcePolicy, } from "./policy"; import { fromRdfJsDataset } from "../rdfjs"; +import { SolidClientError } from "../interfaces"; jest.spyOn(globalThis, "fetch").mockImplementation( async () => @@ -1141,6 +1143,26 @@ describe("removeResourceAcrPolicy", () => { expect(getResourceAcrPolicyAll(updatedPolicyDataset)).toHaveLength(1); }); + + it("errors if the acr does not have an anchor node matching its url", () => { + let mockedAcr = mockAcrFor("https://some.pod/resource"); + let mockedPolicy1 = createThing({ + url: `${getSourceUrl(mockedAcr)}#policy1`, + }); + mockedPolicy1 = setUrl(mockedPolicy1, rdf.type, acp.Policy); + mockedAcr = setThing(mockedAcr, mockedPolicy1); + const mockedResourceWithAcr = addMockAcrTo( + mockSolidDatasetFrom("https://some.pod/resource"), + mockedAcr, + ); + const acr = internal_getAcr(mockedResourceWithAcr); + const acrUrl = getSourceUrl(acr); + const updatedAcr = removeThing(acr, acrUrl); + const updatedResource = internal_setAcr(mockedResourceWithAcr, updatedAcr); + expect(() => getResourcePolicyAll(updatedResource)).toThrow( + SolidClientError, + ); + }); }); describe("setAllowModes", () => { diff --git a/src/acp/policy.ts b/src/acp/policy.ts index 4f9b2efd49..82ee233cc1 100644 --- a/src/acp/policy.ts +++ b/src/acp/policy.ts @@ -459,11 +459,7 @@ export function getResourcePolicyAll( return ( getTermAll(acrSubj, acp.accessControl) .reduce((prev, accessControlId) => { - const accessControl = getThing(acr, accessControlId.value); - if (accessControl === null) { - // If the access control isn't found, there are no policies to add. - return prev; - } + const accessControl = getThing(acr, accessControlId.value)!; const accessControlPolicies = getTermAll( accessControl, acp.apply,