Skip to content

Commit

Permalink
Make rule executors return state as an object property (#147891)
Browse files Browse the repository at this point in the history 
In this PR, I'm changing the return type of rule executors from `return
state;` to `return { state };`.

This change had to touch all rule type executors so they return `state`
as a key. In the future, the framework could accept more than `state` in
the object, like warnings as an example.

**Before:**
```
executor: async (...) {
  const state = {...};
  return state;
}
```

**After:**
```
executor: async (...) {
  const state = {...};
  return { state };
}
```

**Future:**
```
executor: async (...) {
  return {
    state: {...},
    warnings: [...],
    metrics: {...},
    ...
  };
}
```

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
@mikecote @kibanamachine
mikecote and kibanamachine authored Jan 12, 2023
1 parent 42cb6a2 commit a1923c5
Show file tree
Hide file tree
Showing 58 changed files with 388 additions and 177 deletions.
4 changes: 3 additions & 1 deletion x-pack/examples/alerting_example/server/alert_types/always_firing.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ export const alertType: RuleType<
});

return {
count,
state: {
count,
},
};
},
producer: ALERTING_EXAMPLE_APP_ID,
Expand Down
4 changes: 3 additions & 1 deletion x-pack/examples/alerting_example/server/alert_types/astros.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,9 @@ export const alertType: RuleType<
}

return {
peopleInSpace,
state: {
peopleInSpace,
},
};
},
producer: ALERTING_EXAMPLE_APP_ID,
Expand Down
4 changes: 3 additions & 1 deletion x-pack/plugins/alerting/server/authorization/alerting_authorization.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,9 @@ beforeEach(() => {
minimumLicenseRequired: 'basic',
isExportable: true,
recoveryActionGroup: RecoveredActionGroup,
async executor() {},
async executor() {
return { state: {} };
},
producer: 'myApp',
}));
features.getKibanaFeatures.mockReturnValue([
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/alerting/server/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export type {
AlertingApiRequestHandlerContext,
RuleParamsAndRefs,
GetSummarizedAlertsFnOpts,
ExecutorType,
} from './types';
export { RuleNotifyWhen } from '../common';
export { DEFAULT_MAX_EPHEMERAL_ACTIONS_PER_ALERT } from './config';
Expand Down
14 changes: 8 additions & 6 deletions x-pack/plugins/alerting/server/plugin.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,17 @@ const generateAlertingConfig = (): AlertingConfig => ({
},
});

const sampleRuleType: RuleType<never, never, never, never, never, 'default'> = {
const sampleRuleType: RuleType<never, never, {}, never, never, 'default'> = {
id: 'test',
name: 'test',
minimumLicenseRequired: 'basic',
isExportable: true,
actionGroups: [],
defaultActionGroupId: 'default',
producer: 'test',
async executor() {},
async executor() {
return { state: {} };
},
};

describe('Alerting Plugin', () => {
Expand Down Expand Up @@ -165,7 +167,7 @@ describe('Alerting Plugin', () => {
const ruleType = {
...sampleRuleType,
minimumLicenseRequired: 'basic',
} as RuleType<never, never, never, never, never, 'default', never>;
} as RuleType<never, never, {}, never, never, 'default', never>;
await setup.registerType(ruleType);
expect(ruleType.ruleTaskTimeout).toBe('5m');
});
Expand All @@ -175,7 +177,7 @@ describe('Alerting Plugin', () => {
...sampleRuleType,
minimumLicenseRequired: 'basic',
ruleTaskTimeout: '20h',
} as RuleType<never, never, never, never, never, 'default', never>;
} as RuleType<never, never, {}, never, never, 'default', never>;
await setup.registerType(ruleType);
expect(ruleType.ruleTaskTimeout).toBe('20h');
});
Expand All @@ -184,7 +186,7 @@ describe('Alerting Plugin', () => {
const ruleType = {
...sampleRuleType,
minimumLicenseRequired: 'basic',
} as RuleType<never, never, never, never, never, 'default', never>;
} as RuleType<never, never, {}, never, never, 'default', never>;
await setup.registerType(ruleType);
expect(ruleType.cancelAlertsOnRuleTimeout).toBe(true);
});
Expand All @@ -194,7 +196,7 @@ describe('Alerting Plugin', () => {
...sampleRuleType,
minimumLicenseRequired: 'basic',
cancelAlertsOnRuleTimeout: false,
} as RuleType<never, never, never, never, never, 'default', never>;
} as RuleType<never, never, {}, never, never, 'default', never>;
await setup.registerType(ruleType);
expect(ruleType.cancelAlertsOnRuleTimeout).toBe(false);
});
Expand Down
8 changes: 5 additions & 3 deletions x-pack/plugins/alerting/server/rule_type_registry.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -685,15 +685,17 @@ function ruleTypeWithVariables<ActionGroupIds extends string>(
id: ActionGroupIds,
context: string,
state: string
): RuleType<never, never, never, never, never, ActionGroupIds> {
const baseAlert: RuleType<never, never, never, never, never, ActionGroupIds> = {
): RuleType<never, never, {}, never, never, ActionGroupIds> {
const baseAlert: RuleType<never, never, {}, never, never, ActionGroupIds> = {
id,
name: `${id}-name`,
actionGroups: [],
defaultActionGroupId: id,
isExportable: true,
minimumLicenseRequired: 'basic',
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
};

Expand Down
4 changes: 3 additions & 1 deletion x-pack/plugins/alerting/server/rules_client/tests/bulk_delete.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,9 @@ describe('bulkDelete', () => {
minimumLicenseRequired: 'basic',
isExportable: true,
recoveryActionGroup: RecoveredActionGroup,
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
});
});
Expand Down
12 changes: 9 additions & 3 deletions x-pack/plugins/alerting/server/rules_client/tests/bulk_edit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,9 @@ describe('bulkEdit()', () => {
minimumLicenseRequired: 'basic',
isExportable: true,
recoveryActionGroup: RecoveredActionGroup,
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
});
});
Expand Down Expand Up @@ -1682,7 +1684,9 @@ describe('bulkEdit()', () => {
param1: schema.string(),
}),
},
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
});

Expand Down Expand Up @@ -1724,7 +1728,9 @@ describe('bulkEdit()', () => {
},
},
},
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
});

Expand Down
32 changes: 24 additions & 8 deletions x-pack/plugins/alerting/server/rules_client/tests/create.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1206,7 +1206,9 @@ describe('create()', () => {
defaultActionGroupId: 'default',
minimumLicenseRequired: 'basic',
isExportable: true,
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
useSavedObjectReferences: {
extractReferences: extractReferencesFn,
Expand Down Expand Up @@ -1377,7 +1379,9 @@ describe('create()', () => {
defaultActionGroupId: 'default',
minimumLicenseRequired: 'basic',
isExportable: true,
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
useSavedObjectReferences: {
extractReferences: extractReferencesFn,
Expand Down Expand Up @@ -2160,7 +2164,9 @@ describe('create()', () => {
},
minimumLicenseRequired: 'basic',
isExportable: true,
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
});
await expect(rulesClient.create({ data })).rejects.toThrowErrorMatchingInlineSnapshot(
Expand Down Expand Up @@ -2612,7 +2618,9 @@ describe('create()', () => {
defaultActionGroupId: 'default',
minimumLicenseRequired: 'basic',
isExportable: true,
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
useSavedObjectReferences: {
extractReferences: jest.fn(),
Expand Down Expand Up @@ -2678,7 +2686,9 @@ describe('create()', () => {
defaultActionGroupId: 'default',
minimumLicenseRequired: 'basic',
isExportable: true,
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
useSavedObjectReferences: {
extractReferences: jest.fn(),
Expand Down Expand Up @@ -2707,7 +2717,9 @@ describe('create()', () => {
defaultActionGroupId: 'default',
minimumLicenseRequired: 'basic',
isExportable: true,
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
useSavedObjectReferences: {
extractReferences: jest.fn(),
Expand Down Expand Up @@ -2794,7 +2806,9 @@ describe('create()', () => {
defaultActionGroupId: 'default',
minimumLicenseRequired: 'basic',
isExportable: true,
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
useSavedObjectReferences: {
extractReferences: jest.fn(),
Expand Down Expand Up @@ -2834,7 +2848,9 @@ describe('create()', () => {
defaultActionGroupId: 'default',
minimumLicenseRequired: 'basic',
isExportable: true,
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
useSavedObjectReferences: {
extractReferences: jest.fn(),
Expand Down
16 changes: 12 additions & 4 deletions x-pack/plugins/alerting/server/rules_client/tests/find.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,9 @@ describe('find()', () => {
defaultActionGroupId: 'default',
minimumLicenseRequired: 'basic',
isExportable: true,
async executor() {},
async executor() {
return { state: {} };
},
producer: 'myApp',
}));
ruleTypeRegistry.get.mockImplementationOnce(() => ({
Expand All @@ -367,7 +369,9 @@ describe('find()', () => {
defaultActionGroupId: 'default',
minimumLicenseRequired: 'basic',
isExportable: true,
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
useSavedObjectReferences: {
extractReferences: jest.fn(),
Expand Down Expand Up @@ -552,7 +556,9 @@ describe('find()', () => {
defaultActionGroupId: 'default',
minimumLicenseRequired: 'basic',
isExportable: true,
async executor() {},
async executor() {
return { state: {} };
},
producer: 'myApp',
}));
ruleTypeRegistry.get.mockImplementationOnce(() => ({
Expand All @@ -563,7 +569,9 @@ describe('find()', () => {
defaultActionGroupId: 'default',
minimumLicenseRequired: 'basic',
isExportable: true,
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
useSavedObjectReferences: {
extractReferences: jest.fn(),
Expand Down
8 changes: 6 additions & 2 deletions x-pack/plugins/alerting/server/rules_client/tests/get.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,9 @@ describe('get()', () => {
defaultActionGroupId: 'default',
minimumLicenseRequired: 'basic',
isExportable: true,
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
useSavedObjectReferences: {
extractReferences: jest.fn(),
Expand Down Expand Up @@ -335,7 +337,9 @@ describe('get()', () => {
defaultActionGroupId: 'default',
minimumLicenseRequired: 'basic',
isExportable: true,
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
useSavedObjectReferences: {
extractReferences: jest.fn(),
Expand Down
4 changes: 3 additions & 1 deletion x-pack/plugins/alerting/server/rules_client/tests/lib.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,9 @@ export function getBeforeSetup(
defaultActionGroupId: 'default',
minimumLicenseRequired: 'basic',
isExportable: true,
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
}));
rulesClientParams.getEventLogClient.mockResolvedValue(
Expand Down
8 changes: 6 additions & 2 deletions x-pack/plugins/alerting/server/rules_client/tests/resolve.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,9 @@ describe('resolve()', () => {
defaultActionGroupId: 'default',
minimumLicenseRequired: 'basic',
isExportable: true,
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
useSavedObjectReferences: {
extractReferences: jest.fn(),
Expand Down Expand Up @@ -397,7 +399,9 @@ describe('resolve()', () => {
defaultActionGroupId: 'default',
minimumLicenseRequired: 'basic',
isExportable: true,
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
useSavedObjectReferences: {
extractReferences: jest.fn(),
Expand Down
16 changes: 12 additions & 4 deletions x-pack/plugins/alerting/server/rules_client/tests/update.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,9 @@ describe('update()', () => {
minimumLicenseRequired: 'basic',
isExportable: true,
recoveryActionGroup: RecoveredActionGroup,
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
});
});
Expand Down Expand Up @@ -704,7 +706,9 @@ describe('update()', () => {
defaultActionGroupId: 'default',
minimumLicenseRequired: 'basic',
isExportable: true,
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
useSavedObjectReferences: {
extractReferences: extractReferencesFn,
Expand Down Expand Up @@ -1201,7 +1205,9 @@ describe('update()', () => {
param1: schema.string(),
}),
},
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
});
await expect(
Expand Down Expand Up @@ -1575,7 +1581,9 @@ describe('update()', () => {
minimumLicenseRequired: 'basic',
isExportable: true,
recoveryActionGroup: RecoveredActionGroup,
async executor() {},
async executor() {
return { state: {} };
},
producer: 'alerts',
});
encryptedSavedObjects.getDecryptedAsInternalUser.mockResolvedValueOnce({
Expand Down
Loading

0 comments on commit a1923c5

Please sign in to comment.