Skip to content

Commit

Permalink
Enforces attribute constraints in data method
Browse files Browse the repository at this point in the history 
I believe a regression or oversight impacted the `data` method that it did not enforce schema `readOnly` and `required` options.
  • Loading branch information
@tywalch
tywalch committed Oct 16, 2022
1 parent 77dd8bb commit 1e877ec
Show file tree
Hide file tree
Showing 9 changed files with 139 additions and 61 deletions.
77 changes: 43 additions & 34 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3401,7 +3401,7 @@ await StoreLocations
o.add(a.tenant, newTenant); // electrodb "set" -> dynamodb "set"
o.add(a.rent, 100); // electrodb "number" -> dynamodb "number"
o.subtract(a.deposit, 200); // electrodb "number" -> dynamodb "number"
o.remove(a.leaseEndDate); // electrodb "string" -> dynamodb "string"
o.remove(attr.discount); // electrodb "number" -> dynamodb "number"
o.append(a.rentalAgreement, [{ // electrodb "list" -> dynamodb "list"
type: "ammendment", // electrodb "map" -> dynamodb "map"
detail: "no soup for you"
Expand All @@ -3425,40 +3425,49 @@ Response Format:
Equivalent DocClient Parameters:
```json
{
"UpdateExpression": "SET #category = :category_u0, #rent = #rent + :rent_u0, #deposit = #deposit - :deposit_u0, #rentalAgreement = list_append(#rentalAgreement, :rentalAgreement_u0), #totalFees = #totalFees + #petFee REMOVE #leaseEndDate, #gsi2sk ADD #tenant :tenant_u0, #leaseHolders :tenant_u0 DELETE #tags :tags_u0, #contact :contact_u0",
"ExpressionAttributeNames": {
"#category": "category",
"#tenant": "tenant",
"#rent": "rent",
"#deposit": "deposit",
"#leaseEndDate": "leaseEndDate",
"#rentalAgreement": "rentalAgreement",
"#tags": "tags",
"#contact": "contact",
"#totalFees": "totalFees",
"#petFee": "petFee",
"#leaseHolders": "leaseHolders",
"#gsi2sk": "gsi2sk"
},
"ExpressionAttributeValues": {
":category0": "food/coffee",
":category_u0": "food/meal",
":tenant_u0": ["larry"],
":rent_u0": 100,
":deposit_u0": 200,
":rentalAgreement_u0": [{
"type": "amendment",
"detail": "no soup for you"
}],
":tags_u0": ["coffee"], // <- DynamoDB Set
":contact_u0": ["555-345-2222"], // <- DynamoDB Set
"UpdateExpression": "SET #category = :category_u0, #deposit = #deposit - :deposit_u0, #rentalAgreement = list_append(#rentalAgreement, :rentalAgreement_u0), #totalFees = #totalFees + #petFee, #cityId = :cityId_u0, #mallId = :mallId_u0, #buildingId = :buildingId_u0, #storeId = :storeId_u0, #__edb_e__ = :__edb_e___u0, #__edb_v__ = :__edb_v___u0 REMOVE #discount ADD #tenant :tenant_u0, #rent :rent_u0, #leaseHolders :tenant_u0 DELETE #tags :tags_u0, #contact :contact_u0",
"ExpressionAttributeNames": {
"#category": "category",
"#tenant": "tenant",
"#rent": "rent",
"#deposit": "deposit",
"#discount": "discount",
"#rentalAgreement": "rentalAgreement",
"#tags": "tags",
"#contact": "contact",
"#totalFees": "totalFees",
"#petFee": "petFee",
"#leaseHolders": "leaseHolders",
"#buildingId": "buildingId",
"#cityId": "cityId",
"#mallId": "mallId",
"#storeId": "storeId",
"#__edb_e__": "__edb_e__", "#__edb_v__": "__edb_v__",
},
"TableName": "electro",
"Key": {
"pk": `$mallstoredirectory#cityid_12345#mallid_eastpointe`,
"sk": "$mallstore_1#buildingid_a34#storeid_lattelarrys"
},
"ConditionExpression": "#category = :category0"
"ExpressionAttributeValues": {
":buildingId_u0": "A34",
":cityId_u0": "portland",
":category0": "food/coffee",
":category_u0": "food/meal",
":tenant_u0": ["larry"],
":rent_u0": 100,
":deposit_u0": 200,
":rentalAgreement_u0": [{
"type": "amendment",
"detail": "no soup for you"
}],
":tags_u0": ["coffee"],
":contact_u0": ["555-345-2222"],
":mallId_u0": "EastPointe",
":storeId_u0": "LatteLarrys",
":__edb_e___u0": "MallStore", ":__edb_v___u0": "1",
},
"TableName": "electro",
"Key": {
"pk": "$mallstoredirectory#cityid_portland#mallid_eastpointe",
"sk": "$mallstore_1#buildingid_a34#storeid_lattelarrys"
},
"ConditionExpression": "#category = :category0"
}
```

Expand Down
4 changes: 2 additions & 2 deletions index.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,7 @@ export type BatchWriteGo<ResponseType> = <O extends BulkOptions>(options?: O) =>

export type ParamRecord<Options = ParamOptions> = <P>(options?: Options) => P;

export interface ElectroError extends Error {
export class ElectroError extends Error {
readonly name: 'ElectroError';
readonly code: number;
readonly date: number;
Expand Down Expand Up @@ -733,7 +733,7 @@ export interface ElectroValidationErrorFieldReference<T extends Error = Error> {
readonly cause: T | undefined;
}

export interface ElectroValidationError<T extends Error = Error> extends ElectroError {
export class ElectroValidationError<T extends Error = Error> extends ElectroError {
readonly fields: ReadonlyArray<ElectroValidationErrorFieldReference<T>>;
}

Expand Down
9 changes: 8 additions & 1 deletion index.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
const { Entity } = require("./src/entity");
const { Service } = require("./src/service");
const { createCustomAttribute } = require('./src/schema');
const { ElectroError, ElectroValidationError, ElectroUserValidationError, ElectroAttributeValidationError } = require('./src/errors');

module.exports = { Entity, Service, createCustomAttribute };
module.exports = {
Entity,
Service,
createCustomAttribute,
ElectroError,
ElectroValidationError,
};
5 changes: 5 additions & 0 deletions src/clauses.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,11 @@ let clauses = {
}
try {
state.query.updateProxy.invokeCallback(cb);
for (const path of Object.keys(state.query.update.refs)) {
const operation = state.query.update.impacted[path];
const attribute = state.query.update.refs[path];
entity.model.schema.checkOperation(attribute, operation);
}
return state;
} catch(err) {
state.setError(err);
Expand Down
6 changes: 4 additions & 2 deletions src/operations.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ class ExpressionState {
this.impacted = {};
this.expression = "";
this.prefix = prefix || "";
this.refs = {};
}

incrementName(name) {
Expand Down Expand Up @@ -316,8 +317,9 @@ class ExpressionState {
return this.expression;
}

setImpacted(operation, path) {
setImpacted(operation, path, ref) {
this.impacted[path] = operation;
this.refs[path] = ref;
}
}

Expand Down Expand Up @@ -407,7 +409,7 @@ class AttributeOperationProxy {
}

const formatted = template(options, target, paths.expression, ...attributeValues);
builder.setImpacted(operation, paths.json);
builder.setImpacted(operation, paths.json, target);
if (canNest) {
seen.add(paths.expression);
seen.add(formatted);
Expand Down
10 changes: 10 additions & 0 deletions src/schema.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1361,6 +1361,16 @@ class Schema {
return record;
}

checkOperation(attribute, operation) {
if (!attribute) {
throw new e.ElectroAttributeValidationError(path, `Attribute "${path}" does not exist on model.`);
} else if (attribute.required && operation === 'remove') {
throw new e.ElectroAttributeValidationError(attribute.path, `Attribute "${attribute.path}" is Required and cannot be removed`);
} else if (attribute.readOnly) {
throw new e.ElectroAttributeValidationError(attribute.path, `Attribute "${attribute.path}" is Read-Only and cannot be updated`);
}
}

checkRemove(paths = []) {
for (const path of paths) {
const attribute = this.traverser.getPath(path);
Expand Down
10 changes: 4 additions & 6 deletions test/connected.update.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1154,7 +1154,7 @@ describe("Update Item", () => {
op.add(attr.tenant, newTenant);
op.add(attr.rent, 100);
op.subtract(attr.deposit, 200);
op.remove(attr.leaseEndDate);
op.remove(attr.discount);
op.append(attr.rentalAgreement, [{type: "amendment", detail: "no soup for you"}]);
op.delete(attr.tags, 'coffee');
op.del(attr.contact, '555-345-2222');
Expand All @@ -1165,26 +1165,24 @@ describe("Update Item", () => {
.params();

expect(JSON.parse(JSON.stringify(allParameters))).to.deep.equal({
"UpdateExpression": "SET #category = :category_u0, #deposit = #deposit - :deposit_u0, #rentalAgreement = list_append(#rentalAgreement, :rentalAgreement_u0), #totalFees = #totalFees + #petFee, #cityId = :cityId_u0, #mallId = :mallId_u0, #buildingId = :buildingId_u0, #storeId = :storeId_u0, #__edb_e__ = :__edb_e___u0, #__edb_v__ = :__edb_v___u0 REMOVE #leaseEndDate, #gsi2sk ADD #tenant :tenant_u0, #rent :rent_u0, #leaseHolders :tenant_u0 DELETE #tags :tags_u0, #contact :contact_u0",
"UpdateExpression": "SET #category = :category_u0, #deposit = #deposit - :deposit_u0, #rentalAgreement = list_append(#rentalAgreement, :rentalAgreement_u0), #totalFees = #totalFees + #petFee, #cityId = :cityId_u0, #mallId = :mallId_u0, #buildingId = :buildingId_u0, #storeId = :storeId_u0, #__edb_e__ = :__edb_e___u0, #__edb_v__ = :__edb_v___u0 REMOVE #discount ADD #tenant :tenant_u0, #rent :rent_u0, #leaseHolders :tenant_u0 DELETE #tags :tags_u0, #contact :contact_u0",
"ExpressionAttributeNames": {
"#category": "category",
"#tenant": "tenant",
"#rent": "rent",
"#deposit": "deposit",
"#leaseEndDate": "leaseEndDate",
"#discount": "discount",
"#rentalAgreement": "rentalAgreement",
"#tags": "tags",
"#contact": "contact",
"#totalFees": "totalFees",
"#petFee": "petFee",
"#leaseHolders": "leaseHolders",
"#gsi2sk": "gsi2sk",
"#buildingId": "buildingId",
"#cityId": "cityId",
"#mallId": "mallId",
"#storeId": "storeId",
"#__edb_e__": "__edb_e__", "#__edb_v__": "__edb_v__",

},
"ExpressionAttributeValues": {
":buildingId_u0": "A34",
Expand Down Expand Up @@ -1219,7 +1217,7 @@ describe("Update Item", () => {
op.set(attr.category, "food/meal");
op.add(attr.tenant, newTenant);
op.subtract(attr.deposit, 200);
op.remove(attr.leaseEndDate);
op.remove(attr.discount);
op.append(attr.rentalAgreement, [{type: "amendment", detail: "absolutely no soup for you"}]);
op.delete(attr.tags, 'coffee');
op.del(attr.contact, '555-345-2222');
Expand Down
57 changes: 47 additions & 10 deletions test/ts_connected.complex.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1179,6 +1179,18 @@ describe("Simple Crud On Complex Entity", () => {
throw err;
}
}).to.throw(`Attribute "map.test" is Read-Only and cannot be updated`);
expect( () => {
try {
entity.update({stringVal, stringVal2})
.data((attr, op) =>
// @ts-ignore
op.set(attr.map.test, 'abc')
)
.params();
} catch(err) {
throw err;
}
}).to.throw(`Attribute "map.test" is Read-Only and cannot be updated`);
});

it("should apply readOnly constraints to nested attributes under a list", () => {
Expand Down Expand Up @@ -1234,6 +1246,19 @@ describe("Simple Crud On Complex Entity", () => {
throw err;
}
}).to.throw(`Attribute "list[*].test" is Read-Only and cannot be updated`);
expect( () => {
try {
entity.update({stringVal, stringVal2})

.data((attr, op) =>
// @ts-ignore
op.set(attr.list[3].test, 'def')
)
.params();
} catch(err) {
throw err;
}
}).to.throw(`Attribute "list[*].test" is Read-Only and cannot be updated`);
});

it("should apply not null style removal constraints to required nested attributes", () => {
Expand Down Expand Up @@ -1276,16 +1301,16 @@ describe("Simple Crud On Complex Entity", () => {
}, {table, client});
const stringVal = uuid();
const stringVal2 = uuid();
expect( () => {
try {
entity.update({stringVal, stringVal2})
// @ts-ignore
.remove(["map.test"])
.params();
} catch(err) {
throw err;
}
}).to.throw(`Attribute "map.test" is Required and cannot be removed`);
expect( () => entity.update({stringVal, stringVal2})
// @ts-ignore
.remove(["map.test"])
.params()
).to.throw(`Attribute "map.test" is Required and cannot be removed`);
expect(() => entity.update({stringVal, stringVal2})
.data((attr, op) =>
op.remove(attr.map.test))
.params()
).to.throw(`Attribute "map.test" is Required and cannot be removed`);
});

it("should apply not null style removal constraints to required nested attributes under a list", () => {
Expand Down Expand Up @@ -1341,6 +1366,18 @@ describe("Simple Crud On Complex Entity", () => {
throw err;
}
}).to.throw(`Attribute "list[*].test" is Required and cannot be removed`);
expect( () => {
try {
entity.update({stringVal, stringVal2})
// @ts-ignore
.data((attr, op) =>
op.remove(attr.list[3].test)
)
.params();
} catch(err) {
throw err;
}
}).to.throw(`Attribute "list[*].test" is Required and cannot be removed`);
});

it("should be able to create DynamoDB compatible Set objects without the client", async () => {
Expand Down
22 changes: 16 additions & 6 deletions test/ts_connected.update.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1391,7 +1391,7 @@ describe("Update Item", () => {
op.add(attr.tenant, newTenant);
op.add(attr.rent, 100);
op.subtract(attr.deposit, 200);
op.remove(attr.leaseEndDate);
op.remove(attr.discount);
op.append(attr.rentalAgreement, [{type: "ammendment", detail: "no soup for you"}]);
op.delete(attr.tags, ['coffee']);
op.del(attr.contact, ['555-345-2222']);
Expand All @@ -1402,20 +1402,19 @@ describe("Update Item", () => {
.params()

expect(JSON.parse(JSON.stringify(allParameters))).to.deep.equal({
"UpdateExpression": "SET #category = :category_u0, #deposit = #deposit - :deposit_u0, #rentalAgreement = list_append(#rentalAgreement, :rentalAgreement_u0), #totalFees = #totalFees + #petFee, #cityId = :cityId_u0, #mallId = :mallId_u0, #buildingId = :buildingId_u0, #storeId = :storeId_u0, #__edb_e__ = :__edb_e___u0, #__edb_v__ = :__edb_v___u0 REMOVE #leaseEndDate, #gsi2sk ADD #tenant :tenant_u0, #rent :rent_u0, #leaseHolders :tenant_u0 DELETE #tags :tags_u0, #contact :contact_u0",
"UpdateExpression": "SET #category = :category_u0, #deposit = #deposit - :deposit_u0, #rentalAgreement = list_append(#rentalAgreement, :rentalAgreement_u0), #totalFees = #totalFees + #petFee, #cityId = :cityId_u0, #mallId = :mallId_u0, #buildingId = :buildingId_u0, #storeId = :storeId_u0, #__edb_e__ = :__edb_e___u0, #__edb_v__ = :__edb_v___u0 REMOVE #discount ADD #tenant :tenant_u0, #rent :rent_u0, #leaseHolders :tenant_u0 DELETE #tags :tags_u0, #contact :contact_u0",
"ExpressionAttributeNames": {
"#category": "category",
"#tenant": "tenant",
"#rent": "rent",
"#deposit": "deposit",
"#leaseEndDate": "leaseEndDate",
"#discount": "discount",
"#rentalAgreement": "rentalAgreement",
"#tags": "tags",
"#contact": "contact",
"#totalFees": "totalFees",
"#petFee": "petFee",
"#leaseHolders": "leaseHolders",
"#gsi2sk": "gsi2sk",
"#buildingId": "buildingId",
"#cityId": "cityId",
"#mallId": "mallId",
Expand Down Expand Up @@ -2160,13 +2159,24 @@ describe("Update Item", () => {

const removal = ["createdAt"] as any;

const error = await repositories
const removeError = await repositories
.update({repoName, repoOwner})
.remove(removal)
.go()
.catch(err => err);

expect(error.message).to.equal(`Attribute "createdAt" is Read-Only and cannot be removed - For more detail on this error reference: https://github.com/tywalch/electrodb#invalid-attribute`);
expect(removeError.message).to.equal(`Attribute "createdAt" is Read-Only and cannot be removed - For more detail on this error reference: https://github.com/tywalch/electrodb#invalid-attribute`);

const dataRemoveError = await repositories
.update({repoName, repoOwner})
.data((attr, op) =>
// @ts-ignore
op.remove(attr.createdAt)
)
.go()
.catch(err => err);

expect(dataRemoveError.message).to.equal(`Attribute "createdAt" is Read-Only and cannot be updated - For more detail on this error reference: https://github.com/tywalch/electrodb#invalid-attribute`);
});

it("should remove properties from an item", async () => {
Expand Down

0 comments on commit 1e877ec

Please sign in to comment.