Skip to content

Commit

Permalink
fix!: remove deprecated aliasAccessor methods
Browse files Browse the repository at this point in the history
  • Loading branch information
mshanemc committed Apr 2, 2024
1 parent 929b03a commit 02896ec
Show file tree
Hide file tree
Showing 7 changed files with 16 additions and 87 deletions.
3 changes: 1 addition & 2 deletions src/org/authInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -769,8 +769,7 @@ export class AuthInfo extends AsyncOptionalCreatable<AuthInfo.Options> {
* @param alias alias to set
*/
public async setAlias(alias: string): Promise<void> {
this.stateAggregator.aliases.set(alias, this.getUsername());
await this.stateAggregator.aliases.write();
return this.stateAggregator.aliases.setAndSave(alias, this.getUsername());
}

/**
Expand Down
6 changes: 4 additions & 2 deletions src/org/authRemover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,9 @@ export class AuthRemover extends AsyncOptionalCreatable {
if (existingAliases.length === 0) return;

this.logger.debug(`Found these aliases to remove: ${existingAliases.join(',')}`);
existingAliases.forEach((alias) => this.stateAggregator.aliases.unset(alias));
await this.stateAggregator.aliases.write();
for (const alias of existingAliases) {
// eslint-disable-next-line no-await-in-loop
await this.stateAggregator.aliases.unsetAndSave(alias);
}
}
}
76 changes: 2 additions & 74 deletions src/stateAggregator/accessors/aliasAccessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@
import { join, dirname } from 'node:path';
import { homedir } from 'node:os';
import { readFile, writeFile, mkdir } from 'node:fs/promises';
import { writeFileSync, readFileSync } from 'node:fs';
import { lock, unlock, lockSync, unlockSync } from 'proper-lockfile';
import { lock, unlock } from 'proper-lockfile';

import { AsyncOptionalCreatable, ensureArray } from '@salesforce/kit';

Expand All @@ -18,7 +17,7 @@ import { Global } from '../../global';
import { AuthFields } from '../../org/authInfo';
import { ConfigContents } from '../../config/configStackTypes';
import { SfError } from '../../sfError';
import { lockRetryOptions, lockOptions } from '../../util/lockRetryOptions';
import { lockRetryOptions } from '../../util/lockRetryOptions';

export type Aliasable = string | Partial<AuthFields>;
export const DEFAULT_GROUP = 'orgs';
Expand Down Expand Up @@ -109,20 +108,6 @@ export class AliasAccessor extends AsyncOptionalCreatable {
return Array.from(this.aliasStore.entries()).find(([, value]) => value === usernameOrAlias)?.[0];
}

/**
* Set an alias for the given aliasable entity. Writes to the file
*
* @deprecated use setAndSave
* @param alias the alias you want to set
* @param entity the aliasable entity that's being aliased
*/
public set(alias: string, entity: Aliasable): void {
// get a very fresh copy to merge with to avoid conflicts
this.readFileToAliasStoreSync();
this.aliasStore.set(alias, getNameOf(entity));
this.saveAliasStoreToFileSync();
}

/**
* Set an alias for the given aliasable entity. Writes to the file
*
Expand All @@ -136,18 +121,6 @@ export class AliasAccessor extends AsyncOptionalCreatable {
return this.saveAliasStoreToFile();
}

/**
* Unset the given alias. Writes to the file
*
* @deprecated use unsetAndSave
*
*/
public unset(alias: string): void {
this.readFileToAliasStoreSync();
this.aliasStore.delete(alias);
this.saveAliasStoreToFileSync();
}

/**
* Unset the given alias(es). Writes to the file
*
Expand All @@ -158,20 +131,6 @@ export class AliasAccessor extends AsyncOptionalCreatable {
return this.saveAliasStoreToFile();
}

/**
* Unsets all the aliases for the given entity.
*
* @deprecated use unsetValuesAndSave
*
* @param entity the aliasable entity for which you want to unset all aliases
*/
public unsetAll(entity: Aliasable): void {
this.readFileToAliasStoreSync();
const aliases = this.getAll(entity);
aliases.forEach((a) => this.aliasStore.delete(a));
this.saveAliasStoreToFileSync();
}

/**
* Unset all the aliases for the given array of entity.
*
Expand All @@ -185,13 +144,6 @@ export class AliasAccessor extends AsyncOptionalCreatable {
return this.saveAliasStoreToFile();
}

/**
* @deprecated the set/unset methods now write to the file when called. Use (un)setAndSave instead of calling (un)set and then calling write()
*/
public async write(): Promise<ConfigContents<string>> {
return Promise.resolve(this.getAll());
}

/**
* Returns true if the provided alias exists
*
Expand Down Expand Up @@ -232,30 +184,6 @@ export class AliasAccessor extends AsyncOptionalCreatable {
await writeFile(this.fileLocation, aliasStoreToRawFileContents(this.aliasStore));
return unlockIfLocked(this.fileLocation);
}

/**
* @deprecated use the async version of this method instead
* provided for the legacy sync set/unset methods. */
private readFileToAliasStoreSync(): void {
// the file is guaranteed to exist because this init method ensures it
// put a lock in place. This method is only used by legacy set/unset methods.
lockSync(this.fileLocation, lockOptions);
this.aliasStore = fileContentsRawToAliasStore(readFileSync(this.fileLocation, 'utf-8'));
}

/**
* @deprecated use the async version of this method instead
* provided for the legacy sync set/unset methods */
private saveAliasStoreToFileSync(): void {
writeFileSync(this.fileLocation, aliasStoreToRawFileContents(this.aliasStore));
try {
unlockSync(this.fileLocation);
} catch (e) {
// ignore the error. If it wasn't locked, that's what we wanted
if (errorIsNotAcquired(e)) return;
throw e;
}
}
}

