Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Ignore events within the app that triggered them #2323

Open
wants to merge 2 commits into
base: fix/convert-v1-profiles
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/imperative/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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" });
Expand All @@ -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 () => {
Expand Down
7 changes: 6 additions & 1 deletion packages/imperative/src/config/src/ProfileInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down
1 change: 1 addition & 0 deletions packages/imperative/src/events/src/Event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[];

Expand Down
1 change: 1 addition & 0 deletions packages/imperative/src/events/src/EventProcessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion packages/imperative/src/events/src/EventUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Loading