Skip to content

Commit

Permalink
Add expand limit to connection (#447)
Browse files Browse the repository at this point in the history
* Make onFormChange more type safe

* Create mapToConnection() function

* Make disabledFields more type safe

* Better error message from globalMockFetch

* Add node expansion limit

* Ensure limit passed in is followed

* Add constants for default values
  • Loading branch information
kmcginnes committed Jun 20, 2024
1 parent fedcd6a commit abc48b4
Show file tree
Hide file tree
Showing 19 changed files with 185 additions and 85 deletions.
2 changes: 2 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
(<https://github.com/aws/graph-explorer/pull/434>)
- Improved scrolling behavior in expand sidebar
(<https://github.com/aws/graph-explorer/pull/436>)
- Add node expansion limit per connection
(<https://github.com/aws/graph-explorer/pull/447>)

**Bug Fixes and Minor Changes**

Expand Down
6 changes: 5 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,8 @@ defaults, and their descriptions.
- `GRAPH_EXP_FETCH_REQUEST_TIMEOUT` - `240000` - Controls the timeout for the
fetch request. Measured in milliseconds (i.e. 240000 is 240 seconds or 4
minutes).
- `GRAPH_EXP_NODE_EXPANSION_LIMIT` - `None` - Controls the limit for node
counts and expansion queries.
- Conditionally Required:
- Required if `USING_PROXY_SERVER=True`
- `GRAPH_CONNECTION_URL` - `None` - See
Expand Down Expand Up @@ -216,7 +218,8 @@ attributes:
"GRAPH_EXP_HTTPS_CONNECTION": true,
"PROXY_SERVER_HTTPS_CONNECTION": true,
// Measured in milliseconds (i.e. 240000 is 240 seconds or 4 minutes)
"GRAPH_EXP_FETCH_REQUEST_TIMEOUT": 240000
"GRAPH_EXP_FETCH_REQUEST_TIMEOUT": 240000,
"GRAPH_EXP_NODE_EXPANSION_LIMIT": 500,
}
```

Expand Down Expand Up @@ -246,6 +249,7 @@ docker run -p 80:80 -p 443:443 \
--env SERVICE_TYPE=neptune-db \
--env PROXY_SERVER_HTTPS_CONNECTION=true \
--env GRAPH_EXP_FETCH_REQUEST_TIMEOUT=240000 \
--env GRAPH_EXP_NODE_EXPANSION_LIMIT=500 \
graph-explorer
```

Expand Down
Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
import neighborsCountTemplate from "./neighborsCountTemplate";

describe("Gremlin > neighborsCountTemplate", () => {
it("Should return a template for the given vertex id with default limit", () => {
it("Should return a template for the given vertex id", () => {
const template = neighborsCountTemplate({
vertexId: "12",
idType: "string",
});

expect(template).toBe(
'g.V("12").both().limit(500).dedup().group().by(label).by(count())'
'g.V("12").both().dedup().group().by(label).by(count())'
);
});

it("Should return a template for the given vertex id with default limit and number type", () => {
it("Should return a template for the given vertex id with number type", () => {
const template = neighborsCountTemplate({
vertexId: "12",
idType: "number",
});

expect(template).toBe(
"g.V(12L).both().limit(500).dedup().group().by(label).by(count())"
"g.V(12L).both().dedup().group().by(label).by(count())"
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type { NeighborsCountRequest } from "../../useGEFetchTypes";
*/
const neighborsCountTemplate = ({
vertexId,
limit = 500,
limit = 0,
idType,
}: NeighborsCountRequest) => {
let template = "";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ describe("Gremlin > oneHopTemplate", () => {

expect(template).toBe(
'g.V("12").project("vertices", "edges")' +
".by(both().dedup().range(0, 10).fold())" +
".by(both().dedup().fold())" +
".by(bothE().dedup().fold())"
);
});
Expand All @@ -22,7 +22,7 @@ describe("Gremlin > oneHopTemplate", () => {

expect(template).toBe(
'g.V(12L).project("vertices", "edges")' +
".by(both().dedup().range(0, 10).fold())" +
".by(both().dedup().fold())" +
".by(bothE().dedup().fold())"
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,10 @@ const oneHopTemplate = ({
filterByVertexTypes = [],
edgeTypes = [],
filterCriteria = [],
limit = 10,
limit = 0,
offset = 0,
}: Omit<NeighborsRequest, "vertexType">): string => {
const range = `.range(${offset}, ${offset + limit})`;
const range = limit > 0 ? `.range(${offset}, ${offset + limit})` : "";
let template = "";
if (idType === "number") {
template = `g.V(${vertexId}L)`;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import neighborsCountTemplate from "./neighborsCountTemplate";

describe("OpenCypher > neighborsCountTemplate", () => {
it("Should return a template for the given vertex id with default limit", () => {
it("Should return a template for the given vertex id", () => {
const template = neighborsCountTemplate({
vertexId: "12",
idType: "string",
});

expect(template).toBe(
'MATCH (v) -[e]- (t) WHERE ID(v) = "12" RETURN labels(t) AS vertexLabel, count(DISTINCT t) AS count LIMIT 500'
'MATCH (v) -[e]- (t) WHERE ID(v) = "12" RETURN labels(t) AS vertexLabel, count(DISTINCT t) AS count'
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import type { NeighborsCountRequest } from "../../useGEFetchTypes";
*/
const neighborsCountTemplate = ({
vertexId,
limit = 500,
limit = 0,
}: NeighborsCountRequest) => {
let template = "";
template = `MATCH (v) -[e]- (t) WHERE ID(v) = "${vertexId}" RETURN labels(t) AS vertexLabel, count(DISTINCT t) AS count`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ describe("OpenCypher > oneHopTemplate", () => {
});

expect(template).toBe(
'MATCH (v)-[e]-(tgt) WHERE ID(v) = "12" WITH collect(DISTINCT tgt)[..10] AS vObjects, collect({edge: e, sourceType: labels(v), targetType: labels(tgt)})[..10] AS eObjects RETURN vObjects, eObjects'
'MATCH (v)-[e]-(tgt) WHERE ID(v) = "12" WITH collect(DISTINCT tgt) AS vObjects, collect({edge: e, sourceType: labels(v), targetType: labels(tgt)}) AS eObjects RETURN vObjects, eObjects'
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ const oneHopTemplate = ({
filterByVertexTypes = [],
edgeTypes = [],
filterCriteria = [],
limit = 10,
limit = 0,
}: Omit<NeighborsRequest, "vertexType">): string => {
let template = `MATCH (v)`;

Expand Down Expand Up @@ -137,7 +137,9 @@ const oneHopTemplate = ({
template += `AND ${filterCriteriaTemplate} `;
}

template += `WITH collect(DISTINCT tgt)[..${limit}] AS vObjects, collect({edge: e, sourceType: labels(v), targetType: labels(tgt)})[..${limit}] AS eObjects RETURN vObjects, eObjects`;
const limitTemplate = limit > 0 ? `[..${limit}]` : "";

template += `WITH collect(DISTINCT tgt)${limitTemplate} AS vObjects, collect({edge: e, sourceType: labels(v), targetType: labels(tgt)})${limitTemplate} AS eObjects RETURN vObjects, eObjects`;

return template;
};
Expand Down
9 changes: 7 additions & 2 deletions packages/graph-explorer/src/connector/queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,19 @@ export type NeighborCountsQueryResponse = {
* @param explorer The service client to use for fetching the neighbors count.
* @returns The count of neighbors for the given node as a total and per type.
*/
export const neighborsCountQuery = (id: VertexId, explorer: Explorer | null) =>
export const neighborsCountQuery = (
id: VertexId,
limit: number | undefined,
explorer: Explorer | null
) =>
queryOptions({
queryKey: ["neighborsCount", id, explorer],
queryKey: ["neighborsCount", id, limit, explorer],
enabled: Boolean(explorer),
queryFn: async (): Promise<NeighborCountsQueryResponse | undefined> => {
const result = await explorer?.fetchNeighborsCount({
vertexId: id.toString(),
idType: typeofVertexId(id),
limit,
});

if (!result) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,7 @@ const oneHopNeighborsTemplate = ({
return filter;
};

const getLimit = () => {
if (limit === 0) {
return "";
}
return `LIMIT ${limit} OFFSET ${offset}`;
};
const limitTemplate = limit > 0 ? `LIMIT ${limit} OFFSET ${offset}` : "";

return `
SELECT ?subject ?pred ?value ?subjectClass ?pToSubject ?pFromSubject {
Expand All @@ -114,7 +109,7 @@ const oneHopNeighborsTemplate = ({
${getFilters()}
}
}
${getLimit()}
${limitTemplate}
}
FILTER(isLiteral(?value))
}
Expand Down
16 changes: 12 additions & 4 deletions packages/graph-explorer/src/connector/testUtils/globalMockFetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,26 @@ const RESPONSES_FILES_MAP: Record<string, string> = {
"7062d2e": `${GREMLIN}/edges-labels-and-counts.json`,
"35be2501": `${GREMLIN}/should-return-1-random-node.json`,
"54fa1494": `${GREMLIN}/should-return-airports-whose-code-matches-with-SFA.json`,
"77c63d2": `${GREMLIN}/should-return-all-neighbors-from-node-2018.json`,
e9e93b2: `${GREMLIN}/should-return-all-neighbors-from-node-2018.json`,
"37e14b1": `${GREMLIN}/should-return-all-neighbors-from-node-2018-counts.json`,
"308b4988": `${GREMLIN}/should-return-filtered-neighbors-from-node-2018.json`,
"5698a21a": `${GREMLIN}/should-return-filtered-neighbors-from-node-2018.json`,
"40a4690b": `${GREMLIN}/should-return-filtered-neighbors-from-node-2018-counts.json`,
"1614f686": `${GREMLIN}/should-return-neighbors-counts-for-node-123.json`,
"59bc2d43": `${GREMLIN}/should-return-neighbors-counts-for-node-123.json`,
};

const globalMockFetch = () => {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
global.fetch = jest.fn(async (url: string) => {
const response = await import(RESPONSES_FILES_MAP[shortHash(url)]);
const key = shortHash(url);
const filePath = RESPONSES_FILES_MAP[key];
if (!filePath) {
throw new Error(
`Failed to find a response file in the map for key '${key}'`,
{ cause: { url } }
);
}
const response = await import(filePath);
return Promise.resolve({
json: () => {
return Promise.resolve(response);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,14 @@ export type ConnectionConfig = {
awsRegion?: string;
/**
* Number of milliseconds before aborting a request.
* By default, 60 seconds.
* By default, undefined.
*/
fetchTimeoutMs?: number;
/**
* A default limit on the number of nodes that can be expanded in one query.
* By default, undefined.
*/
nodeExpansionLimit?: number;
};

export type Schema = {
Expand Down
6 changes: 4 additions & 2 deletions packages/graph-explorer/src/core/connector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@ import { mergedConfigurationSelector } from "./StateProvider/configuration";
import { selector } from "recoil";
import { equalSelector } from "../utils/recoilState";
import { env } from "../utils";
import { ConnectionConfig } from "./ConfigurationProvider";

/**
* Active connection where the value will only change when one of the
* properties we care about are changed.
*/
const activeConnectionSelector = equalSelector({
export const activeConnectionSelector = equalSelector({
key: "activeConnection",
get: ({ get }) => {
const config = get(mergedConfigurationSelector);
Expand All @@ -33,7 +34,8 @@ const activeConnectionSelector = equalSelector({
"awsAuthEnabled",
"awsRegion",
"fetchTimeoutMs",
] as const;
"nodeExpansionLimit",
] as (keyof ConnectionConfig)[];
return every(attrs, attr => isEqual(latest[attr] as string, prior[attr]));
},
});
Expand Down
18 changes: 14 additions & 4 deletions packages/graph-explorer/src/hooks/useExpandNode.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import type {
NeighborsRequest,
NeighborsResponse,
} from "../connector/useGEFetchTypes";
import { explorerSelector } from "../core/connector";
import { activeConnectionSelector, explorerSelector } from "../core/connector";
import useEntities from "./useEntities";
import { useRecoilValue } from "recoil";
import { useMutation, useQueries } from "@tanstack/react-query";
Expand Down Expand Up @@ -111,9 +111,16 @@ export function ExpandNodeProvider(props: PropsWithChildren) {
return () => clearNotification(notificationId);
}, [clearNotification, enqueueNotification, mutation.isPending]);

const connection = useRecoilValue(activeConnectionSelector);
const expandNode = useCallback(
(vertex: Vertex, filters?: ExpandNodeFilters) => {
const request: ExpandNodeRequest = { vertex, filters };
const request: ExpandNodeRequest = {
vertex,
filters: {
...filters,
limit: filters?.limit || connection?.nodeExpansionLimit,
},
};

// Only allow expansion if we are not busy with another expansion
if (mutation.isPending) {
Expand All @@ -131,7 +138,7 @@ export function ExpandNodeProvider(props: PropsWithChildren) {

mutation.mutate(request);
},
[enqueueNotification, mutation]
[connection?.nodeExpansionLimit, enqueueNotification, mutation]
);

const value: ExpandNodeContextType = {
Expand All @@ -153,13 +160,16 @@ export function ExpandNodeProvider(props: PropsWithChildren) {
*/
function useUpdateNodeCounts() {
const [entities, setEntities] = useEntities();
const connection = useRecoilValue(activeConnectionSelector);
const explorer = useRecoilValue(explorerSelector);
const { enqueueNotification, clearNotification } = useNotification();

const nodeIds = [...new Set(entities.nodes.map(n => n.data.id))];

const query = useQueries({
queries: nodeIds.map(id => neighborsCountQuery(id, explorer)),
queries: nodeIds.map(id =>
neighborsCountQuery(id, connection?.nodeExpansionLimit, explorer)
),
combine: results => {
// Combines data with existing node data and filters out undefined
const data = results
Expand Down
2 changes: 2 additions & 0 deletions packages/graph-explorer/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ const grabConfig = async (): Promise<RawConfiguration | undefined> => {
defaultConnectionData.GRAPH_EXP_SERVICE_TYPE || DEFAULT_SERVICE_TYPE,
fetchTimeoutMs:
defaultConnectionData.GRAPH_EXP_FETCH_REQUEST_TIMEOUT || 240000,
nodeExpansionLimit:
defaultConnectionData.GRAPH_EXP_NODE_EXPANSION_LIMIT,
},
};
} catch (error) {
Expand Down
Loading

0 comments on commit abc48b4

Please sign in to comment.