Skip to content

Commit

Permalink
Fixups and style
Browse files Browse the repository at this point in the history
* change 'data' state shape, nesting the tree fetcher data
* rename 'TreeFetcherParameters' from 'DatabaseParameters' to make it
more specific to the API it works on
* fix bug in 'equal' method of 'TreeFetcherParameters'`
* use mockTreeFetcherParameters method in tests that need to specify a
TreeFetcherParameters but when the value isn't relevant to the test
* Hide Resolver if there is no databaseDocumentID
* add doc comments
  • Loading branch information
oatkiller committed Sep 3, 2020
1 parent 1e37369 commit dcf5369
Show file tree
Hide file tree
Showing 17 changed files with 295 additions and 189 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { TreeFetcherParameters } from '../types';

/**
* A factory for the most basic `TreeFetcherParameters`. Many tests need to provide this even when the values aren't relevant to the test.
*/
export function mockTreeFetcherParameters(): TreeFetcherParameters {
return {
databaseDocumentID: '',
indices: [],
};
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { TreeFetcherParameters } from '../types';

import { equal } from './tree_fetcher_parameters';
describe('TreeFetcherParameters#equal:', () => {
const cases: Array<[TreeFetcherParameters, TreeFetcherParameters, boolean]> = [
// different databaseDocumentID
[{ databaseDocumentID: 'a', indices: [] }, { databaseDocumentID: 'b', indices: [] }, false],
// different indices length
[{ databaseDocumentID: 'a', indices: [''] }, { databaseDocumentID: 'a', indices: [] }, false],
// same indices length, different databaseDocumentID
[{ databaseDocumentID: 'a', indices: [''] }, { databaseDocumentID: 'b', indices: [''] }, false],
// 1 item in `indices`
[{ databaseDocumentID: 'b', indices: [''] }, { databaseDocumentID: 'b', indices: [''] }, true],
// 2 item in `indices`
[
{ databaseDocumentID: 'b', indices: ['1', '2'] },
{ databaseDocumentID: 'b', indices: ['1', '2'] },
true,
],
// 2 item in `indices`, but order inversed
[
{ databaseDocumentID: 'b', indices: ['2', '1'] },
{ databaseDocumentID: 'b', indices: ['1', '2'] },
true,
],
];
describe.each(cases)('%p when compared to %p', (first, second, expected) => {
it(`should ${expected ? '' : 'not'}be equal`, () => {
expect(equal(first, second)).toBe(expected);
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { TreeFetcherParameters } from '../types';

/**
* Determine if two instances of `TreeFetcherParameters` are equivalent. Use this to determine if
* a change to a `TreeFetcherParameters` warrants invaliding a request or response.
*/
export function equal(param1: TreeFetcherParameters, param2?: TreeFetcherParameters): boolean {
if (!param2) {
return false;
}
if (param1 === param2) {
return true;
}
if (param1.databaseDocumentID !== param2.databaseDocumentID) {
return false;
}
return arrayEqual(param1.indices, param2.indices);
}

function arrayEqual(first: unknown[], second: unknown[]): boolean {
if (first === second) {
return true;
}
if (first.length !== second.length) {
return false;
}
const firstSet = new Set(first);
for (let index = 0; index < second.length; index++) {
if (!firstSet.has(second[index])) {
return false;
}
}
return true;
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { ResolverRelatedEvents, ResolverTree } from '../../../../common/endpoint/types';
import { DatabaseParameters } from '../../types';
import { TreeFetcherParameters } from '../../types';

interface ServerReturnedResolverData {
readonly type: 'serverReturnedResolverData';
Expand All @@ -17,7 +17,7 @@ interface ServerReturnedResolverData {
/**
* The database parameters that was used to fetch the resolver tree
*/
parameters: DatabaseParameters;
parameters: TreeFetcherParameters;
};
}

Expand All @@ -26,23 +26,23 @@ interface AppRequestedResolverData {
/**
* entity ID used to make the request.
*/
readonly payload: DatabaseParameters;
readonly payload: TreeFetcherParameters;
}

interface ServerFailedToReturnResolverData {
readonly type: 'serverFailedToReturnResolverData';
/**
* entity ID used to make the failed request
*/
readonly payload: DatabaseParameters;
readonly payload: TreeFetcherParameters;
}

interface AppAbortedResolverDataRequest {
readonly type: 'appAbortedResolverDataRequest';
/**
* entity ID used to make the aborted request
*/
readonly payload: DatabaseParameters;
readonly payload: TreeFetcherParameters;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { DataState } from '../../types';
import { DataAction } from './action';
import { ResolverChildNode, ResolverTree } from '../../../../common/endpoint/types';
import * as eventModel from '../../../../common/endpoint/models/event';
import { mockTreeFetcherParameters } from '../../mocks/tree_fetcher_parameters';

/**
* Test the data reducer and selector.
Expand All @@ -27,7 +28,7 @@ describe('Resolver Data Middleware', () => {
type: 'serverReturnedResolverData',
payload: {
result: tree,
parameters: { databaseDocumentID: '', indices: [] },
parameters: mockTreeFetcherParameters(),
},
};
store.dispatch(action);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,41 +7,53 @@
import { Reducer } from 'redux';
import { DataState } from '../../types';
import { ResolverAction } from '../actions';
import * as databaseParameters from '../../models/database_parameters';
import * as treeFetcherParameters from '../../models/tree_fetcher_parameters';

const initialState: DataState = {
relatedEvents: new Map(),
relatedEventsReady: new Map(),
resolverComponentInstanceID: undefined,
tree: {},
};

export const dataReducer: Reducer<DataState, ResolverAction> = (state = initialState, action) => {
if (action.type === 'appReceivedNewExternalProperties') {
const nextState: DataState = {
...state,
currentParameters: {
databaseDocumentID: action.payload.databaseDocumentID,
indices: action.payload.indices,
tree: {
...state.tree,
currentParameters: {
databaseDocumentID: action.payload.databaseDocumentID,
indices: action.payload.indices,
},
},
resolverComponentInstanceID: action.payload.resolverComponentInstanceID,
};
return nextState;
} else if (action.type === 'appRequestedResolverData') {
// keep track of what we're requesting, this way we know when to request and when not to.
return {
const nextState: DataState = {
...state,
pendingRequestParameters: {
databaseDocumentID: action.payload.databaseDocumentID,
indices: action.payload.indices,
tree: {
...state.tree,
pendingRequestParameters: {
databaseDocumentID: action.payload.databaseDocumentID,
indices: action.payload.indices,
},
},
};
return nextState;
} else if (action.type === 'appAbortedResolverDataRequest') {
if (databaseParameters.equal(action.payload, state.pendingRequestParameters)) {
if (treeFetcherParameters.equal(action.payload, state.tree.pendingRequestParameters)) {
// the request we were awaiting was aborted
return {
const nextState: DataState = {
...state,
pendingRequestParameters: undefined,
tree: {
...state.tree,
pendingRequestParameters: undefined,
},
};
return nextState;
} else {
return state;
}
Expand All @@ -50,29 +62,35 @@ export const dataReducer: Reducer<DataState, ResolverAction> = (state = initialS
const nextState: DataState = {
...state,

/**
* Store the last received data, as well as the databaseDocumentID it relates to.
*/
lastResponse: {
result: action.payload.result,
parameters: action.payload.parameters,
successful: true,
},
tree: {
...state.tree,
/**
* Store the last received data, as well as the databaseDocumentID it relates to.
*/
lastResponse: {
result: action.payload.result,
parameters: action.payload.parameters,
successful: true,
},

// This assumes that if we just received something, there is no longer a pending request.
// This cannot model multiple in-flight requests
pendingRequestParameters: undefined,
// This assumes that if we just received something, there is no longer a pending request.
// This cannot model multiple in-flight requests
pendingRequestParameters: undefined,
},
};
return nextState;
} else if (action.type === 'serverFailedToReturnResolverData') {
/** Only handle this if we are expecting a response */
if (state.pendingRequestParameters !== undefined) {
if (state.tree.pendingRequestParameters !== undefined) {
const nextState: DataState = {
...state,
pendingRequestParameters: undefined,
lastResponse: {
parameters: state.pendingRequestParameters,
successful: false,
tree: {
...state.tree,
pendingRequestParameters: undefined,
lastResponse: {
parameters: state.tree.pendingRequestParameters,
successful: false,
},
},
};
return nextState;
Expand All @@ -83,16 +101,18 @@ export const dataReducer: Reducer<DataState, ResolverAction> = (state = initialS
action.type === 'userRequestedRelatedEventData' ||
action.type === 'appDetectedMissingEventData'
) {
return {
const nextState: DataState = {
...state,
relatedEventsReady: new Map([...state.relatedEventsReady, [action.payload, false]]),
};
return nextState;
} else if (action.type === 'serverReturnedRelatedEventData') {
return {
const nextState: DataState = {
...state,
relatedEventsReady: new Map([...state.relatedEventsReady, [action.payload.entityID, true]]),
relatedEvents: new Map([...state.relatedEvents, [action.payload.entityID, action.payload]]),
};
return nextState;
} else {
return state;
}
Expand Down
Loading

0 comments on commit dcf5369

Please sign in to comment.