diff --git a/packages/imperative/CHANGELOG.md b/packages/imperative/CHANGELOG.md index b0f0a47903..c46f398945 100644 --- a/packages/imperative/CHANGELOG.md +++ b/packages/imperative/CHANGELOG.md @@ -8,6 +8,8 @@ All notable changes to the Imperative package will be documented in this file. - BugFix: Fixed an issue where the `ProfileInfo.onlyV1ProfilesExist` method could wrongly return true when V2 profiles exist. [#2311](https://github.com/zowe/zowe-cli/issues/2311) - Deprecated the static method `ProfileInfo.onlyV1ProfilesExist` and replaced it with an `onlyV1ProfilesExist` instance method on the `ProfileInfo` class. - BugFix: Fixed an issue where the `ConvertV1Profiles.convert` method may create team configuration files in the wrong directory if the environment variable `ZOWE_CLI_HOME` is set. [#2312](https://github.com/zowe/zowe-cli/issues/2312) +- BugFix: Fixed an issue where the Imperative Event Emitter would fire event callbacks for the same app that triggered the event. [#2279](https://github.com/zowe/zowe-cli/issues/2279) +- BugFix: Fixed an issue where the `ProfileInfo.updateKnownProperty` method could rewrite team config file to disk without any changes when storing secure value. [#2324](https://github.com/zowe/zowe-cli/issues/2324) ## `8.7.0` diff --git a/packages/imperative/src/config/__tests__/ProfileInfo.TeamConfig.unit.test.ts b/packages/imperative/src/config/__tests__/ProfileInfo.TeamConfig.unit.test.ts index 22bfa02d3b..df704dcb0f 100644 --- a/packages/imperative/src/config/__tests__/ProfileInfo.TeamConfig.unit.test.ts +++ b/packages/imperative/src/config/__tests__/ProfileInfo.TeamConfig.unit.test.ts @@ -1306,7 +1306,7 @@ describe("TeamConfig ProfileInfo tests", () => { }); describe("updateKnownProperty", () => { - it("should throw and error if the property location type is invalid", async () => { + it("should throw an error if the property location type is invalid", async () => { const profInfo = createNewProfInfo(teamProjDir); await profInfo.readProfilesFromDisk(); let caughtError; @@ -1345,6 +1345,7 @@ describe("TeamConfig ProfileInfo tests", () => { const profInfo = createNewProfInfo(teamProjDir); await profInfo.readProfilesFromDisk(); const jsonPathMatchesSpy = jest.spyOn(ConfigUtils, "jsonPathMatches"); + const configSaveSpy = jest.spyOn(profInfo.getTeamConfig(), "save"); const prof = profInfo.mergeArgsForProfile(profInfo.getAllProfiles("dummy")[0]); const ret = await profInfo.updateKnownProperty({ mergedArgs: prof, property: "host", value: "example.com" }); @@ -1353,6 +1354,26 @@ describe("TeamConfig ProfileInfo tests", () => { expect(newHost).toEqual("example.com"); expect(ret).toBe(true); expect(jsonPathMatchesSpy).toHaveBeenCalled(); // Verify that profile names are matched correctly + expect(configSaveSpy).toHaveBeenCalled(); + }); + + it("should update the given property in the vault and return true", async () => { + const profInfo = createNewProfInfo(teamProjDir); + await profInfo.readProfilesFromDisk(); + const jsonPathMatchesSpy = jest.spyOn(ConfigUtils, "jsonPathMatches"); + jest.spyOn(profInfo.getTeamConfig().api.secure, "secureFields").mockReturnValue(["profiles.LPAR4.properties.host"]); + const configSaveSpy = jest.spyOn(profInfo.getTeamConfig(), "save"); + const configSecureSaveSpy = jest.spyOn(profInfo.getTeamConfig().api.secure, "save"); + + const prof = profInfo.mergeArgsForProfile(profInfo.getAllProfiles("dummy")[0]); + const ret = await profInfo.updateKnownProperty({ mergedArgs: prof, property: "host", value: "example.com", setSecure: true }); + const newHost = profInfo.getTeamConfig().api.layers.get().properties.profiles.LPAR4.properties.host; + + expect(newHost).toEqual("example.com"); + expect(ret).toBe(true); + expect(jsonPathMatchesSpy).toHaveBeenCalled(); // Verify that profile names are matched correctly + expect(configSaveSpy).not.toHaveBeenCalled(); + expect(configSecureSaveSpy).toHaveBeenCalled(); }); it("should remove the given property if the value specified if undefined", async () => { diff --git a/packages/imperative/src/config/src/ProfileInfo.ts b/packages/imperative/src/config/src/ProfileInfo.ts index 4d718f7264..3331526be7 100644 --- a/packages/imperative/src/config/src/ProfileInfo.ts +++ b/packages/imperative/src/config/src/ProfileInfo.ts @@ -298,8 +298,13 @@ export class ProfileInfo { this.getTeamConfig().api.layers.activate(osLoc.user, osLoc.global); } + const updateVaultOnly = options.setSecure && this.getTeamConfig().api.secure.secureFields().includes(toUpdate.argLoc.jsonLoc); this.getTeamConfig().set(toUpdate.argLoc.jsonLoc, options.value, { secure: options.setSecure }); - await this.getTeamConfig().save(false); + if (!updateVaultOnly) { + await this.getTeamConfig().save(false); + } else { + await this.getTeamConfig().api.secure.save(false); + } if (oldLayer) { this.getTeamConfig().api.layers.activate(oldLayer.user, oldLayer.global); diff --git a/packages/imperative/src/events/__tests__/__unit__/EventUtils.unit.test.ts b/packages/imperative/src/events/__tests__/__unit__/EventUtils.unit.test.ts index 7967ecda59..e8d57eaa38 100644 --- a/packages/imperative/src/events/__tests__/__unit__/EventUtils.unit.test.ts +++ b/packages/imperative/src/events/__tests__/__unit__/EventUtils.unit.test.ts @@ -11,16 +11,30 @@ import { EventOperator } from '../../src/EventOperator'; import { EventProcessor } from '../../src/EventProcessor'; -import { Logger } from '../../..'; +import { EventUtils, Logger } from '../../..'; import { IProcessorTypes } from '../../src/doc'; import { Event } from '../../..'; import { EventTypes, ZoweUserEvents } from "../../src/EventConstants"; +import * as fs from "fs"; jest.mock('../../src/EventProcessor'); jest.mock('../../../logger'); const logger = Logger.getImperativeLogger(); +const createTestEvent = (appName: string) => ({ + eventTime: '', + eventName: 'testEvent', + eventType: EventTypes.SharedEvents, + appName, + subscriptions: [ + { + removeAllListeners: jest.fn(), + close: jest.fn() + } + ] +} as unknown as Event); + describe("EventOperator Unit Tests", () => { beforeEach(() => { jest.clearAllMocks(); @@ -69,18 +83,7 @@ describe("EventOperator Unit Tests", () => { const appName = 'DeleteApp'; const processor = new EventProcessor(appName, IProcessorTypes.BOTH); processor.subscribedEvents = new Map([ - ['testEvent', { - eventTime: '', - eventName: 'testEvent', - eventType: EventTypes.SharedEvents, - appName: appName, - subscriptions: new Set([ - { - removeAllListeners: jest.fn(), - close: jest.fn() - } - ]) - } as unknown as Event] + ['testEvent', createTestEvent(appName)] ]); EventOperator.deleteProcessor(appName); @@ -97,22 +100,85 @@ describe("EventOperator Unit Tests", () => { expect(EventProcessor).toHaveBeenCalledWith(appName, IProcessorTypes.WATCHER, logger); expect(processor).toBeInstanceOf(EventProcessor); }); + + it("'setupWatcher' should setup a watcher that handles events", () => { + const appName = "WatcherApp"; + const testEvent = createTestEvent(appName); + const processor = new EventProcessor(appName, IProcessorTypes.WATCHER); + processor.eventTimes = new Map(); + processor.logger = { debug: jest.fn() } as any; + processor.subscribedEvents = new Map([ + ['testEvent', testEvent] + ]); + + const createEventSpy = jest.spyOn(EventUtils, "createEvent").mockReturnValue(testEvent); + const getEventContentsSpy = jest.spyOn(EventUtils, "getEventContents").mockReturnValue(testEvent); + jest.spyOn(fs, "watch").mockImplementationOnce((_filename, listener: any) => listener()); + + const eventCb = jest.fn(); + EventUtils.createSubscription(processor, testEvent.eventName, EventTypes.SharedEvents); + EventUtils.setupWatcher(processor, testEvent.eventName, eventCb); + + expect(createEventSpy).toHaveBeenCalledTimes(1); + expect(getEventContentsSpy).toHaveBeenCalledTimes(1); + expect(eventCb).toHaveBeenCalled(); + }); + + it("'setupWatcher' should setup a watcher that ignores events with same time", () => { + const appName = "WatcherApp"; + const testEvent = createTestEvent(appName); + testEvent.eventTime = new Date().toISOString(); + const processor = new EventProcessor(appName, IProcessorTypes.WATCHER); + processor.eventTimes = new Map([ + [testEvent.eventName, testEvent.eventTime] + ]); + processor.logger = { debug: jest.fn() } as any; + processor.subscribedEvents = new Map([ + ['testEvent', testEvent] + ]); + + const createEventSpy = jest.spyOn(EventUtils, "createEvent").mockReturnValue(testEvent); + const getEventContentsSpy = jest.spyOn(EventUtils, "getEventContents").mockReturnValue(testEvent); + jest.spyOn(fs, "watch").mockImplementationOnce((_filename, listener: any) => listener()); + + const eventCb = jest.fn(); + EventUtils.createSubscription(processor, testEvent.eventName, EventTypes.SharedEvents); + EventUtils.setupWatcher(processor, testEvent.eventName, eventCb); + + expect(createEventSpy).toHaveBeenCalledTimes(1); + expect(getEventContentsSpy).toHaveBeenCalledTimes(1); + expect(eventCb).not.toHaveBeenCalled(); + }); + + it("'setupWatcher' should setup a watcher that ignores events with same PID", () => { + const appName = "WatcherApp"; + const testEvent = createTestEvent(appName); + testEvent.appProcId = process.pid; + const processor = new EventProcessor(appName, IProcessorTypes.WATCHER); + processor.eventTimes = new Map(); + processor.logger = { debug: jest.fn() } as any; + processor.subscribedEvents = new Map([ + ['testEvent', testEvent] + ]); + + const createEventSpy = jest.spyOn(EventUtils, "createEvent").mockReturnValue(testEvent); + const getEventContentsSpy = jest.spyOn(EventUtils, "getEventContents").mockReturnValue(testEvent); + jest.spyOn(fs, "watch").mockImplementationOnce((_filename, listener: any) => listener()); + + const eventCb = jest.fn(); + EventUtils.createSubscription(processor, testEvent.eventName, EventTypes.SharedEvents); + EventUtils.setupWatcher(processor, testEvent.eventName, eventCb); + + expect(createEventSpy).toHaveBeenCalledTimes(1); + expect(getEventContentsSpy).toHaveBeenCalledTimes(1); + expect(eventCb).not.toHaveBeenCalled(); + }); + it("'deleteWatcher' should remove the correct event processor", () => { const appName = 'DeleteWatcher'; const processor = new EventProcessor(appName, IProcessorTypes.WATCHER); processor.subscribedEvents = new Map([ - ['testEvent', { - eventTime: '', - eventName: 'testEvent', - eventType: EventTypes.UserEvents, - appName: appName, - subscriptions: new Set([ - { - removeAllListeners: jest.fn(), - close: jest.fn() - } - ]) - } as unknown as Event] + ['testEvent', createTestEvent(appName)] ]); EventOperator.deleteWatcher(appName); @@ -134,18 +200,7 @@ describe("EventOperator Unit Tests", () => { const appName = 'DeleteEmitter'; const processor = new EventProcessor(appName, IProcessorTypes.EMITTER); processor.subscribedEvents = new Map([ - ['testEvent', { - eventTime: '', - eventName: 'testEvent', - eventType: EventTypes.UserEvents, - appName: appName, - subscriptions: new Set([ - { - removeAllListeners: jest.fn(), - close: jest.fn() - } - ]) - } as unknown as Event] + ['testEvent', createTestEvent(appName)] ]); EventOperator.deleteEmitter(appName); diff --git a/packages/imperative/src/events/src/Event.ts b/packages/imperative/src/events/src/Event.ts index d316e215c9..81989809e5 100644 --- a/packages/imperative/src/events/src/Event.ts +++ b/packages/imperative/src/events/src/Event.ts @@ -22,6 +22,7 @@ export class Event implements IEventJson { public eventName: string; public eventType: EventTypes; public appName: string; + public appProcId: number; public eventFilePath: string; public subscriptions: FSWatcher[]; diff --git a/packages/imperative/src/events/src/EventProcessor.ts b/packages/imperative/src/events/src/EventProcessor.ts index f012da1f64..159ec429c6 100644 --- a/packages/imperative/src/events/src/EventProcessor.ts +++ b/packages/imperative/src/events/src/EventProcessor.ts @@ -122,6 +122,7 @@ export class EventProcessor { private emit(eventName: string) { try { const event = this.subscribedEvents.get(eventName) ?? EventUtils.createEvent(eventName, this.appName); + event.appProcId = process.pid; event.eventTime = new Date().toISOString(); EventUtils.writeEvent(event); } catch (err) { diff --git a/packages/imperative/src/events/src/EventUtils.ts b/packages/imperative/src/events/src/EventUtils.ts index 7bf5a8e4f4..f8b82116fe 100644 --- a/packages/imperative/src/events/src/EventUtils.ts +++ b/packages/imperative/src/events/src/EventUtils.ts @@ -205,7 +205,7 @@ export class EventUtils { // Accommodates for the delay between actual file change event and fsWatcher's perception //(Node.JS triggers this notification event 3 times) event.eventTime = EventUtils.getEventContents(event.eventFilePath).eventTime; - if (eeInst.eventTimes.get(eventName) !== event.eventTime) { + if (eeInst.eventTimes.get(eventName) !== event.eventTime && process.pid !== event.appProcId) { eeInst.logger.debug(`EventEmitter: Event "${trigger}" emitted: ${eventName}`); if (Array.isArray(callbacks)) { callbacks.forEach(cb => cb());