Skip to content

Commit

Permalink
Sm/better trim (#741)
Browse files Browse the repository at this point in the history
* feat: better typing and easier exports from sfdc

* test: more trimTo15 tests

* refactor: pr feedback
  • Loading branch information
mshanemc authored Jan 4, 2023
1 parent 2e8eb60 commit 3349e68
Show file tree
Hide file tree
Showing 11 changed files with 181 additions and 98 deletions.
4 changes: 2 additions & 2 deletions src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { Dictionary, ensure, isString, JsonPrimitive, Nullable } from '@salesfor
import { Global } from '../global';
import { Logger } from '../logger';
import { Messages } from '../messages';
import { sfdc } from '../util/sfdc';
import { validateApiVersion } from '../util/sfdc';
import { SfdcUrl } from '../util/sfdcUrl';
import { ORG_CONFIG_ALLOWED_PROPERTIES, OrgConfigProperties } from '../org/orgConfigProperties';
import { ConfigFile } from './configFile';
Expand Down Expand Up @@ -221,7 +221,7 @@ export const SFDX_ALLOWED_PROPERTIES = [
hidden: true,
input: {
// If a value is provided validate it otherwise no value is unset.
validator: (value: ConfigValue): boolean => value == null || (isString(value) && sfdc.validateApiVersion(value)),
validator: (value: ConfigValue): boolean => value == null || (isString(value) && validateApiVersion(value)),
failedMessage: messages.getMessage('invalidApiVersion'),
},
},
Expand Down
8 changes: 4 additions & 4 deletions src/org/authInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import { Config } from '../config/config';
import { ConfigAggregator } from '../config/configAggregator';
import { Logger } from '../logger';
import { SfError } from '../sfError';
import { sfdc } from '../util/sfdc';
import { matchesAccessToken, trimTo15 } from '../util/sfdc';
import { StateAggregator } from '../stateAggregator';
import { Messages } from '../messages';
import { getLoginAudienceCombos, SfdcUrl } from '../util/sfdcUrl';
Expand Down Expand Up @@ -443,7 +443,7 @@ export class AuthInfo extends AsyncOptionalCreatable<AuthInfo.Options> {
const devHubOrg = await Org.create({ aliasOrUsername: devHubUsername });
const conn = devHubOrg.getConnection();
const data = await conn.query<QueryResult<{ Id: string }>>(
`select Id from ScratchOrgInfo where ScratchOrg = '${sfdc.trimTo15(scratchOrgId)}'`
`select Id from ScratchOrgInfo where ScratchOrg = '${trimTo15(scratchOrgId)}'`
);
return data;
}
Expand Down Expand Up @@ -494,7 +494,7 @@ export class AuthInfo extends AsyncOptionalCreatable<AuthInfo.Options> {
this.update(authData);
const username = ensure(this.getUsername());

if (sfdc.matchesAccessToken(username)) {
if (matchesAccessToken(username)) {
this.logger.debug('Username is an accesstoken. Skip saving authinfo to disk.');
return this;
}
Expand Down Expand Up @@ -711,7 +711,7 @@ export class AuthInfo extends AsyncOptionalCreatable<AuthInfo.Options> {
} // Else it will be set in initAuthOptions below.

// If the username is an access token, use that for auth and don't persist
if (isString(oauthUsername) && sfdc.matchesAccessToken(oauthUsername)) {
if (isString(oauthUsername) && matchesAccessToken(oauthUsername)) {
// Need to initAuthOptions the logger and authInfoCrypto since we don't call init()
this.logger = await Logger.child('AuthInfo');

Expand Down
4 changes: 2 additions & 2 deletions src/org/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { MyDomainResolver } from '../status/myDomainResolver';
import { ConfigAggregator } from '../config/configAggregator';
import { Logger } from '../logger';
import { SfError } from '../sfError';
import { sfdc } from '../util/sfdc';
import { validateApiVersion } from '../util/sfdc';
import { Messages } from '../messages';
import { Lifecycle } from '../lifecycleEvents';
import { AuthFields, AuthInfo } from './authInfo';
Expand Down Expand Up @@ -320,7 +320,7 @@ export class Connection<S extends Schema = Schema> extends JSForceConnection<S>
* @param version The API version.
*/
public setApiVersion(version: string): void {
if (!sfdc.validateApiVersion(version)) {
if (!validateApiVersion(version)) {
throw messages.createError('incorrectAPIVersionError', [version]);
}
this.version = version;
Expand Down
6 changes: 3 additions & 3 deletions src/org/org.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import { Global } from '../global';
import { Lifecycle } from '../lifecycleEvents';
import { Logger } from '../logger';
import { SfError } from '../sfError';
import { sfdc } from '../util/sfdc';
import { trimTo15 } from '../util/sfdc';
import { WebOAuthServer } from '../webOAuthServer';
import { Messages } from '../messages';
import { StateAggregator } from '../stateAggregator';
Expand Down Expand Up @@ -434,7 +434,7 @@ export class Org extends AsyncOptionalCreatable<Org.Options> {

const thisOrgAuthConfig = this.getConnection().getAuthInfoFields();

const trimmedId: string = sfdc.trimTo15(thisOrgAuthConfig.orgId) as string;
const trimmedId = trimTo15(thisOrgAuthConfig.orgId);

const DEV_HUB_SOQL = `SELECT CreatedDate,Edition,ExpirationDate FROM ActiveScratchOrg WHERE ScratchOrg='${trimmedId}'`;

Expand Down Expand Up @@ -1092,7 +1092,7 @@ export class Org extends AsyncOptionalCreatable<Org.Options> {
}
} catch {
// if an error is thrown, don't panic yet. we'll try querying by orgId
const trimmedId = sfdc.trimTo15(this.getOrgId()) as string;
const trimmedId = trimTo15(this.getOrgId());
this.logger.debug(`defaulting to trimming id from ${this.getOrgId()} to ${trimmedId}`);
try {
result = await this.queryProduction(prodOrg, 'SandboxOrganization', trimmedId);
Expand Down
4 changes: 2 additions & 2 deletions src/org/orgConfigProperties.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { isString } from '@salesforce/ts-types';
import { ConfigValue } from '../config/configStore';
import { Messages } from '../messages';
import { SfdcUrl } from '../util/sfdcUrl';
import { sfdc } from '../util/sfdc';
import { validateApiVersion } from '../util/sfdc';

Messages.importMessagesDirectory(pathJoin(__dirname));
const messages = Messages.load('@salesforce/core', 'config', [
Expand Down Expand Up @@ -102,7 +102,7 @@ export const ORG_CONFIG_ALLOWED_PROPERTIES = [
hidden: true,
input: {
// If a value is provided validate it otherwise no value is unset.
validator: (value: ConfigValue): boolean => value == null || (isString(value) && sfdc.validateApiVersion(value)),
validator: (value: ConfigValue): boolean => value == null || (isString(value) && validateApiVersion(value)),
failedMessage: messages.getMessage('invalidApiVersion'),
},
},
Expand Down
6 changes: 3 additions & 3 deletions src/org/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { Logger } from '../logger';
import { Messages } from '../messages';
import { SecureBuffer } from '../crypto/secureBuffer';
import { SfError } from '../sfError';
import { sfdc } from '../util/sfdc';
import { matchesAccessToken, validateSalesforceId } from '../util/sfdc';
import { Connection } from './connection';
import { PermissionSetAssignment } from './permissionSetAssignment';
import { Org } from './org';
Expand Down Expand Up @@ -94,7 +94,7 @@ async function retrieveUserFields(logger: Logger, username: string): Promise<Use
authInfo: await AuthInfo.create({ username }),
});

if (sfdc.matchesAccessToken(username)) {
if (matchesAccessToken(username)) {
logger.debug('received an accessToken for the username. Converting...');
username = (await connection.identity()).username;
logger.debug(`accessToken converted to ${username}`);
Expand Down Expand Up @@ -138,7 +138,7 @@ async function retrieveUserFields(logger: Logger, username: string): Promise<Use
* @param connection The connection for the query.
*/
async function retrieveProfileId(name: string, connection: Connection): Promise<string> {
if (!sfdc.validateSalesforceId(name)) {
if (!validateSalesforceId(name)) {
const profileQuery = `SELECT Id FROM Profile WHERE name='${name}'`;
const result = await connection.query<{ Id: string }>(profileQuery);
if (result.records.length > 0) {
Expand Down
4 changes: 2 additions & 2 deletions src/sfProject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { SchemaValidator } from './schema/validator';
import { resolveProjectPath, resolveProjectPathSync, SFDX_PROJECT_JSON } from './util/internal';

import { SfError } from './sfError';
import { sfdc } from './util/sfdc';
import { findUpperCaseKeys } from './util/sfdc';
import { Messages } from './messages';

Messages.importMessagesDirectory(__dirname);
Expand Down Expand Up @@ -392,7 +392,7 @@ export class SfProjectJson extends ConfigFile {

private validateKeys(): void {
// Verify that the configObject does not have upper case keys; throw if it does. Must be heads down camel case.
const upperCaseKey = sfdc.findUpperCaseKeys(this.toObject(), SfProjectJson.BLOCKLIST);
const upperCaseKey = findUpperCaseKeys(this.toObject(), SfProjectJson.BLOCKLIST);
if (upperCaseKey) {
throw coreMessages.createError('invalidJsonCasing', [upperCaseKey, this.getPath()]);
}
Expand Down
4 changes: 2 additions & 2 deletions src/util/checkLightningDomain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@
import { URL } from 'url';
import { Env, Duration } from '@salesforce/kit';
import { MyDomainResolver } from '../status/myDomainResolver';
import { sfdc } from './sfdc';
import { isInternalUrl } from './sfdc';

export default async function checkLightningDomain(url: string): Promise<boolean> {
const domain = `https://${/https?:\/\/([^.]*)/.exec(url)?.slice(1, 2).pop()}.lightning.force.com`;
const quantity = new Env().getNumber('SFDX_DOMAIN_RETRY', 240) ?? 0;
const timeout = new Duration(quantity, Duration.Unit.SECONDS);

if (sfdc.isInternalUrl(url) || timeout.seconds === 0) {
if (isInternalUrl(url) || timeout.seconds === 0) {
return true;
}

Expand Down
156 changes: 86 additions & 70 deletions src/util/sfdc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,82 +9,98 @@ import { findKey } from '@salesforce/kit';
import { AnyJson, asJsonMap, isJsonMap, JsonMap, Optional } from '@salesforce/ts-types';
import { SfdcUrl } from './sfdcUrl';

export const sfdc = {
/**
* Converts an 18 character Salesforce ID to 15 characters.
*
* @param id The id to convert.
*/
trimTo15: (id?: string): Optional<string> => {
if (id?.length && id.length > 15) {
id = id.substring(0, 15);
}
return id;
},
/**
* Converts an 18 character Salesforce ID to 15 characters.
*
* @param id The id to convert.
*/
export function trimTo15(id: string): string;
export function trimTo15(id?: undefined): undefined;
export function trimTo15(id: string | undefined): string | undefined;
export function trimTo15(id: string | undefined): string | undefined {
if (!id) {
return undefined;
}
if (id.length && id.length > 15) {
id = id.substring(0, 15);
}
return id;
}

/**
* Tests whether an API version matches the format `i.0`.
*
* @param value The API version as a string.
*/
validateApiVersion: (value: string): boolean => value == null || /^[1-9]\d\.0$/.test(value),
/**
* Tests whether an API version matches the format `i.0`.
*
* @param value The API version as a string.
*/
export const validateApiVersion = (value: string): boolean => value == null || /^[1-9]\d\.0$/.test(value);

/**
* Tests whether an email matches the format `[email protected]`
*
* @param value The email as a string.
*/
validateEmail: (value: string): boolean => /^[^.][^@]*@[^.]+(\.[^.\s]+)+$/.test(value),
/**
* Tests whether an email matches the format `[email protected]`
*
* @param value The email as a string.
*/
export const validateEmail = (value: string): boolean => /^[^.][^@]*@[^.]+(\.[^.\s]+)+$/.test(value);

/**
* Tests whether a given url is an internal Salesforce domain
*
* @param url
*/
export const isInternalUrl = (url: string): boolean => new SfdcUrl(url).isInternalUrl();

/**
* Tests whether a Salesforce ID is in the correct format, a 15- or 18-character length string with only letters and numbers
*
* @param value The ID as a string.
*/
validateSalesforceId: (value: string): boolean =>
/[a-zA-Z0-9]{18}|[a-zA-Z0-9]{15}/.test(value) && (value.length === 15 || value.length === 18),
/**
* Tests whether a Salesforce ID is in the correct format, a 15- or 18-character length string with only letters and numbers
*
* @param value The ID as a string.
*/
export const validateSalesforceId = (value: string): boolean =>
/[a-zA-Z0-9]{18}|[a-zA-Z0-9]{15}/.test(value) && (value.length === 15 || value.length === 18);

/**
* Tests whether a path is in the correct format; the value doesn't include the characters "[", "]", "?", "<", ">", "?", "|"
*
* @param value The path as a string.
*/
validatePathDoesNotContainInvalidChars: (value: string): boolean =>
// eslint-disable-next-line no-useless-escape
!/[\["\?<>\|\]]+/.test(value),
/**
* Returns the first key within the object that has an upper case first letter.
*
* @param data The object in which to check key casing.
* @param sectionBlocklist properties in the object to exclude from the search. e.g. a blocklist of `["a"]` and data of `{ "a": { "B" : "b"}}` would ignore `B` because it is in the object value under `a`.
*/
findUpperCaseKeys: (data?: JsonMap, sectionBlocklist: string[] = []): Optional<string> => {
let key: Optional<string>;
findKey(data, (val: AnyJson, k: string) => {
if (k.substr(0, 1) === k.substr(0, 1).toUpperCase()) {
key = k;
} else if (isJsonMap(val)) {
if (sectionBlocklist.includes(k)) {
return key;
}
key = sfdc.findUpperCaseKeys(asJsonMap(val));
/**
* Tests whether a path is in the correct format; the value doesn't include the characters "[", "]", "?", "<", ">", "?", "|"
*
* @param value The path as a string.
*/
export const validatePathDoesNotContainInvalidChars = (value: string): boolean =>
// eslint-disable-next-line no-useless-escape
!/[\["\?<>\|\]]+/.test(value);
/**
* Returns the first key within the object that has an upper case first letter.
*
* @param data The object in which to check key casing.
* @param sectionBlocklist properties in the object to exclude from the search. e.g. a blocklist of `["a"]` and data of `{ "a": { "B" : "b"}}` would ignore `B` because it is in the object value under `a`.
*/
export const findUpperCaseKeys = (data?: JsonMap, sectionBlocklist: string[] = []): Optional<string> => {
let key: Optional<string>;
findKey(data, (val: AnyJson, k: string) => {
if (/^[A-Z]/.test(k)) {
key = k;
} else if (isJsonMap(val)) {
if (sectionBlocklist.includes(k)) {
return key;
}
return key;
});
key = findUpperCaseKeys(asJsonMap(val));
}
return key;
},
});
return key;
};

/**
* Tests whether a given string is an access token
*
* @param value
*/
matchesAccessToken: (value: string): boolean => /^(00D\w{12,15})![.\w]*$/.test(value),
/**
* Tests whether a given string is an access token
*
* @param value
*/
export const matchesAccessToken = (value: string): boolean => /^(00D\w{12,15})![.\w]*$/.test(value);

/**
* Tests whether a given url is an internal Salesforce domain
*
* @param url
*/
isInternalUrl: (url: string): boolean => new SfdcUrl(url).isInternalUrl(),
/** @deprecated import the individual functions instead of the whole object */
export const sfdc = {
trimTo15,
validateApiVersion,
validateEmail,
isInternalUrl,
matchesAccessToken,
validateSalesforceId,
validatePathDoesNotContainInvalidChars,
findUpperCaseKeys,
};
4 changes: 2 additions & 2 deletions test/unit/org/scratchOrgSettingsGeneratorTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import * as path from 'path';
import * as sinon from 'sinon';
import { expect } from 'chai';
import { Org, Connection } from '../../../src/org';
import { sfdc } from '../../../src/util/sfdc';
import { validateApiVersion } from '../../../src/util/sfdc';
import { ZipWriter } from '../../../src/util/zipWriter';
import { ScratchOrgInfo } from '../../../src/org/scratchOrgTypes';
import SettingsGenerator, {
Expand Down Expand Up @@ -70,7 +70,7 @@ const fakeConnection = (
({
setApiVersion: (apiVersion: string) => {
expect(apiVersion).to.be.a('string').and.length.to.be.greaterThan(0);
expect(sfdc.validateApiVersion(apiVersion)).to.not.be.undefined;
expect(validateApiVersion(apiVersion)).to.not.be.undefined;
},
deploy: (zipInput: Buffer) => {
expect(zipInput).to.have.property('length').and.to.be.greaterThan(0);
Expand Down
Loading

0 comments on commit 3349e68

Please sign in to comment.