/**
Expand Down
4 changes: 2 additions & 2 deletions test/unit/org/authInfoTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1329,10 +1329,10 @@ describe('AuthInfo', () => {
const alias = 'MyAlias';

it('should set alias', async () => {
const aliasAccessorSpy = spyMethod($$.SANDBOX, AliasAccessor.prototype, 'set');
const aliasAccessorSpy = spyMethod($$.SANDBOX, AliasAccessor.prototype, 'setAndSave');
const authInfo = await AuthInfo.create({ username: testOrg.username });
await authInfo.setAlias(alias);
expect(aliasAccessorSpy.calledOnce).to.be.true;
expect(aliasAccessorSpy.callCount).to.equal(1);
expect(aliasAccessorSpy.firstCall.args).to.deep.equal([alias, testOrg.username]);
});
});
Expand Down
2 changes: 1 addition & 1 deletion test/unit/org/authRemoverTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ describe('AuthRemover', () => {

describe('unsetAliases', () => {
it('should unset aliases for provided username', async () => {
const aliasesSpy = spyMethod($$.SANDBOX, AliasAccessor.prototype, 'unset');
const aliasesSpy = spyMethod($$.SANDBOX, AliasAccessor.prototype, 'unsetAndSave');
const alias1 = 'MyAlias';
const alias2 = 'MyOtherAlias';
$$.stubAliases({ [alias1]: username, [alias2]: username });
Expand Down
4 changes: 2 additions & 2 deletions test/unit/org/orgTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ describe('Org Tests', () => {
const org = await createOrgViaAuthInfo();

const stateAggregator = await StateAggregator.getInstance();
stateAggregator.aliases.set('foo', testData.username);
await stateAggregator.aliases.setAndSave('foo', testData.username);

const user = stateAggregator.aliases.getUsername('foo');
expect(user).eq(testData.username);
Expand Down Expand Up @@ -642,7 +642,7 @@ describe('Org Tests', () => {
const org1Username = orgs[1].getUsername();
assert(org1Username);
const stateAggregator = await StateAggregator.getInstance();
stateAggregator.aliases.set('foo', org1Username);
await stateAggregator.aliases.setAndSave('foo', org1Username);
const user = stateAggregator.aliases.getUsername('foo');
expect(user).eq(org1Username);

Expand Down
8 changes: 4 additions & 4 deletions test/unit/stateAggregator/accessors/aliasAccessorTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,14 @@ describe('AliasAccessor', () => {
describe('set', () => {
it('should set an alias for a username', async () => {
const stateAggregator = await StateAggregator.getInstance();
stateAggregator.aliases.set('foobar', username1);
await stateAggregator.aliases.setAndSave('foobar', username1);
const aliases = stateAggregator.aliases.getAll(username1);
expect(aliases).to.include('foobar');
});

it('should set an alias for an org', async () => {
const stateAggregator = await StateAggregator.getInstance();
stateAggregator.aliases.set('foobar', await org.getConfig());
await stateAggregator.aliases.setAndSave('foobar', await org.getConfig());
const aliases = stateAggregator.aliases.getAll(org.username);
expect(aliases).to.include('foobar');
});
Expand Down Expand Up @@ -144,7 +144,7 @@ describe('AliasAccessor', () => {
describe('unsetAll', () => {
it('should unset all the aliases for a given username', async () => {
const stateAggregator = await StateAggregator.getInstance();
stateAggregator.aliases.unsetAll(username1);
await stateAggregator.aliases.unsetValuesAndSave([username1]);
const aliases = stateAggregator.aliases.getAll(username1);
expect(aliases).to.deep.equal([]);
});
Expand Down Expand Up @@ -184,7 +184,7 @@ describe('AliasAccessor', () => {
const stateAggregator = await StateAggregator.getInstance();
const testArray = Array(quantity).map((v, i) => i.toString());
await Promise.all(
testArray.map((i) => Promise.resolve(stateAggregator.aliases.set(i.toString(), i.toString())))
testArray.map((i) => Promise.resolve(stateAggregator.aliases.setAndSave(i.toString(), i.toString())))
);
testArray.forEach((i) => {
expect(stateAggregator.aliases.get(i)).to.equal(i);
Expand Down

3 comments on commit 02896ec

@svc-cli-bot
Copy link
Contributor

Choose a reason for hiding this comment

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

Logger Benchmarks - ubuntu-latest

Benchmark suite Current: 02896ec Previous: d0f2239 Ratio
Child logger creation 481603 ops/sec (±0.61%) 468664 ops/sec (±0.70%) 0.97
Logging a string on root logger 825780 ops/sec (±8.34%) 789526 ops/sec (±6.90%) 0.96
Logging an object on root logger 663192 ops/sec (±10.15%) 602368 ops/sec (±7.67%) 0.91
Logging an object with a message on root logger 25506 ops/sec (±184.31%) 8969 ops/sec (±203.81%) 0.35
Logging an object with a redacted prop on root logger 493935 ops/sec (±7.75%) 495148 ops/sec (±6.04%) 1.00
Logging a nested 3-level object on root logger 22591 ops/sec (±184.51%) 368162 ops/sec (±6.90%) 16.30

This comment was automatically generated by workflow using github-action-benchmark.

@svc-cli-bot
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Logger Benchmarks - ubuntu-latest'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 02896ec Previous: d0f2239 Ratio
Logging a nested 3-level object on root logger 22591 ops/sec (±184.51%) 368162 ops/sec (±6.90%) 16.30

This comment was automatically generated by workflow using github-action-benchmark.

@svc-cli-bot
Copy link
Contributor

Choose a reason for hiding this comment

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

Logger Benchmarks - windows-latest

Benchmark suite Current: 02896ec Previous: d0f2239 Ratio
Child logger creation 337268 ops/sec (±0.45%) 342302 ops/sec (±0.64%) 1.01
Logging a string on root logger 862776 ops/sec (±6.70%) 828857 ops/sec (±5.94%) 0.96
Logging an object on root logger 647075 ops/sec (±6.45%) 613209 ops/sec (±8.95%) 0.95
Logging an object with a message on root logger 21807 ops/sec (±184.20%) 4064 ops/sec (±215.26%) 0.19
Logging an object with a redacted prop on root logger 494972 ops/sec (±6.04%) 485028 ops/sec (±5.96%) 0.98
Logging a nested 3-level object on root logger 331099 ops/sec (±5.11%) 329493 ops/sec (±5.10%) 1.00

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.