Skip to content

Commit

Permalink
fix: comments from review
Browse files Browse the repository at this point in the history
  • Loading branch information
Victor Wiebe committed Jan 14, 2020
1 parent eca864d commit a47af52
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 20 deletions.
4 changes: 2 additions & 2 deletions src/procedures/AssignStoRole.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
StoRole,
} from '../types';
import { PolymathError } from '../PolymathError';
import { isValidBytes32CompliantString } from '~/utils';
import { checkStringLength } from '../utils';

export class AssignStoRole extends Procedure<AssignStoRoleProcedureArgs> {
public type = ProcedureType.AssignStoRole;
Expand Down Expand Up @@ -67,7 +67,7 @@ export class AssignStoRole extends Procedure<AssignStoRoleProcedureArgs> {
});
}
} else {
isValidBytes32CompliantString(description, 'description');
checkStringLength(description, 'description');
// Delegate not found. Add them here
await this.addTransaction(permissionModule.addDelegate, {
tag: PolyTransactionTag.ChangePermission,
Expand Down
4 changes: 2 additions & 2 deletions src/procedures/ModifyTieredStoData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
Currency,
} from '../types';
import { PolymathError } from '../PolymathError';
import { areSameAddress, isValidBytes32CompliantString } from '../utils';
import { areSameAddress, checkStringLength } from '../utils';
import { SecurityToken, TieredSto } from '../entities';
import { TieredStoFactory } from '../entities/factories';

Expand Down Expand Up @@ -219,7 +219,7 @@ export class ModifyTieredStoData extends Procedure<ModifyTieredStoDataProcedureA
polyOracleAddress = currentPolyOracleAddress;
}

isValidBytes32CompliantString(currencySymbol, 'denominated currency symbol');
checkStringLength(currencySymbol, 'denominated currency symbol');

if (
currencySymbol !== denominatedCurrency ||
Expand Down
6 changes: 3 additions & 3 deletions src/procedures/SetDocument.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Procedure } from './Procedure';
import { ProcedureType, PolyTransactionTag, SetDocumentProcedureArgs, ErrorCode } from '../types';
import { PolymathError } from '../PolymathError';
import { isNotEmptyValidBytes32CompliantString } from '~/utils';
import { checkStringLength } from '../utils';

export class SetDocument extends Procedure<SetDocumentProcedureArgs> {
public type = ProcedureType.SetDocument;
Expand Down Expand Up @@ -32,8 +32,8 @@ export class SetDocument extends Procedure<SetDocumentProcedureArgs> {
});
}

isNotEmptyValidBytes32CompliantString(name, 'name');
isNotEmptyValidBytes32CompliantString(documentHash, 'document hash');
checkStringLength(name, 'name', { minLength: 1, maxLength: 32 });
checkStringLength(documentHash, 'document hash', { minLength: 1, maxLength: 32 });

/*
* Transactions
Expand Down
47 changes: 46 additions & 1 deletion src/utils/__tests__/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { serialize, unserialize } from '../index';
import { checkStringLength, serialize, unserialize } from '../index';
import { PolymathError } from '../../PolymathError';
import { ErrorCode } from '../../types';

describe('serialize and unserialize', () => {
const entityType = 'someEntity';
Expand Down Expand Up @@ -31,3 +33,46 @@ describe('serialize and unserialize', () => {
expect(unserialize(serialize(entityType, inversePojo1))).toEqual(pojo1);
});
});

describe('check string length', () => {
const testVariableType = 'myVar';

test('checkStringLength used on a non empty string, catches error above the default 32 characters', () => {
try {
checkStringLength('0123456789012345678901234567890123456789', testVariableType);
} catch (error) {
expect(error).toEqual(
new PolymathError({
code: ErrorCode.ProcedureValidationError,
message: `You must provide a valid ${testVariableType} up to 32 characters long`,
})
);
}
});

test('checkStringLength used on an empty string, catches error due to not meeting custom requirement of 1 - 32 characters', () => {
try {
checkStringLength('', testVariableType, { minLength: 1, maxLength: 32 });
} catch (error) {
expect(error).toEqual(
new PolymathError({
code: ErrorCode.ProcedureValidationError,
message: `You must provide a valid ${testVariableType} between 1 and 32 characters long`,
})
);
}
});

test('checkStringLength used on a 6 character string, catches error due to not meeting custom requirement of 1 - 5 characters', () => {
try {
checkStringLength('0123456', testVariableType, { minLength: 1, maxLength: 5 });
} catch (error) {
expect(error).toEqual(
new PolymathError({
code: ErrorCode.ProcedureValidationError,
message: `You must provide a valid ${testVariableType} between 1 and 5 characters long`,
})
);
}
});
});
31 changes: 19 additions & 12 deletions src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,20 +50,27 @@ export function areSameAddress(a: string, b: string) {
return a.toUpperCase() === b.toUpperCase();
}

export function isValidBytes32CompliantString(value: string, variableName: string) {
if (value.length > 32) {
/**
* Check the length of a given string to ensure it meets correct bounds
* @param value - the string itself
* @param variableName - the name of the variable associated to the string itself
* @param opts - optional min and max length of the string. Defaults to a minimum of 0 (empty string) and a maximum of 32 characters
*/
export function checkStringLength(
value: string,
variableName: string,
opts?: { minLength: number | undefined; maxLength: number }
) {
const minLength = opts && opts.minLength != undefined ? opts.minLength : 0;
const maxLength = opts ? opts.maxLength : 32;
if (value.length < minLength || value.length > maxLength) {
throw new PolymathError({
code: ErrorCode.ProcedureValidationError,
message: `You must provide a valid ${variableName} up to 32 characters long`,
});
}
}

export function isNotEmptyValidBytes32CompliantString(value: string, variableName: string) {
if (value.length < 1 || value.length > 32) {
throw new PolymathError({
code: ErrorCode.ProcedureValidationError,
message: `You must provide a valid ${variableName} between 1 and 32 characters long`,
message: `You must provide a valid ${variableName} ${
opts && opts.minLength != undefined
? `between ${minLength} and ${maxLength}`
: `up to ${maxLength}`
} characters long`,
});
}
}
Expand Down

0 comments on commit a47af52

Please sign in to comment.