diff --git a/.gitignore b/.gitignore index 698bc9c5a..d4bb1b099 100644 --- a/.gitignore +++ b/.gitignore @@ -17,6 +17,7 @@ /test-jest/ node_modules/ /typings/ +/storage/ *.iml dev*.html diff --git a/CHANGELOG.md b/CHANGELOG.md index 337e455d9..c2e2bd439 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to ### Changed - Improve efficiency of the default date/time parsing methods. [#876](https://github.com/handsontable/hyperformula/issues/876) +- Improve efficiency of the operations on the dependency graph. [#876](https://github.com/handsontable/hyperformula/issues/876) ### Fixed diff --git a/src/BuildEngineFactory.ts b/src/BuildEngineFactory.ts index 9dc9cc3e6..e52407822 100644 --- a/src/BuildEngineFactory.ts +++ b/src/BuildEngineFactory.ts @@ -86,7 +86,7 @@ export class BuildEngineFactory { throw new SheetSizeLimitExceededError() } const sheetId = sheetMapping.addSheet(sheetName) - addressMapping.autoAddSheet(sheetId, sheet, boundaries) + addressMapping.autoAddSheet(sheetId, boundaries) } } diff --git a/src/DateTimeDefault.ts b/src/DateTimeDefault.ts index 6744d1058..769bcc81a 100644 --- a/src/DateTimeDefault.ts +++ b/src/DateTimeDefault.ts @@ -125,8 +125,8 @@ function defaultParseToTime(timeItems: string[], timeFormat: Maybe): May } const hours = ampm !== undefined - ? hoursParsed % 12 + (ampm ? 12 : 0) - : hoursParsed + ? hoursParsed % 12 + (ampm ? 12 : 0) + : hoursParsed return { hours, minutes, seconds } } diff --git a/src/DependencyGraph/AddressMapping/AddressMapping.ts b/src/DependencyGraph/AddressMapping/AddressMapping.ts index 3b39f90b7..9185e2400 100644 --- a/src/DependencyGraph/AddressMapping/AddressMapping.ts +++ b/src/DependencyGraph/AddressMapping/AddressMapping.ts @@ -61,7 +61,7 @@ export class AddressMapping { this.mapping.set(sheetId, strategy) } - public autoAddSheet(sheetId: number, sheet: Sheet, sheetBoundaries: SheetBoundaries) { + public autoAddSheet(sheetId: number, sheetBoundaries: SheetBoundaries) { const {height, width, fill} = sheetBoundaries const strategyConstructor = this.policy.call(fill) this.addSheet(sheetId, new strategyConstructor(width, height)) diff --git a/src/DependencyGraph/DependencyGraph.ts b/src/DependencyGraph/DependencyGraph.ts index 4e77f32ba..02fe477c3 100644 --- a/src/DependencyGraph/DependencyGraph.ts +++ b/src/DependencyGraph/DependencyGraph.ts @@ -41,10 +41,11 @@ import {AddressMapping} from './AddressMapping/AddressMapping' import {ArrayMapping} from './ArrayMapping' import {collectAddressesDependentToRange} from './collectAddressesDependentToRange' import {FormulaVertex} from './FormulaCellVertex' -import {DependencyQuery, Graph, TopSortResult} from './Graph' +import {DependencyQuery, Graph} from './Graph' import {RangeMapping} from './RangeMapping' import {SheetMapping} from './SheetMapping' import {RawAndParsedValue} from './ValueCellVertex' +import {TopSortResult} from './TopSort' export class DependencyGraph { public readonly graph: Graph @@ -85,7 +86,7 @@ export class DependencyGraph { const newVertex = FormulaVertex.fromAst(ast, address, size, this.lazilyTransformingAstService.version()) this.exchangeOrAddFormulaVertex(newVertex) this.processCellDependencies(dependencies, newVertex) - this.graph.markNodeAsSpecialRecentlyChanged(newVertex) + this.graph.markNodeAsDirty(newVertex) if (hasVolatileFunction) { this.markAsVolatile(newVertex) } @@ -100,7 +101,7 @@ export class DependencyGraph { const vertex = this.shrinkPossibleArrayAndGetCell(address) this.exchangeOrAddGraphNode(vertex, errorVertex) this.addressMapping.setCell(address, errorVertex) - this.graph.markNodeAsSpecialRecentlyChanged(errorVertex) + this.graph.markNodeAsDirty(errorVertex) this.correctInfiniteRangesDependency(address) return this.getAndClearContentChanges() } @@ -111,17 +112,18 @@ export class DependencyGraph { if (vertex instanceof ArrayVertex) { this.arrayMapping.removeArray(vertex.getRange()) } + if (vertex instanceof ValueCellVertex) { const oldValues = vertex.getValues() if (oldValues.rawValue !== value.rawValue) { vertex.setValues(value) - this.graph.markNodeAsSpecialRecentlyChanged(vertex) + this.graph.markNodeAsDirty(vertex) } } else { const newVertex = new ValueCellVertex(value.parsedValue, value.rawValue) this.exchangeOrAddGraphNode(vertex, newVertex) this.addressMapping.setCell(address, newVertex) - this.graph.markNodeAsSpecialRecentlyChanged(newVertex) + this.graph.markNodeAsDirty(newVertex) } this.correctInfiniteRangesDependency(address) @@ -141,7 +143,7 @@ export class DependencyGraph { this.removeVertex(emptyVertex) this.addressMapping.removeCell(address) } else { - this.graph.markNodeAsSpecialRecentlyChanged(emptyVertex) + this.graph.markNodeAsDirty(emptyVertex) this.addressMapping.setCell(address, emptyVertex) } } else { @@ -158,15 +160,17 @@ export class DependencyGraph { } } - public clearRecentlyChangedVertices() { - this.graph.clearSpecialNodesRecentlyChanged() + public clearDirtyVertices() { + this.graph.clearDirtyNodes() } - public verticesToRecompute() { - return new Set([...this.graph.specialNodesRecentlyChanged, ...this.volatileVertices()]) + public verticesToRecompute(): Vertex[] { + return this.graph.getDirtyAndVolatileNodes() } public processCellDependencies(cellDependencies: CellDependency[], endVertex: Vertex) { + const endVertexId = this.graph.getNodeId(endVertex)! + cellDependencies.forEach((dep: CellDependency) => { if (dep instanceof AbsoluteCellRange) { const range = dep @@ -177,18 +181,20 @@ export class DependencyGraph { this.rangeMapping.setRange(rangeVertex) } - this.graph.addNode(rangeVertex) + this.graph.addNodeAndReturnId(rangeVertex) + const rangeVertexId = this.graph.getNodeId(rangeVertex)! + if (!range.isFinite()) { - this.graph.markNodeAsInfiniteRange(rangeVertex) + this.graph.markNodeAsInfiniteRange(rangeVertexId) } const {smallerRangeVertex, restRange} = this.rangeMapping.findSmallerRange(range) if (smallerRangeVertex !== undefined) { - this.graph.addEdge(smallerRangeVertex, rangeVertex) + this.graph.addEdge(smallerRangeVertex, rangeVertexId) if (rangeVertex.bruteForce) { rangeVertex.bruteForce = false for (const cellFromRange of range.addresses(this)) { //if we ever switch heuristic to processing by sorted sizes, this would be unnecessary - this.graph.removeEdge(this.fetchCell(cellFromRange), rangeVertex) + this.graph.removeEdge(this.fetchCell(cellFromRange), rangeVertexId) } } } else { @@ -197,65 +203,76 @@ export class DependencyGraph { const array = this.arrayMapping.getArray(restRange) if (array !== undefined) { - this.graph.addEdge(array, rangeVertex) + this.graph.addEdge(array, rangeVertexId) } else { for (const cellFromRange of restRange.addresses(this)) { - this.graph.addEdge(this.fetchCellOrCreateEmpty(cellFromRange), rangeVertex) + const { vertex, id } = this.fetchCellOrCreateEmpty(cellFromRange) + this.graph.addEdge(id ?? vertex, rangeVertexId) } } - this.graph.addEdge(rangeVertex, endVertex) + this.graph.addEdge(rangeVertexId, endVertexId) if (range.isFinite()) { this.correctInfiniteRangesDependenciesByRangeVertex(rangeVertex) } } else if (dep instanceof NamedExpressionDependency) { const sheetOfVertex = (endVertex as FormulaCellVertex).getAddress(this.lazilyTransformingAstService).sheet - const namedExpressionVertex = this.fetchNamedExpressionVertex(dep.name, sheetOfVertex) - this.graph.addEdge(namedExpressionVertex, endVertex) + const { vertex, id } = this.fetchNamedExpressionVertex(dep.name, sheetOfVertex) + this.graph.addEdge(id ?? vertex, endVertexId) } else { - this.graph.addEdge(this.fetchCellOrCreateEmpty(dep), endVertex) + const { vertex, id } = this.fetchCellOrCreateEmpty(dep) + this.graph.addEdge(id ?? vertex, endVertexId) } }) } - public fetchNamedExpressionVertex(expressionName: string, sheetId: number): CellVertex { + public fetchNamedExpressionVertex(expressionName: string, sheetId: number): { vertex: CellVertex, id: Maybe} { const namedExpression = this.namedExpressions.namedExpressionOrPlaceholder(expressionName, sheetId) return this.fetchCellOrCreateEmpty(namedExpression.address) } public exchangeNode(addressFrom: SimpleCellAddress, addressTo: SimpleCellAddress) { - const vertexFrom = this.fetchCellOrCreateEmpty(addressFrom) - const vertexTo = this.fetchCellOrCreateEmpty(addressTo) + const vertexFrom = this.fetchCellOrCreateEmpty(addressFrom).vertex + const vertexTo = this.fetchCellOrCreateEmpty(addressTo).vertex this.addressMapping.removeCell(addressFrom) this.exchangeGraphNode(vertexFrom, vertexTo) } public correctInfiniteRangesDependency(address: SimpleCellAddress) { - let vertex: Maybe = undefined - for (const range of this.graph.infiniteRanges) { - const infiniteRangeVertex = (range as RangeVertex) - if (infiniteRangeVertex.range.addressInRange(address)) { - vertex = vertex ?? this.fetchCellOrCreateEmpty(address) - this.graph.addEdge(vertex, infiniteRangeVertex) - } + const relevantInfiniteRanges = (this.graph.getInfiniteRanges()) + .filter(({ node }) => (node as RangeVertex).range.addressInRange(address)) + + if (relevantInfiniteRanges.length <= 0) { + return } + + const { vertex, id: maybeVertexId } = this.fetchCellOrCreateEmpty(address) + const vertexId = maybeVertexId ?? this.graph.getNodeId(vertex)! + + relevantInfiniteRanges.forEach(({ id }) => { + this.graph.addEdge(vertexId, id) + }) } - public fetchCellOrCreateEmpty(address: SimpleCellAddress): CellVertex { - let vertex = this.addressMapping.getCell(address) - if (vertex === undefined) { - vertex = new EmptyCellVertex() - this.graph.addNode(vertex) - this.addressMapping.setCell(address, vertex) + public fetchCellOrCreateEmpty(address: SimpleCellAddress): { vertex: CellVertex, id: Maybe } { + const existingVertex = this.addressMapping.getCell(address) + + if (existingVertex !== undefined) { + return { vertex: existingVertex, id: undefined } } - return vertex + + const newVertex = new EmptyCellVertex() + const newVertexId = this.graph.addNodeAndReturnId(newVertex) + this.addressMapping.setCell(address, newVertex) + + return { vertex: newVertex, id: newVertexId } } public removeRows(removedRows: RowsSpan): EagerChangesGraphChangeResult { this.stats.measure(StatType.ADJUSTING_GRAPH, () => { for (const [address, vertex] of this.addressMapping.entriesFromRowsSpan(removedRows)) { for (const adjacentNode of this.graph.adjacentNodes(vertex)) { - this.graph.markNodeAsSpecialRecentlyChanged(adjacentNode) + this.graph.markNodeAsDirty(adjacentNode) } if (vertex instanceof ArrayVertex) { if (vertex.isLeftCorner(address)) { @@ -295,7 +312,7 @@ export class DependencyGraph { for (const [adr, vertex] of this.addressMapping.sheetEntries(removedSheetId)) { for (const adjacentNode of this.graph.adjacentNodes(vertex)) { - this.graph.markNodeAsSpecialRecentlyChanged(adjacentNode) + this.graph.markNodeAsDirty(adjacentNode) } this.removeVertex(vertex) this.addressMapping.removeCell(adr) @@ -334,7 +351,7 @@ export class DependencyGraph { this.stats.measure(StatType.ADJUSTING_GRAPH, () => { for (const [address, vertex] of this.addressMapping.entriesFromColumnsSpan(removedColumns)) { for (const adjacentNode of this.graph.adjacentNodes(vertex)) { - this.graph.markNodeAsSpecialRecentlyChanged(adjacentNode) + this.graph.markNodeAsDirty(adjacentNode) } if (vertex instanceof ArrayVertex) { if (vertex.isLeftCorner(address)) { @@ -385,7 +402,7 @@ export class DependencyGraph { }) for (const vertex of this.addressMapping.verticesFromRowsSpan(addedRows)) { - this.graph.markNodeAsSpecialRecentlyChanged(vertex) + this.graph.markNodeAsDirty(vertex) } this.addStructuralNodesToChangeSet() @@ -409,7 +426,7 @@ export class DependencyGraph { }) for (const vertex of this.addressMapping.verticesFromColumnsSpan(addedColumns)) { - this.graph.markNodeAsSpecialRecentlyChanged(vertex) + this.graph.markNodeAsDirty(vertex) } this.addStructuralNodesToChangeSet() @@ -446,18 +463,18 @@ export class DependencyGraph { this.addressMapping.removeCell(sourceAddress) if (sourceVertex !== undefined) { - this.graph.markNodeAsSpecialRecentlyChanged(sourceVertex) + this.graph.markNodeAsDirty(sourceVertex) this.addressMapping.setCell(targetAddress, sourceVertex) let emptyVertex = undefined for (const adjacentNode of this.graph.adjacentNodes(sourceVertex)) { if (adjacentNode instanceof RangeVertex && !sourceRange.containsRange(adjacentNode.range)) { - emptyVertex = emptyVertex ?? this.fetchCellOrCreateEmpty(sourceAddress) + emptyVertex = emptyVertex ?? this.fetchCellOrCreateEmpty(sourceAddress).vertex this.graph.addEdge(emptyVertex, adjacentNode) this.graph.removeEdge(sourceVertex, adjacentNode) } } if (emptyVertex) { - this.graph.markNodeAsSpecialRecentlyChanged(emptyVertex) + this.graph.markNodeAsDirty(emptyVertex) this.addressMapping.setCell(sourceAddress, emptyVertex) } } @@ -467,9 +484,9 @@ export class DependencyGraph { this.addressMapping.removeCell(targetAddress) } for (const adjacentNode of this.graph.adjacentNodes(targetVertex)) { - sourceVertex = sourceVertex ?? this.fetchCellOrCreateEmpty(targetAddress) + sourceVertex = sourceVertex ?? this.fetchCellOrCreateEmpty(targetAddress).vertex this.graph.addEdge(sourceVertex, adjacentNode) - this.graph.markNodeAsSpecialRecentlyChanged(sourceVertex) + this.graph.markNodeAsDirty(sourceVertex) } this.removeVertex(targetVertex) } @@ -481,10 +498,10 @@ export class DependencyGraph { this.graph.removeEdge(rangeVertex, adjacentNode) for (const address of rangeVertex.range.addresses(this)) { - const newEmptyVertex = this.fetchCellOrCreateEmpty(address) - this.graph.addEdge(newEmptyVertex, adjacentNode) - this.addressMapping.setCell(address, newEmptyVertex) - this.graph.markNodeAsSpecialRecentlyChanged(newEmptyVertex) + const { vertex, id } = this.fetchCellOrCreateEmpty(address) + this.graph.addEdge(id ?? vertex, adjacentNode) + this.addressMapping.setCell(address, vertex) + this.graph.markNodeAsDirty(vertex) } } } @@ -504,11 +521,11 @@ export class DependencyGraph { for (const adjacentNode of adjacentNodes.values()) { const nodeDependencies = collectAddressesDependentToRange(this.functionRegistry, adjacentNode, arrayVertex.getRange(), this.lazilyTransformingAstService, this) for (const address of nodeDependencies) { - const vertex = this.fetchCellOrCreateEmpty(address) - this.graph.addEdge(vertex, adjacentNode) + const { vertex, id } = this.fetchCellOrCreateEmpty(address) + this.graph.addEdge(id ?? vertex, adjacentNode) } if (nodeDependencies.length > 0) { - this.graph.markNodeAsSpecialRecentlyChanged(adjacentNode) + this.graph.markNodeAsDirty(adjacentNode) } } @@ -517,17 +534,17 @@ export class DependencyGraph { } public addVertex(address: SimpleCellAddress, vertex: CellVertex): void { - this.graph.addNode(vertex) + this.graph.addNodeAndReturnId(vertex) this.addressMapping.setCell(address, vertex) } public addArrayVertex(address: SimpleCellAddress, vertex: ArrayVertex): void { - this.graph.addNode(vertex) + this.graph.addNodeAndReturnId(vertex) this.setAddressMappingForArrayVertex(vertex, address) } public* arrayFormulaNodes(): IterableIterator { - for (const vertex of this.graph.nodes) { + for (const vertex of this.graph.getNodes()) { if (vertex instanceof ArrayVertex) { yield vertex } @@ -603,25 +620,21 @@ export class DependencyGraph { } public markAsVolatile(vertex: Vertex) { - this.graph.markNodeAsSpecial(vertex) + this.graph.markNodeAsVolatile(vertex) } - public markAsDependentOnStructureChange(vertex: Vertex) { + public markAsDependentOnStructureChange(vertex: Vertex): void { this.graph.markNodeAsChangingWithStructure(vertex) } public forceApplyPostponedTransformations() { - for (const vertex of this.graph.nodes.values()) { + for (const vertex of this.graph.getNodes()) { if (vertex instanceof FormulaCellVertex) { vertex.ensureRecentData(this.lazilyTransformingAstService) } } } - public volatileVertices() { - return this.graph.specialNodes - } - public getArrayVerticesRelatedToRanges(ranges: RangeVertex[]): Set { const arrayVertices = ranges.map(range => { if (this.graph.hasNode(range)) { @@ -649,7 +662,7 @@ export class DependencyGraph { } public exchangeGraphNode(oldNode: Vertex, newNode: Vertex) { - this.graph.addNode(newNode) + this.graph.addNodeAndReturnId(newNode) const adjNodesStored = this.graph.adjacentNodes(oldNode) this.removeVertex(oldNode) adjNodesStored.forEach((adjacentNode) => { @@ -663,7 +676,7 @@ export class DependencyGraph { if (oldNode) { this.exchangeGraphNode(oldNode, newNode) } else { - this.graph.addNode(newNode) + this.graph.addNodeAndReturnId(newNode) } } @@ -736,13 +749,13 @@ export class DependencyGraph { dependentToCorner = true } this.graph.addEdge(vertex, adjacentVertex) - this.graph.markNodeAsSpecialRecentlyChanged(vertex) + this.graph.markNodeAsDirty(vertex) } if (!dependentToCorner) { this.graph.removeEdge(array, adjacentVertex) } } - this.graph.markNodeAsSpecialRecentlyChanged(array) + this.graph.markNodeAsDirty(array) } public isArrayInternalCell(address: SimpleCellAddress): boolean { @@ -771,16 +784,19 @@ export class DependencyGraph { } private correctInfiniteRangesDependenciesByRangeVertex(vertex: RangeVertex) { - for (const range of this.graph.infiniteRanges) { - const infiniteRangeVertex = (range as RangeVertex) - const intersection = vertex.range.intersectionWith(infiniteRangeVertex.range) - if (intersection === undefined) { - continue - } - for (const address of intersection.addresses(this)) { - this.graph.addEdge(this.fetchCellOrCreateEmpty(address), range) - } - } + this.graph.getInfiniteRanges() + .forEach(({ id: infiniteRangeVertexId, node: infiniteRangeVertex }) => { + const intersection = vertex.range.intersectionWith((infiniteRangeVertex as RangeVertex).range) + + if (intersection === undefined) { + return + } + + intersection.addresses(this).forEach((address: SimpleCellAddress) => { + const { vertex, id } = this.fetchCellOrCreateEmpty(address) + this.graph.addEdge(id ?? vertex, infiniteRangeVertexId) + }) + }) } private cleanAddressMappingUnderArray(vertex: ArrayVertex) { @@ -810,7 +826,7 @@ export class DependencyGraph { continue } if (array.getRange().addressInRange(dependency)) { - const vertex = this.fetchCellOrCreateEmpty(dependency) + const vertex = this.fetchCellOrCreateEmpty(dependency).vertex yield [dependency, vertex] } } @@ -820,7 +836,7 @@ export class DependencyGraph { const {restRange: range} = this.rangeMapping.findSmallerRange(vertex.range) for (const address of range.addresses(this)) { if (array.getRange().addressInRange(address)) { - const cell = this.fetchCellOrCreateEmpty(address) + const cell = this.fetchCellOrCreateEmpty(address).vertex yield [address, cell] } } @@ -867,10 +883,8 @@ export class DependencyGraph { return [address, absolutizeDependencies(deps, address)] } - private addStructuralNodesToChangeSet() { - for (const vertex of this.graph.specialNodesStructuralChanges) { - this.graph.markNodeAsSpecialRecentlyChanged(vertex) - } + private addStructuralNodesToChangeSet(): void { + this.graph.markChangingWithStructureNodesAsDirty() } private fixRangesWhenAddingRows(sheet: number, row: number, numberOfRows: number): void { @@ -880,7 +894,8 @@ export class DependencyGraph { if (rangeVertex.bruteForce) { const addedSubrangeInThatRange = rangeVertex.range.rangeWithSameWidth(row, numberOfRows) for (const address of addedSubrangeInThatRange.addresses(this)) { - this.graph.addEdge(this.fetchCellOrCreateEmpty(address), rangeVertex) + const { vertex, id } = this.fetchCellOrCreateEmpty(address) + this.graph.addEdge(id ?? vertex, rangeVertex) } } else { let currentRangeVertex = rangeVertex @@ -891,7 +906,7 @@ export class DependencyGraph { while (find.smallerRangeVertex === undefined) { const newRangeVertex = new RangeVertex(AbsoluteCellRange.spanFrom(currentRangeVertex.range.start, currentRangeVertex.range.width(), currentRangeVertex.range.height() - 1)) this.rangeMapping.setRange(newRangeVertex) - this.graph.addNode(newRangeVertex) + this.graph.addNodeAndReturnId(newRangeVertex) const restRange = new AbsoluteCellRange(simpleCellAddress(currentRangeVertex.range.start.sheet, currentRangeVertex.range.start.col, currentRangeVertex.range.end.row), currentRangeVertex.range.end) this.addAllFromRange(restRange, currentRangeVertex) this.graph.addEdge(newRangeVertex, currentRangeVertex) @@ -906,9 +921,10 @@ export class DependencyGraph { } } - private addAllFromRange(range: AbsoluteCellRange, vertex: RangeVertex) { + private addAllFromRange(range: AbsoluteCellRange, rangeVertex: RangeVertex) { for (const address of range.addresses(this)) { - this.graph.addEdge(this.fetchCellOrCreateEmpty(address), vertex) + const { vertex, id } = this.fetchCellOrCreateEmpty(address) + this.graph.addEdge(id ?? vertex, rangeVertex) } } @@ -922,7 +938,8 @@ export class DependencyGraph { subrange = AbsoluteCellRange.spanFrom(simpleCellAddress(sheet, column, rangeVertex.range.end.row), numberOfColumns, 1) } for (const address of subrange.addresses(this)) { - this.graph.addEdge(this.fetchCellOrCreateEmpty(address), rangeVertex) + const { vertex, id } = this.fetchCellOrCreateEmpty(address) + this.graph.addEdge(id ?? vertex, rangeVertex) } } } @@ -1090,7 +1107,7 @@ export class DependencyGraph { const adjNodesStored = this.graph.adjacentNodes(newVertex) this.removeVertexAndCleanupDependencies(newVertex) - this.graph.softRemoveEdge(existingVertex, newVertex) + this.graph.removeEdgeIfExists(existingVertex, newVertex) adjNodesStored.forEach((adjacentNode) => { if (this.graph.hasNode(adjacentNode)) { this.graph.addEdge(existingVertex, adjacentNode) diff --git a/src/DependencyGraph/Graph.ts b/src/DependencyGraph/Graph.ts index e52f495bb..5141f28fa 100644 --- a/src/DependencyGraph/Graph.ts +++ b/src/DependencyGraph/Graph.ts @@ -5,43 +5,130 @@ import {SimpleCellAddress} from '../Cell' import {SimpleCellRange} from '../AbsoluteCellRange' +import {TopSort, TopSortResult} from './TopSort' +import {ProcessableValue} from './ProcessableValue' -export type DependencyQuery = (vertex: T) => [(SimpleCellAddress | SimpleCellRange), T][] - -export interface TopSortResult { - sorted: T[], - cycled: T[], -} - -enum NodeVisitStatus { - ON_STACK, - PROCESSED, - POPPED, -} +export type NodeId = number +export type NodeAndId = { node: Node, id: NodeId } +export type DependencyQuery = (vertex: Node) => [(SimpleCellAddress | SimpleCellRange), Node][] /** - * Provides graph directed structure + * Provides directed graph structure. * - * Invariants: - * - this.edges(node) exists if and only if node is in the graph - * - this.specialNodes* are always subset of this.nodes - * - this.edges(node) is subset of this.nodes (i.e. it does not contain nodes not present in graph) -- this invariant DOES NOT HOLD right now + * Idea for performance improvement: + * - use Set[] instead of NodeId[][] for edgesSparseArray */ -export class Graph { - /** Set with nodes in graph. */ - public nodes: Set = new Set() +export class Graph { + /** + * A sparse array. The value nodesSparseArray[n] exists if and only if node n is in the graph. + * @private + */ + private nodesSparseArray: Node[] = [] - public specialNodes: Set = new Set() - public specialNodesStructuralChanges: Set = new Set() - public specialNodesRecentlyChanged: Set = new Set() - public infiniteRanges: Set = new Set() + /** + * A sparse array. The value edgesSparseArray[n] exists if and only if node n is in the graph. + * The edgesSparseArray[n] is also a sparse array. It may contain removed nodes. To make sure check nodesSparseArray. + * @private + */ + private edgesSparseArray: NodeId[][] = [] + + /** + * A mapping from node to its id. The value nodesIds.get(node) exists if and only if node is in the graph. + * @private + */ + private nodesIds: Map = new Map() + + /** + * A ProcessableValue object. + * @private + */ + private dirtyAndVolatileNodeIds = new ProcessableValue<{ dirty: NodeId[], volatile: NodeId[] }, Node[]>( + { dirty: [], volatile: [] }, + r => this.processDirtyAndVolatileNodeIds(r), + ) + + /** + * A set of node ids. The value infiniteRangeIds.get(nodeId) exists if and only if node is in the graph. + * @private + */ + private infiniteRangeIds: Set = new Set() + + /** + * A dense array. It may contain duplicates and removed nodes. + * @private + */ + private changingWithStructureNodeIds: NodeId[] = [] - /** Nodes adjacency mapping. */ - private edges: Map> = new Map() + private nextId: NodeId = 0 constructor( - private readonly dependencyQuery: DependencyQuery - ) { + private readonly dependencyQuery: DependencyQuery + ) {} + + /** + * Iterate over all nodes the in graph + */ + public getNodes(): Node[] { + return this.nodesSparseArray.filter((node: Node) => node !== undefined) + } + + /** + * Checks whether a node is present in graph + * + * @param node - node to check + */ + public hasNode(node: Node): boolean { + return this.nodesIds.has(node) + } + + /** + * Checks whether exists edge between nodes. If one or both of nodes are not present in graph, returns false. + * + * @param fromNode - node from which edge is outcoming + * @param toNode - node to which edge is incoming + */ + public existsEdge(fromNode: Node, toNode: Node): boolean { + const fromId = this.getNodeId(fromNode) + const toId = this.getNodeId(toNode) + + if (fromId === undefined || toId === undefined) { + return false + } + + return this.edgesSparseArray[fromId].includes(toId) + } + + /** + * Returns nodes adjacent to given node. May contain removed nodes. + * + * @param node - node to which adjacent nodes we want to retrieve + * + * Idea for performance improvement: + * - return an array instead of set + */ + public adjacentNodes(node: Node): Set { + const id = this.getNodeId(node) + + if (id === undefined) { + throw this.missingNodeError(node) + } + + return new Set(this.edgesSparseArray[id].filter(id => id !== undefined).map(id => this.nodesSparseArray[id])) + } + + /** + * Returns number of nodes adjacent to given node. Contrary to adjacentNodes(), this method returns only nodes that are present in graph. + * + * @param node - node to which adjacent nodes we want to retrieve + */ + public adjacentNodesCount(node: Node): number { + const id = this.getNodeId(node) + + if (id === undefined) { + throw this.missingNodeError(node) + } + + return this.fixEdgesArrayForNode(id).length } /** @@ -49,11 +136,20 @@ export class Graph { * * @param node - a node to be added */ - public addNode(node: T) { - this.nodes.add(node) - if (!this.edges.has(node)) { - this.edges.set(node, new Set()) + public addNodeAndReturnId(node: Node): NodeId { + const idOfExistingNode = this.nodesIds.get(node) + + if (idOfExistingNode !== undefined) { + return idOfExistingNode } + + const newId = this.nextId + this.nextId++ + + this.nodesSparseArray[newId] = node + this.edgesSparseArray[newId] = [] + this.nodesIds.set(node, newId) + return newId } /** @@ -64,260 +160,260 @@ export class Graph { * @param fromNode - node from which edge is outcoming * @param toNode - node to which edge is incoming */ - public addEdge(fromNode: T, toNode: T) { - if (!this.nodes.has(fromNode)) { - throw new Error(`Unknown node ${fromNode}`) + public addEdge(fromNode: Node | NodeId, toNode: Node | NodeId): void { + const fromId = this.getNodeIdIfNotNumber(fromNode) + const toId = this.getNodeIdIfNotNumber(toNode) + + if (fromId === undefined) { + throw this.missingNodeError(fromNode as Node) + } + + if (toId === undefined) { + throw this.missingNodeError(toNode as Node) } - if (!this.nodes.has(toNode)) { - throw new Error(`Unknown node ${toNode}`) + + if (this.edgesSparseArray[fromId].includes(toId)) { + return } - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - this.edges.get(fromNode)!.add(toNode) + + this.edgesSparseArray[fromId].push(toId) } - public removeEdge(fromNode: T, toNode: T) { - if (this.existsEdge(fromNode, toNode)) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - this.edges.get(fromNode)!.delete(toNode) - } else { + /** + * Removes node from graph + */ + public removeNode(node: Node): [(SimpleCellAddress | SimpleCellRange), Node][] { + const id = this.getNodeId(node) + + if (id === undefined) { + throw this.missingNodeError(node) + } + + if (this.edgesSparseArray[id].length > 0) { + this.edgesSparseArray[id].forEach(adjacentId => this.dirtyAndVolatileNodeIds.rawValue.dirty.push(adjacentId)) + this.dirtyAndVolatileNodeIds.markAsModified() + } + + const dependencies = this.removeDependencies(node) + + delete this.nodesSparseArray[id] + delete this.edgesSparseArray[id] + this.infiniteRangeIds.delete(id) + this.nodesIds.delete(node) + + return dependencies + } + + /** + * Removes edge between nodes. + */ + public removeEdge(fromNode: Node | NodeId, toNode: Node | NodeId): void { + const fromId = this.getNodeIdIfNotNumber(fromNode) + const toId = this.getNodeIdIfNotNumber(toNode) + + if (fromId === undefined) { + throw this.missingNodeError(fromNode as Node) + } + + if (toId === undefined) { + throw this.missingNodeError(toNode as Node) + } + + const indexOfToId = this.edgesSparseArray[fromId].indexOf(toId) + + if (indexOfToId === -1) { throw new Error('Edge does not exist') } + + delete this.edgesSparseArray[fromId][indexOfToId] } - public softRemoveEdge(fromNode: T, toNode: T) { - this.edges.get(fromNode)?.delete(toNode) + /** + * Removes edge between nodes if it exists. + */ + public removeEdgeIfExists(fromNode: Node, toNode: Node): void { + const fromId = this.getNodeId(fromNode) + const toId = this.getNodeId(toNode) + + if (fromId === undefined) { + return + } + + if (toId === undefined) { + return + } + + const indexOfToId = this.edgesSparseArray[fromId].indexOf(toId) + + if (indexOfToId === -1) { + return + } + + delete this.edgesSparseArray[fromId][indexOfToId] } - public removeIncomingEdges(toNode: T) { - this.edges.forEach((nodeEdges) => { - nodeEdges.delete(toNode) - }) + /** + * Sorts the whole graph topologically. Nodes that are on cycles are kept separate. + */ + public topSortWithScc(): TopSortResult { + return this.getTopSortedWithSccSubgraphFrom(this.getNodes(), () => true, () => {}) } /** - * Returns nodes adjacent to given node + * Sorts the graph topologically. Nodes that are on cycles are kept separate. * - * @param node - node to which adjacent nodes we want to retrieve + * @param modifiedNodes - seed for computation. The algorithm assumes that only these nodes have changed since the last run. + * @param operatingFunction - recomputes value of a node, and returns whether a change occurred + * @param onCycle - action to be performed when node is on cycle */ - public adjacentNodes(node: T): Set { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return this.edges.get(node)! + public getTopSortedWithSccSubgraphFrom( + modifiedNodes: Node[], + operatingFunction: (node: Node) => boolean, + onCycle: (node: Node) => void + ): TopSortResult { + const topSortAlgorithm = new TopSort(this.nodesSparseArray, this.edgesSparseArray) + const modifiedNodesIds = modifiedNodes.map(node => this.getNodeId(node)).filter(id => id !== undefined) as NodeId[] + return topSortAlgorithm.getTopSortedWithSccSubgraphFrom(modifiedNodesIds, operatingFunction, onCycle) } - public adjacentNodesCount(node: T): number { - return this.adjacentNodes(node).size + /** + * Marks node as volatile. + */ + public markNodeAsVolatile(node: Node): void { + const id = this.getNodeId(node) + + if (id === undefined) { + return + } + + this.dirtyAndVolatileNodeIds.rawValue.volatile.push(id) + this.dirtyAndVolatileNodeIds.markAsModified() } /** - * Checks whether a node is present in graph - * - * @param node - node to check + * Marks node as dirty. */ - public hasNode(node: T): boolean { - return this.nodes.has(node) + public markNodeAsDirty(node: Node): void { + const id = this.getNodeId(node) + + if (id === undefined) { + return + } + + this.dirtyAndVolatileNodeIds.rawValue.dirty.push(id) + this.dirtyAndVolatileNodeIds.markAsModified() } /** - * Returns number of nodes in graph + * Returns an array of nodes that are marked as dirty and/or volatile. */ - public nodesCount(): number { - return this.nodes.size + public getDirtyAndVolatileNodes(): Node[] { + return this.dirtyAndVolatileNodeIds.getProcessedValue() } /** - * Returns number of edges in graph + * Clears dirty nodes. */ - public edgesCount(): number { - let result = 0 - this.edges.forEach((edgesForNode) => (result += edgesForNode.size)) - return result + public clearDirtyNodes(): void { + this.dirtyAndVolatileNodeIds.rawValue.dirty = [] + this.dirtyAndVolatileNodeIds.markAsModified() } - public removeNode(node: T): [(SimpleCellAddress | SimpleCellRange), T][] { - for (const adjacentNode of this.adjacentNodes(node).values()) { - this.markNodeAsSpecialRecentlyChanged(adjacentNode) + /** + * Marks node as changingWithStructure. + */ + public markNodeAsChangingWithStructure(node: Node): void { + const id = this.getNodeId(node) + + if (id === undefined) { + return } - this.edges.delete(node) - this.nodes.delete(node) - this.specialNodes.delete(node) - this.specialNodesRecentlyChanged.delete(node) - this.specialNodesStructuralChanges.delete(node) - this.infiniteRanges.delete(node) - return this.removeDependencies(node) - } - public markNodeAsSpecial(node: T) { - this.specialNodes.add(node) + this.changingWithStructureNodeIds.push(id) } - public markNodeAsSpecialRecentlyChanged(node: T) { - if (this.nodes.has(node)) { - this.specialNodesRecentlyChanged.add(node) + /** + * Marks all nodes marked as changingWithStructure as dirty. + */ + public markChangingWithStructureNodesAsDirty(): void { + if (this.changingWithStructureNodeIds.length <= 0) { + return } + + this.dirtyAndVolatileNodeIds.rawValue.dirty = [ ...this.dirtyAndVolatileNodeIds.rawValue.dirty, ...this.changingWithStructureNodeIds ] + this.dirtyAndVolatileNodeIds.markAsModified() } - public markNodeAsChangingWithStructure(node: T) { - this.specialNodesStructuralChanges.add(node) + /** + * Marks node as infinite range. + */ + public markNodeAsInfiniteRange(node: Node | NodeId): void { + const id = this.getNodeIdIfNotNumber(node) + + if (id === undefined) { + return + } + + this.infiniteRangeIds.add(id) } - public clearSpecialNodesRecentlyChanged() { - this.specialNodesRecentlyChanged.clear() + /** + * Returns an array of nodes marked as infinite ranges + */ + public getInfiniteRanges(): NodeAndId[] { + return [ ...this.infiniteRangeIds].map(id => ({ node: this.nodesSparseArray[id], id })) } - public markNodeAsInfiniteRange(node: T) { - this.infiniteRanges.add(node) + /** + * Returns the internal id of a node. + */ + public getNodeId(node: Node): NodeId | undefined { + return this.nodesIds.get(node) } /** - * Checks whether exists edge between nodes * - * @param fromNode - node from which edge is outcoming - * @param toNode - node to which edge is incoming */ - public existsEdge(fromNode: T, toNode: T): boolean { - return this.edges.get(fromNode)?.has(toNode) ?? false + private getNodeIdIfNotNumber(node: Node | NodeId): NodeId | undefined { + return typeof node === 'number' ? node : this.nodesIds.get(node) } - /* - * return a topological sort order, but separates vertices that exist in some cycle + /** + * Removes invalid neighbors of a given node from the edges array and returns adjacent nodes for the input node. */ - public topSortWithScc(): TopSortResult { - return this.getTopSortedWithSccSubgraphFrom(Array.from(this.nodes), () => true, () => { - }) + private fixEdgesArrayForNode(id: NodeId): NodeId[] { + const adjacentNodeIds = this.edgesSparseArray[id] + this.edgesSparseArray[id] = adjacentNodeIds.filter(adjacentId => adjacentId !== undefined && this.nodesSparseArray[adjacentId]) + return this.edgesSparseArray[id] } /** - * - * an iterative implementation of Tarjan's algorithm for finding strongly connected compontents - * returns vertices in order of topological sort, but vertices that are on cycles are kept separate - * - * @param modifiedNodes - seed for computation. During engine init run, all of the vertices of grap. In recomputation run, changed vertices. - * @param operatingFunction - recomputes value of a node, and returns whether a change occured - * @param onCycle - action to be performed when node is on cycle + * Removes edges from the given node to its dependencies based on the dependencyQuery function. */ - public getTopSortedWithSccSubgraphFrom(modifiedNodes: T[], operatingFunction: (node: T) => boolean, onCycle: (node: T) => void): TopSortResult { - - const entranceTime: Map = new Map() - const low: Map = new Map() - const parent: Map = new Map() - const inSCC: Set = new Set() - - // node status life cycle: - // undefined -> ON_STACK -> PROCESSED -> POPPED - const nodeStatus: Map = new Map() - const order: T[] = [] - - let time: number = 0 - - const sccNonSingletons: Set = new Set() - - modifiedNodes.reverse() - modifiedNodes.forEach((v: T) => { - if (nodeStatus.get(v) !== undefined) { - return - } - const DFSstack: T[] = [v] - const SCCstack: T[] = [] - nodeStatus.set(v, NodeVisitStatus.ON_STACK) - while (DFSstack.length > 0) { - const u = DFSstack[DFSstack.length - 1] - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - switch (nodeStatus.get(u)!) { - case NodeVisitStatus.ON_STACK: { - entranceTime.set(u, time) - low.set(u, time) - SCCstack.push(u) - time++ - this.adjacentNodes(u).forEach((t: T) => { - if (entranceTime.get(t) === undefined) { - DFSstack.push(t) - parent.set(t, u) - nodeStatus.set(t, NodeVisitStatus.ON_STACK) - } - }) - nodeStatus.set(u, NodeVisitStatus.PROCESSED) - break - } - case NodeVisitStatus.PROCESSED: { // leaving this DFS subtree - let uLow: number - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - uLow = entranceTime.get(u)! - this.adjacentNodes(u).forEach((t: T) => { - if (!inSCC.has(t)) { - if (parent.get(t) === u) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - uLow = Math.min(uLow, low.get(t)!) - } else { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - uLow = Math.min(uLow, entranceTime.get(t)!) - } - } - }) - low.set(u, uLow) - if (uLow === entranceTime.get(u)) { - const currentSCC: T[] = [] - do { - currentSCC.push(SCCstack[SCCstack.length - 1]) - SCCstack.pop() - } while (currentSCC[currentSCC.length - 1] !== u) - currentSCC.forEach((t) => { - inSCC.add(t) - }) - order.push(...currentSCC) - if (currentSCC.length > 1) { - currentSCC.forEach((t) => { - sccNonSingletons.add(t) - }) - } - } - DFSstack.pop() - nodeStatus.set(u, NodeVisitStatus.POPPED) - break - } - case NodeVisitStatus.POPPED: { // it's a 'shadow' copy, we already processed this vertex and can ignore it - DFSstack.pop() - break - } - } - } - }) + private removeDependencies(node: Node): [(SimpleCellAddress | SimpleCellRange), Node][] { + const dependencies = this.dependencyQuery(node) - const shouldBeUpdatedMapping = new Set(modifiedNodes) - - const sorted: T[] = [] - const cycled: T[] = [] - order.reverse() - order.forEach((t: T) => { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - if (sccNonSingletons.has(t) || this.adjacentNodes(t).has(t)) { - cycled.push(t) - onCycle(t) - this.adjacentNodes(t).forEach((s: T) => shouldBeUpdatedMapping.add(s)) - } else { - sorted.push(t) - if (shouldBeUpdatedMapping.has(t) && operatingFunction(t)) { - this.adjacentNodes(t).forEach((s: T) => shouldBeUpdatedMapping.add(s)) - } - } + dependencies.forEach(([_, dependency]) => { + this.removeEdgeIfExists(dependency, node) }) - return {sorted, cycled} + + return dependencies } - public getDependencies(vertex: T): T[] { - const result: T[] = [] - this.edges.forEach((adjacentNodes, sourceNode) => { - if (adjacentNodes.has(vertex)) { - result.push(sourceNode) - } - }) - return result + /** + * processFn for dirtyAndVolatileNodeIds ProcessableValue instance + * @private + */ + private processDirtyAndVolatileNodeIds({ dirty, volatile }: { dirty: NodeId[], volatile: NodeId[] }): Node[] { + return [ ...new Set([ ...dirty, ...volatile]) ] + .map(id => this.nodesSparseArray[id]) + .filter(node => node !== undefined) } - private removeDependencies(node: T): [(SimpleCellAddress | SimpleCellRange), T][] { - const dependencies = this.dependencyQuery(node) - for (const [_, dependency] of dependencies) { - this.softRemoveEdge(dependency, node) - } - return dependencies + /** + * Returns error for missing node. + */ + private missingNodeError(node: Node): Error { + return new Error(`Unknown node ${node}`) } } diff --git a/src/DependencyGraph/ProcessableValue.ts b/src/DependencyGraph/ProcessableValue.ts new file mode 100644 index 000000000..a12b4580b --- /dev/null +++ b/src/DependencyGraph/ProcessableValue.ts @@ -0,0 +1,25 @@ +/** + * @license + * Copyright (c) 2023 Handsoncode. All rights reserved. + */ + +export class ProcessableValue { + private processedValue: Processed | null = null + + constructor( + public rawValue: Raw, + private processFn: (r: Raw) => Processed + ) {} + + getProcessedValue(): Processed { + if (this.processedValue === null) { + this.processedValue = this.processFn(this.rawValue) + } + + return this.processedValue + } + + markAsModified() { + this.processedValue = null + } +} diff --git a/src/DependencyGraph/TopSort.ts b/src/DependencyGraph/TopSort.ts new file mode 100644 index 000000000..f53d22b0f --- /dev/null +++ b/src/DependencyGraph/TopSort.ts @@ -0,0 +1,189 @@ +/** + * @license + * Copyright (c) 2023 Handsoncode. All rights reserved. + */ + +export interface TopSortResult { + sorted: T[], + cycled: T[], +} + +// node status life cycle: undefined -> ON_STACK -> PROCESSED -> POPPED +enum NodeVisitStatus { + ON_STACK, + PROCESSED, + POPPED, +} + +/** + * An algorithm class. Provides an iterative implementation of Tarjan's algorithm for finding strongly connected components + */ +export class TopSort { + private entranceTime: number[] = [] + private low: number[] = [] + private parent: number[] = [] + private inSCC: boolean[] = [] + private nodeStatus: NodeVisitStatus[] = [] + private order: number[] = [] + private sccNonSingletons: boolean[] = [] + private timeCounter: number = 0 + + constructor( + private nodesSparseArray: T[] = [], + private edgesSparseArray: number[][] = [], + ) { + } + + /** + * An iterative implementation of Tarjan's algorithm for finding strongly connected components. + * Returns vertices in order of topological sort, but vertices that are on cycles are kept separate. + * + * @param modifiedNodes - seed for computation. During engine init run, all of the vertices of grap. In recomputation run, changed vertices. + * @param operatingFunction - recomputes value of a node, and returns whether a change occured + * @param onCycle - action to be performed when node is on cycle + */ + public getTopSortedWithSccSubgraphFrom( + modifiedNodeIds: number[], + operatingFunction: (node: T) => boolean, + onCycle: (node: T) => void + ): TopSortResult { + const modifiedNodeIdsReversed = modifiedNodeIds.reverse() + modifiedNodeIdsReversed.forEach((id: number) => this.runDFS(id)) + return this.postprocess(modifiedNodeIdsReversed, onCycle, operatingFunction) + } + + /** + * Returns adjacent nodes of a given node. + */ + private getAdjacentNodeIds(id: number): number[] { + return this.edgesSparseArray[id].filter(adjacentId => adjacentId !== undefined && this.nodesSparseArray[adjacentId]) + } + + /** + * Runs DFS starting from a given node. + */ + private runDFS(v: number) { + if (this.nodeStatus[v] !== undefined) { + return + } + + this.nodeStatus[v] = NodeVisitStatus.ON_STACK + const DFSstack: number[] = [v] + const SCCstack: number[] = [] + + while (DFSstack.length > 0) { + const u = DFSstack[DFSstack.length - 1] + + switch (this.nodeStatus[u]) { + case NodeVisitStatus.ON_STACK: { + this.handleOnStack(u, SCCstack, DFSstack) + break + } + case NodeVisitStatus.PROCESSED: { // leaving this DFS subtree + this.handleProcessed(u, SCCstack, DFSstack) + break + } + case NodeVisitStatus.POPPED: { // it's a 'shadow' copy, we already processed this vertex and can ignore it + DFSstack.pop() + break + } + } + } + } + + /** + * Handles a node that is on stack. + */ + private handleOnStack(u: number, SCCstack: number[], DFSstack: number[]) { + this.entranceTime[u] = this.timeCounter + this.low[u] = this.timeCounter + this.timeCounter++ + SCCstack.push(u) + + this.getAdjacentNodeIds(u).forEach((t: number) => { + if (this.entranceTime[t] === undefined) { + DFSstack.push(t) + this.parent[t] = u + this.nodeStatus[t] = NodeVisitStatus.ON_STACK + } + }) + + this.nodeStatus[u] = NodeVisitStatus.PROCESSED + } + + /** + * Handles a node that is already processed. + */ + private handleProcessed(u: number, SCCstack: number[], DFSstack: number[]) { + let uLow = this.entranceTime[u] + + this.getAdjacentNodeIds(u).forEach((t: number) => { + if (this.inSCC[t]) { + return + } + + uLow = this.parent[t] === u ? Math.min(uLow, this.low[t]) : Math.min(uLow, this.entranceTime[t]) + }) + + this.low[u] = uLow + + if (uLow === this.entranceTime[u]) { + const currentSCC: number[] = [] + + do { + currentSCC.push(SCCstack[SCCstack.length - 1]) + SCCstack.pop() + } while (currentSCC[currentSCC.length - 1] !== u) + + currentSCC.forEach((t) => { + this.inSCC[t] = true + }) + + this.order.push(...currentSCC) + + if (currentSCC.length > 1) { + currentSCC.forEach((t) => { + this.sccNonSingletons[t] = true + }) + } + } + + DFSstack.pop() + this.nodeStatus[u] = NodeVisitStatus.POPPED + } + + /** + * Postprocesses the result of Tarjan's algorithm. + */ + private postprocess(modifiedNodeIds: number[], onCycle: (node: T) => void, operatingFunction: (node: T) => boolean) { + const shouldBeUpdatedMapping: boolean[] = [] + + modifiedNodeIds.forEach((t: number) => { + shouldBeUpdatedMapping[t] = true + }) + + const sorted: T[] = [] + const cycled: T[] = [] + this.order.reverse() + + this.order.forEach((t: number) => { + const adjacentNodes = this.getAdjacentNodeIds(t) + + // The following line is a potential performance bottleneck. + // Array.includes() is O(n) operation, which makes the whole algorithm O(n^2). + // Idea for improvement: use Set[] instead of number[][] for edgesSparseArray. + if (this.sccNonSingletons[t] || adjacentNodes.includes(t)) { + cycled.push(this.nodesSparseArray[t]) + onCycle(this.nodesSparseArray[t]) + adjacentNodes.forEach((s: number) => shouldBeUpdatedMapping[s] = true) + } else { + sorted.push(this.nodesSparseArray[t]) + if (shouldBeUpdatedMapping[t] && operatingFunction(this.nodesSparseArray[t])) { + adjacentNodes.forEach((s: number) => shouldBeUpdatedMapping[s] = true) + } + } + }) + + return {sorted, cycled} + } +} diff --git a/src/DependencyGraph/index.ts b/src/DependencyGraph/index.ts index 62c93a1fe..da1c0c879 100644 --- a/src/DependencyGraph/index.ts +++ b/src/DependencyGraph/index.ts @@ -6,6 +6,7 @@ export {DependencyGraph} from './DependencyGraph' export {AddressMapping} from './AddressMapping/AddressMapping' export {Graph} from './Graph' +export {TopSort} from './TopSort' export {RangeMapping} from './RangeMapping' export {SheetMapping} from './SheetMapping' export {ArrayMapping} from './ArrayMapping' diff --git a/src/HyperFormula.ts b/src/HyperFormula.ts index c5b351e01..1dbffeece 100644 --- a/src/HyperFormula.ts +++ b/src/HyperFormula.ts @@ -4380,8 +4380,8 @@ export class HyperFormula implements TypedEmitter { private recomputeIfDependencyGraphNeedsIt(): ExportedChange[] { if (!this._evaluationSuspended) { const changes = this._crudOperations.getAndClearContentChanges() - const verticesToRecomputeFrom = Array.from(this.dependencyGraph.verticesToRecompute()) - this.dependencyGraph.clearRecentlyChangedVertices() + const verticesToRecomputeFrom = this.dependencyGraph.verticesToRecompute() + this.dependencyGraph.clearDirtyVertices() if (verticesToRecomputeFrom.length > 0) { changes.addAll(this.evaluator.partialRun(verticesToRecomputeFrom)) diff --git a/src/Operations.ts b/src/Operations.ts index d924c235b..c2ae01254 100644 --- a/src/Operations.ts +++ b/src/Operations.ts @@ -248,7 +248,7 @@ export class Operations { public addSheet(name?: string) { const sheetId = this.sheetMapping.addSheet(name) const sheet: Sheet = [] - this.dependencyGraph.addressMapping.autoAddSheet(sheetId, sheet, findBoundaries(sheet)) + this.dependencyGraph.addressMapping.autoAddSheet(sheetId, findBoundaries(sheet)) return this.sheetMapping.fetchDisplayName(sheetId) } @@ -807,9 +807,13 @@ export class Operations { if (sheetId === undefined) { return } - const localVertex = this.dependencyGraph.fetchCellOrCreateEmpty(namedExpression.address) + const { vertex: localVertex, id: maybeLocalVertexId } = this.dependencyGraph.fetchCellOrCreateEmpty(namedExpression.address) + const localVertexId = maybeLocalVertexId ?? this.dependencyGraph.graph.getNodeId(localVertex)! + const globalNamedExpression = this.namedExpressions.workbookNamedExpressionOrPlaceholder(expressionName) - const globalVertex = this.dependencyGraph.fetchCellOrCreateEmpty(globalNamedExpression.address) + const { vertex: globalVertex, id: maybeGlobalVertexId } = this.dependencyGraph.fetchCellOrCreateEmpty(globalNamedExpression.address) + const globalVertexId = maybeGlobalVertexId ?? this.dependencyGraph.graph.getNodeId(globalVertex)! + for (const adjacentNode of this.dependencyGraph.graph.adjacentNodes(globalVertex)) { if (adjacentNode instanceof FormulaCellVertex && adjacentNode.getAddress(this.lazilyTransformingAstService).sheet === sheetId) { const ast = adjacentNode.getFormula(this.lazilyTransformingAstService) @@ -817,8 +821,8 @@ export class Operations { const {dependencies} = this.parser.fetchCachedResultForAst(ast) for (const dependency of absolutizeDependencies(dependencies, formulaAddress)) { if (dependency instanceof NamedExpressionDependency && dependency.name.toLowerCase() === namedExpression.displayName.toLowerCase()) { - this.dependencyGraph.graph.removeEdge(globalVertex, adjacentNode) - this.dependencyGraph.graph.addEdge(localVertex, adjacentNode) + this.dependencyGraph.graph.removeEdge(globalVertexId, adjacentNode) + this.dependencyGraph.graph.addEdge(localVertexId, adjacentNode) } } } @@ -875,15 +879,15 @@ export class Operations { } const expressionName = namedExpressionDependency.name - const sourceVertex = this.dependencyGraph.fetchNamedExpressionVertex(expressionName, sourceSheet) + const sourceVertex = this.dependencyGraph.fetchNamedExpressionVertex(expressionName, sourceSheet).vertex const namedExpressionInTargetScope = this.namedExpressions.isExpressionInScope(expressionName, targetAddress.sheet) const targetScopeExpressionVertex = namedExpressionInTargetScope - ? this.dependencyGraph.fetchNamedExpressionVertex(expressionName, targetAddress.sheet) + ? this.dependencyGraph.fetchNamedExpressionVertex(expressionName, targetAddress.sheet).vertex : this.copyOrFetchGlobalNamedExpressionVertex(expressionName, sourceVertex, addedGlobalNamedExpressions) if (targetScopeExpressionVertex !== sourceVertex) { - this.dependencyGraph.graph.softRemoveEdge(sourceVertex, vertex) + this.dependencyGraph.graph.removeEdgeIfExists(sourceVertex, vertex) this.dependencyGraph.graph.addEdge(targetScopeExpressionVertex, vertex) } } @@ -910,7 +914,7 @@ export class Operations { this.setValueToCell(sourceVertex.getValues(), expression.address) } } - return this.dependencyGraph.fetchCellOrCreateEmpty(expression.address) + return this.dependencyGraph.fetchCellOrCreateEmpty(expression.address).vertex } } diff --git a/src/errors.ts b/src/errors.ts index 970b6fecf..f509e5045 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -262,8 +262,6 @@ export class EvaluationSuspendedError extends Error { /** * Error thrown when translation is missing in translation package. - * - * TODO */ export class MissingTranslationError extends Error { constructor(key: string) { diff --git a/test/address-mapping.spec.ts b/test/address-mapping.spec.ts index 7757a233e..920d34799 100644 --- a/test/address-mapping.spec.ts +++ b/test/address-mapping.spec.ts @@ -693,7 +693,7 @@ describe('AddressMapping', () => { [null, null, null], [null, null, '1'], ] - addressMapping.autoAddSheet(0, sheet, findBoundaries(sheet)) + addressMapping.autoAddSheet(0, findBoundaries(sheet)) expect(addressMapping.strategyFor(0)).toBeInstanceOf(SparseStrategy) }) @@ -704,7 +704,7 @@ describe('AddressMapping', () => { ['1', '1'], ['1', '1'], ] - addressMapping.autoAddSheet(0, sheet, findBoundaries(sheet)) + addressMapping.autoAddSheet(0, findBoundaries(sheet)) expect(addressMapping.strategyFor(0)).toBeInstanceOf(DenseStrategy) }) diff --git a/test/column-range.spec.ts b/test/column-range.spec.ts index 0503629cb..f7f8691e2 100644 --- a/test/column-range.spec.ts +++ b/test/column-range.spec.ts @@ -64,7 +64,7 @@ describe('Column ranges', () => { engine.removeColumns(0, [1, 1]) - expect(engine.graph.infiniteRanges.size).toBe(0) + expect(engine.graph.getInfiniteRanges().length).toBe(0) }) it('should not move infinite range', () => { diff --git a/test/crud-random.spec.ts b/test/crud-random.spec.ts index 2085d1a8d..68b80fc9b 100644 --- a/test/crud-random.spec.ts +++ b/test/crud-random.spec.ts @@ -271,7 +271,7 @@ describe('large psuedo-random test', () => { verifyValues(engine) } randomCleanup(engine, rectangleFromCorner({x: 0, y: 0}, 2 * (n + 1) * sideX, 2 * sideY)) - expect(engine.dependencyGraph.graph.nodesCount()).toBe(0) + expect(engine.dependencyGraph.graph.getNodes().length).toBe(0) expect(engine.dependencyGraph.rangeMapping.getMappingSize(0)).toBe(0) }) }) diff --git a/test/cruds/change-cell-content.spec.ts b/test/cruds/change-cell-content.spec.ts index 982b032e2..96ff5eba7 100644 --- a/test/cruds/change-cell-content.spec.ts +++ b/test/cruds/change-cell-content.spec.ts @@ -159,7 +159,7 @@ describe('changing cell content', () => { expect(engine.getCellValue(adr('B1'))).toBe(1) engine.setCellContents(adr('B1'), [[null]]) expect(engine.getCellValue(adr('B1'))).toBe(null) - expect(engine.graph.nodes).not.toContain(b1) + expect([ ...engine.graph.getNodes() ]).not.toContain(b1) expect(engine.graph.existsEdge(a1, b1)).toBe(false) }) diff --git a/test/cruds/cut-paste.spec.ts b/test/cruds/cut-paste.spec.ts index a10b9b4fd..c3a042a1e 100644 --- a/test/cruds/cut-paste.spec.ts +++ b/test/cruds/cut-paste.spec.ts @@ -11,7 +11,7 @@ import { expectEngineToBeTheSameAs, extractMatrixRange, extractRange, - extractReference, + extractReference, graphEdgesCount, } from '../testUtils' import {ExpectedValueOfTypeError} from '../../src/errors' @@ -201,8 +201,8 @@ describe('Move cells', () => { engine.cut(AbsoluteCellRange.spanFrom(adr('A1'), 1, 1)) engine.paste(adr('A2')) - expect(engine.graph.edgesCount()).toBe(0) - expect(engine.graph.nodesCount()).toBe(1) + expect(graphEdgesCount(engine.graph)).toBe(0) + expect(engine.graph.getNodes().length).toBe(1) expect(engine.getCellValue(adr('A1'))).toBe(null) expect(engine.getCellValue(adr('A2'))).toBe(1) }) @@ -260,10 +260,10 @@ describe('Move cells', () => { const source = engine.addressMapping.getCell(adr('A1')) const target = engine.addressMapping.fetchCell(adr('A2')) - expect(engine.graph.edgesCount()).toBe( + expect(graphEdgesCount(engine.graph)).toBe( 2, // A2 -> B1, A2 -> B2 ) - expect(engine.graph.nodesCount()).toBe( + expect(engine.graph.getNodes().length).toBe( +2 // formulas + 1, // A2 ) @@ -380,13 +380,13 @@ describe('moving ranges', () => { expect(source).toBeInstanceOf(EmptyCellVertex) expect(source.getCellValue()).toBe(EmptyValue) - expect(engine.graph.nodesCount()).toBe( + expect(engine.graph.getNodes().length).toBe( +2 // formulas + 1 // A2 + 1 // A1 (Empty) + 1, // A1:A2 range ) - expect(engine.graph.edgesCount()).toBe( + expect(graphEdgesCount(engine.graph)).toBe( +2 // A1 (Empty) -> A1:A2, A2 -> A1:A2 + 1 // A1:A2 -> B1 + 1, // A2 -> B2 @@ -422,12 +422,12 @@ describe('moving ranges', () => { expect(a1).toBe(undefined) expect(a2).toBe(undefined) - expect(engine.graph.nodesCount()).toBe( + expect(engine.graph.getNodes().length).toBe( +2 // formulas + 2 // C1, C2 + 1, // C1:C2 range ) - expect(engine.graph.edgesCount()).toBe( + expect(graphEdgesCount(engine.graph)).toBe( +2 // C1 -> C1:C2, C2 -> C1:C2 + 1 // C1:C2 -> B1 + 1, // C2 -> B2 diff --git a/test/cruds/move-cells.spec.ts b/test/cruds/move-cells.spec.ts index 1f7a60915..808ee38cd 100644 --- a/test/cruds/move-cells.spec.ts +++ b/test/cruds/move-cells.spec.ts @@ -18,7 +18,7 @@ import { extractMatrixRange, extractRange, extractReference, - extractRowRange, + extractRowRange, graphEdgesCount, rowEnd, rowStart, } from '../testUtils' @@ -304,8 +304,8 @@ describe('Move cells', () => { engine.moveCells(AbsoluteCellRange.spanFrom(adr('A1'), 1, 1), adr('A2')) - expect(engine.graph.edgesCount()).toBe(0) - expect(engine.graph.nodesCount()).toBe(1) + expect(graphEdgesCount(engine.graph)).toBe(0) + expect(engine.graph.getNodes().length).toBe(1) expect(engine.getCellValue(adr('A1'))).toBe(null) expect(engine.getCellValue(adr('A2'))).toBe(1) }) @@ -359,10 +359,10 @@ describe('Move cells', () => { const source = engine.addressMapping.getCell(adr('A1')) const target = engine.addressMapping.fetchCell(adr('A2')) - expect(engine.graph.edgesCount()).toBe( + expect(graphEdgesCount(engine.graph)).toBe( 2, // A2 -> B1, A2 -> B2 ) - expect(engine.graph.nodesCount()).toBe( + expect(engine.graph.getNodes().length).toBe( +2 // formulas + 1, // A2 ) @@ -512,13 +512,13 @@ describe('moving ranges', () => { expect(source).toBeInstanceOf(EmptyCellVertex) expect(source.getCellValue()).toBe(EmptyValue) - expect(engine.graph.nodesCount()).toBe( + expect(engine.graph.getNodes().length).toBe( +2 // formulas + 1 // A2 + 1 // A1 (Empty) + 1, // A1:A2 range ) - expect(engine.graph.edgesCount()).toBe( + expect(graphEdgesCount(engine.graph)).toBe( +2 // A1 (Empty) -> A1:A2, A2 -> A1:A2 + 1 // A1:A2 -> B1 + 1, // A2 -> B2 @@ -553,12 +553,12 @@ describe('moving ranges', () => { expect(a1).toBe(undefined) expect(a2).toBe(undefined) - expect(engine.graph.nodesCount()).toBe( + expect(engine.graph.getNodes().length).toBe( +2 // formulas + 2 // C1, C2 + 1, // C1:C2 range ) - expect(engine.graph.edgesCount()).toBe( + expect(graphEdgesCount(engine.graph)).toBe( +2 // C1 -> C1:C2, C2 -> C1:C2 + 1 // C1:C2 -> B1 + 1, // C2 -> B2 diff --git a/test/cruds/removing-columns.spec.ts b/test/cruds/removing-columns.spec.ts index 35b9b3dec..638352a32 100644 --- a/test/cruds/removing-columns.spec.ts +++ b/test/cruds/removing-columns.spec.ts @@ -657,18 +657,18 @@ describe('Removing columns - graph', function() { ['1', '2', '3', '4'], ['1', '2', '3', '4'], ]) - expect(engine.graph.nodes.size).toBe(8) + expect(engine.graph.getNodes().length).toBe(8) engine.removeColumns(0, [0, 2]) - expect(engine.graph.nodes.size).toBe(4) // left two vertices in first column, two in last + expect(engine.graph.getNodes().length).toBe(4) // left two vertices in first column, two in last }) it('works if there are empty cells removed', function() { const engine = HyperFormula.buildFromArray([ ['1', null, '3'], ]) - expect(engine.graph.nodes.size).toBe(2) + expect(engine.graph.getNodes().length).toBe(2) engine.removeColumns(0, [1, 1]) - expect(engine.graph.nodes.size).toBe(2) + expect(engine.graph.getNodes().length).toBe(2) }) }) @@ -884,7 +884,7 @@ describe('Removing columns - merge ranges', () => { verifyRangesInSheet(engine, 0, []) verifyValues(engine) - expect(engine.dependencyGraph.graph.nodesCount()).toBe(0) + expect(engine.dependencyGraph.graph.getNodes().length).toBe(0) expect(engine.dependencyGraph.rangeMapping.getMappingSize(0)).toBe(0) }) diff --git a/test/cruds/removing-rows.spec.ts b/test/cruds/removing-rows.spec.ts index 78499fea7..466328e29 100644 --- a/test/cruds/removing-rows.spec.ts +++ b/test/cruds/removing-rows.spec.ts @@ -11,7 +11,7 @@ import { expectReferenceToHaveRefError, extractMatrixRange, extractRange, - extractReference, + extractReference, graphReversedAdjacentNodes, noSpace, verifyRangesInSheet, verifyValues, @@ -746,9 +746,9 @@ describe('Removing rows - graph', function() { ['1', '2'], ['3', '4'], ]) - expect(engine.graph.nodes.size).toBe(4) + expect(engine.graph.getNodes().length).toBe(4) engine.removeRows(0, [0, 2]) - expect(engine.graph.nodes.size).toBe(0) + expect(engine.graph.getNodes().length).toBe(0) }) it('works if there are empty cells removed', function() { @@ -757,9 +757,9 @@ describe('Removing rows - graph', function() { [null], ['3'], ]) - expect(engine.graph.nodes.size).toBe(2) + expect(engine.graph.getNodes().length).toBe(2) engine.removeRows(0, [1, 1]) - expect(engine.graph.nodes.size).toBe(2) + expect(engine.graph.getNodes().length).toBe(2) }) }) @@ -815,11 +815,11 @@ describe('Removing rows - range mapping', function() { ]) const a1a3 = engine.rangeMapping.fetchRange(adr('A1'), adr('A3')) - expect(engine.graph.getDependencies(a1a3).length).toBe(2) + expect(graphReversedAdjacentNodes(engine.graph, a1a3).length).toBe(2) engine.removeRows(0, [0, 2]) const a1a1 = engine.rangeMapping.fetchRange(adr('A1'), adr('A1')) expect(a1a1).toBe(a1a3) - expect(engine.graph.getDependencies(a1a1).length).toBe(1) + expect(graphReversedAdjacentNodes(engine.graph, a1a1).length).toBe(1) }) }) @@ -1027,7 +1027,7 @@ describe('Removing rows - merge ranges', () => { verifyRangesInSheet(engine, 0, []) verifyValues(engine) - expect(engine.dependencyGraph.graph.nodesCount()).toBe(0) + expect(engine.dependencyGraph.graph.getNodes().length).toBe(0) expect(engine.dependencyGraph.rangeMapping.getMappingSize(0)).toBe(0) }) diff --git a/test/graph-builder.spec.ts b/test/graph-builder.spec.ts index c17e633f9..0efd0dc79 100644 --- a/test/graph-builder.spec.ts +++ b/test/graph-builder.spec.ts @@ -2,7 +2,7 @@ import {HyperFormula} from '../src' import {Config} from '../src/Config' import {EmptyCellVertex, ValueCellVertex} from '../src/DependencyGraph' import {SheetSizeLimitExceededError} from '../src/errors' -import {adr, colEnd, colStart} from './testUtils' +import {adr, colEnd, colStart, graphEdgesCount} from './testUtils' describe('GraphBuilder', () => { it('build sheet with simple number cell', () => { @@ -80,7 +80,7 @@ describe('GraphBuilder', () => { expect(engine.graph.existsEdge(b1, a1b2)).toBe(true) expect(engine.graph.existsEdge(a1b2, a2)).toBe(true) expect(engine.graph.existsEdge(a1b2, a3)).toBe(true) - expect(engine.graph.nodesCount()).toBe( + expect(engine.graph.getNodes().length).toBe( 4 + // for cells above 1, // for both ranges (reuse same ranges) ) @@ -99,7 +99,7 @@ describe('GraphBuilder', () => { expect(engine.graph.existsEdge(a3, a1a3)).toBe(true) expect(engine.graph.existsEdge(a1a2, a1a3)).toBe(true) - expect(engine.graph.edgesCount()).toBe( + expect(graphEdgesCount(engine.graph)).toBe( 2 + // from cells to range(A1:A2) 2 + // from A3 and range(A1:A2) to range(A1:A3) 2, // from range vertexes to formulas @@ -111,12 +111,12 @@ describe('GraphBuilder', () => { ['1', '0', '=SUM(A1:B2)'], ]) - expect(engine.graph.nodesCount()).toBe( + expect(engine.graph.getNodes().length).toBe( 3 + // for cells above 1 + // for range vertex 2, // for 2 EmptyCellVertex instances ) - expect(engine.graph.edgesCount()).toBe( + expect(graphEdgesCount(engine.graph)).toBe( 2 + // from cells to range vertex 2 + // from EmptyCellVertex instances to range vertices 1, // from range to cell with SUM @@ -136,7 +136,7 @@ describe('GraphBuilder', () => { expect(engine.graph.existsEdge(a2, a1a3)).toBe(true) expect(engine.graph.existsEdge(a2, a1a2)).toBe(true) expect(engine.graph.existsEdge(a1a2, a1a3)).toBe(false) - expect(engine.graph.edgesCount()).toBe( + expect(graphEdgesCount(engine.graph)).toBe( 3 + // from 3 cells to range(A1:A2) 2 + // from 2 cells to range(A1:A2) 2, // from range vertexes to formulas diff --git a/test/graph-garbage-collection.spec.ts b/test/graph-garbage-collection.spec.ts index d8d8c5470..bc6ea1932 100644 --- a/test/graph-garbage-collection.spec.ts +++ b/test/graph-garbage-collection.spec.ts @@ -8,9 +8,9 @@ describe('vertex counting', () => { ['1', '2'], ['3', '4'] ]) - expect(engine.dependencyGraph.graph.nodesCount()).toBe(4) + expect(engine.dependencyGraph.graph.getNodes().length).toBe(4) engine.calculateFormula('=SUM(A1:B2)', 0) - expect(engine.dependencyGraph.graph.nodesCount()).toBe(4) + expect(engine.dependencyGraph.graph.getNodes().length).toBe(4) }) it('cruds', () => { @@ -18,11 +18,11 @@ describe('vertex counting', () => { ['1', '2'], ['3', '4'] ]) - expect(engine.dependencyGraph.graph.nodesCount()).toBe(4) + expect(engine.dependencyGraph.graph.getNodes().length).toBe(4) engine.setCellContents(adr('A1'), '=SUM(A2:B2)') - expect(engine.dependencyGraph.graph.nodesCount()).toBe(5) + expect(engine.dependencyGraph.graph.getNodes().length).toBe(5) engine.setCellContents(adr('A1'), 1) - expect(engine.dependencyGraph.graph.nodesCount()).toBe(4) + expect(engine.dependencyGraph.graph.getNodes().length).toBe(4) }) }) @@ -75,7 +75,7 @@ describe('larger tests', () => { engine.setCellContents({sheet: 0, col: x, row: y}, null) } } - expect(engine.dependencyGraph.graph.nodesCount()).toBe(0) + expect(engine.dependencyGraph.graph.getNodes().length).toBe(0) expect(engine.dependencyGraph.rangeMapping.getMappingSize(0)).toBe(0) }) @@ -92,7 +92,7 @@ describe('larger tests', () => { engine.setCellContents({sheet: 0, col: 1, row: 0}, null) engine.setCellContents({sheet: 0, col: 2, row: 0}, null) engine.setCellContents({sheet: 0, col: 3, row: 0}, null) - expect(engine.dependencyGraph.graph.nodesCount()).toBe(0) + expect(engine.dependencyGraph.graph.getNodes().length).toBe(0) expect(engine.dependencyGraph.rangeMapping.getMappingSize(0)).toBe(0) }) @@ -106,7 +106,7 @@ describe('larger tests', () => { engine.setCellContents({sheet: 0, col: 0, row: 0}, null) - expect(engine.dependencyGraph.graph.nodesCount()).toBe(0) + expect(engine.dependencyGraph.graph.getNodes().length).toBe(0) expect(engine.dependencyGraph.rangeMapping.getMappingSize(0)).toBe(0) }) @@ -123,7 +123,7 @@ describe('larger tests', () => { engine.setCellContents({sheet: 0, col: 3, row: 0}, null) engine.setCellContents({sheet: 0, col: 4, row: 0}, null) - expect(engine.dependencyGraph.graph.nodesCount()).toBe(0) + expect(engine.dependencyGraph.graph.getNodes().length).toBe(0) expect(engine.dependencyGraph.rangeMapping.getMappingSize(0)).toBe(0) }) @@ -156,7 +156,7 @@ describe('larger tests', () => { engine.setCellContents({sheet: 0, col: x, row: y}, null) } } - expect(engine.dependencyGraph.graph.nodesCount()).toBe(0) + expect(engine.dependencyGraph.graph.getNodes().length).toBe(0) expect(engine.dependencyGraph.rangeMapping.getMappingSize(0)).toBe(0) }) }) @@ -175,7 +175,7 @@ describe('cruds', () => { engine.removeRows(0, [0, 2]) engine.removeRows(0, [2, 2]) - expect(engine.dependencyGraph.graph.nodesCount()).toBe(0) + expect(engine.dependencyGraph.graph.getNodes().length).toBe(0) expect(engine.dependencyGraph.rangeMapping.getMappingSize(0)).toBe(0) }) @@ -192,7 +192,7 @@ describe('cruds', () => { engine.removeRows(0, [0, 2]) engine.removeRows(0, [2, 2]) - expect(engine.dependencyGraph.graph.nodesCount()).toBe(0) + expect(engine.dependencyGraph.graph.getNodes().length).toBe(0) expect(engine.dependencyGraph.rangeMapping.getMappingSize(0)).toBe(0) }) @@ -208,7 +208,7 @@ describe('cruds', () => { engine.removeRows(0, [4, 2]) - expect(engine.dependencyGraph.graph.nodesCount()).toBe(0) + expect(engine.dependencyGraph.graph.getNodes().length).toBe(0) expect(engine.dependencyGraph.rangeMapping.getMappingSize(0)).toBe(0) }) @@ -229,7 +229,7 @@ describe('cruds', () => { engine.removeRows(0, [0, 6]) - expect(engine.dependencyGraph.graph.nodesCount()).toBe(0) + expect(engine.dependencyGraph.graph.getNodes().length).toBe(0) expect(engine.dependencyGraph.rangeMapping.getMappingSize(0)).toBe(0) }) @@ -250,7 +250,7 @@ describe('cruds', () => { engine.removeRows(0, [0, 6]) - expect(engine.dependencyGraph.graph.nodesCount()).toBe(0) + expect(engine.dependencyGraph.graph.getNodes().length).toBe(0) expect(engine.dependencyGraph.rangeMapping.getMappingSize(0)).toBe(0) }) @@ -269,7 +269,7 @@ describe('cruds', () => { engine.removeRows(0, [0, 6]) - expect(engine.dependencyGraph.graph.nodesCount()).toBe(0) + expect(engine.dependencyGraph.graph.getNodes().length).toBe(0) expect(engine.dependencyGraph.rangeMapping.getMappingSize(0)).toBe(0) }) @@ -292,7 +292,7 @@ describe('cruds', () => { engine.removeRows(0, [0, 6]) - expect(engine.dependencyGraph.graph.nodesCount()).toBe(0) + expect(engine.dependencyGraph.graph.getNodes().length).toBe(0) expect(engine.dependencyGraph.rangeMapping.getMappingSize(0)).toBe(0) }) @@ -329,7 +329,7 @@ describe('cruds', () => { engine.setCellContents(adr('A5'), null) engine.setCellContents(adr('A6'), null) - expect(engine.dependencyGraph.graph.nodesCount()).toBe(0) + expect(engine.dependencyGraph.graph.getNodes().length).toBe(0) expect(engine.dependencyGraph.rangeMapping.getMappingSize(0)).toBe(0) }) diff --git a/test/graph-vertex.spec.ts b/test/graph-vertex.spec.ts index b3d07add3..0d52430f0 100644 --- a/test/graph-vertex.spec.ts +++ b/test/graph-vertex.spec.ts @@ -3,15 +3,15 @@ import {Graph, ValueCellVertex, Vertex} from '../src/DependencyGraph' const dummyGetDependenciesQuery: () => any[] = () => [] describe('Graph with Vertex', () => { - it('#addNode works correctly with Vertex instances', () => { + it('#addNodeAndReturnId works correctly with Vertex instances', () => { const graph = new Graph(dummyGetDependenciesQuery) const v1 = new ValueCellVertex("1'", "1'") const v2 = new ValueCellVertex('2', '2') - graph.addNode(v1) - graph.addNode(v1) - graph.addNode(v2) + graph.addNodeAndReturnId(v1) + graph.addNodeAndReturnId(v1) + graph.addNodeAndReturnId(v2) - expect(graph.nodesCount()).toBe(2) + expect(graph.getNodes().length).toBe(2) }) }) diff --git a/test/graph.spec.ts b/test/graph.spec.ts index 288b0fc4a..bf5515369 100644 --- a/test/graph.spec.ts +++ b/test/graph.spec.ts @@ -1,425 +1,554 @@ import {Graph} from '../src/DependencyGraph' +import {DependencyQuery} from '../src/DependencyGraph/Graph' +import {graphEdgesCount} from './testUtils' -const identifiableString = (id: number, str: string) => ({id, str}) +class IdentifiableString { + constructor( + public id: number, + public str: string) { + } +} -const dummyGetDependenciesQuery: () => any[] = () => [] +const dummyDependencyQuery: DependencyQuery = () => [] -describe('Basic Graph manipulation', () => { - it('#addNode', () => { - const graph = new Graph(dummyGetDependenciesQuery) +describe('Graph class', () => { + describe('addNodeAndReturnId', () => { + it('adds a node to the empty graph', () => { + const graph = new Graph(dummyDependencyQuery) - const node = identifiableString(0, 'foo') - graph.addNode(node) + const node = new IdentifiableString(0, 'foo') + graph.addNodeAndReturnId(node) - expect(graph.nodesCount()).toBe(1) - }) - - it('#addNode for the second time', () => { - const graph = new Graph(dummyGetDependenciesQuery) + expect(graph.getNodes().length).toBe(1) + }) - const node = identifiableString(0, 'foo') - graph.addNode(node) - graph.addNode(node) + it('does not add duplicate nodes', () => { + const graph = new Graph(dummyDependencyQuery) - expect(graph.nodesCount()).toBe(1) - }) + const node = new IdentifiableString(0, 'foo') + graph.addNodeAndReturnId(node) + graph.addNodeAndReturnId(node) - it('#addNode for the second time doesnt reset adjacent nodes', () => { - const graph = new Graph(dummyGetDependenciesQuery) + expect(graph.getNodes().length).toBe(1) + }) - const node0 = identifiableString(0, 'foo') - const node1 = identifiableString(1, 'bar') - graph.addNode(node0) - graph.addNode(node1) - graph.addEdge(node0, node1) + it('keeps existing edges when dealing with duplicates', () => { + const graph = new Graph(dummyDependencyQuery) - graph.addNode(node0) - - expect(graph.adjacentNodes(node0)).toEqual(new Set([node1])) - }) + const node0 = new IdentifiableString(0, 'foo') + const node1 = new IdentifiableString(1, 'bar') + graph.addNodeAndReturnId(node0) + graph.addNodeAndReturnId(node1) + expect(graph.adjacentNodes(node0)).toEqual(new Set([])) - it('#hasNode when there is node', () => { - const graph = new Graph(dummyGetDependenciesQuery) + graph.addEdge(node0, node1) + expect(graph.adjacentNodes(node0)).toEqual(new Set([node1])) - const node = identifiableString(0, 'foo') - graph.addNode(node) + graph.addNodeAndReturnId(node0) - expect(graph.hasNode(node)).toBe(true) + expect(graph.adjacentNodes(node0)).toEqual(new Set([node1])) + }) }) - it('#hasNode when there is no node', () => { - const graph = new Graph(dummyGetDependenciesQuery) + describe('removeNode', () => { + it('removes a node if it exists', () => { + const graph = new Graph(dummyDependencyQuery) - expect(graph.hasNode(identifiableString(0, 'foo'))).toBe(false) - }) + const node = new IdentifiableString(0, 'foo') + graph.addNodeAndReturnId(node) + graph.removeNode(node) - it('#adjacentNodes', () => { - const graph = new Graph(dummyGetDependenciesQuery) + expect(graph.getNodes().length).toBe(0) + }) - const node0 = identifiableString(0, 'foo') - const node1 = identifiableString(1, 'bar') - graph.addNode(node0) - graph.addNode(node1) - graph.addEdge(node0, node1) + it('throws error when node does not exist', () => { + const graph = new Graph(dummyDependencyQuery) - expect(graph.adjacentNodes(node0)).toEqual(new Set([node1])) + expect(() => graph.removeNode(new IdentifiableString(0, 'foo'))).toThrowError(/Unknown node/) + }) }) - it('#addEdge removes multiple edges', () => { - const graph = new Graph(dummyGetDependenciesQuery) + describe('hasNode', () => { + it('returns false when graph is empty', () => { + const graph = new Graph(dummyDependencyQuery) - const node0 = identifiableString(0, 'foo') - const node1 = identifiableString(1, 'bar') - graph.addNode(node0) - graph.addNode(node1) - graph.addEdge(node0, node1) - graph.addEdge(node0, node1) + expect(graph.hasNode(new IdentifiableString(0, 'foo'))).toBe(false) + }) - expect(graph.adjacentNodes(node0)).toEqual(new Set([node1])) - }) + it('returns true if node exists', () => { + const graph = new Graph(dummyDependencyQuery) - it('#addEdge is raising an error when the origin node not present', () => { - const graph = new Graph(dummyGetDependenciesQuery) - const node = identifiableString(1, 'target') - graph.addNode(node) + const node = new IdentifiableString(0, 'foo') + graph.addNodeAndReturnId(node) - expect(() => { - graph.addEdge(identifiableString(0, 'origin'), node) - }).toThrowError(/Unknown node/) - }) + expect(graph.hasNode(node)).toBe(true) + }) - it('#addEdge is raising an error when the target node not present', () => { - const graph = new Graph(dummyGetDependenciesQuery) - const node = identifiableString(0, 'origin') - graph.addNode(node) + it('returns false if node does not exist', () => { + const graph = new Graph(dummyDependencyQuery) - expect(() => { - graph.addEdge(node, identifiableString(1, 'target')) - }).toThrowError(/Unknown node/) - }) + const node = new IdentifiableString(0, 'foo') + graph.addNodeAndReturnId(node) - it('#existsEdge works', () => { - const graph = new Graph(dummyGetDependenciesQuery) - const node0 = identifiableString(0, 'foo') - const node1 = identifiableString(1, 'bar') - graph.addNode(node0) - graph.addNode(node1) - graph.addEdge(node0, node1) + expect(graph.hasNode(new IdentifiableString(1, 'foo'))).toBe(false) + }) - expect(graph.existsEdge(node0, node1)).toBe(true) - }) + it('returns false if node was removed', () => { + const graph = new Graph(dummyDependencyQuery) - it('#existsEdge when there is origin node but no edge', () => { - const graph = new Graph(dummyGetDependenciesQuery) - const node = identifiableString(0, 'foo') - graph.addNode(node) + const node = new IdentifiableString(0, 'foo') + graph.addNodeAndReturnId(node) + graph.removeNode(node) - expect(graph.existsEdge(node, identifiableString(1, 'bar'))).toBe(false) + expect(graph.hasNode(node)).toBe(false) + }) }) - it('#existsEdge when there is no node', () => { - const graph = new Graph(dummyGetDependenciesQuery) + describe('addEdge', () => { + it('does not add duplicated edges', () => { + const graph = new Graph(dummyDependencyQuery) - expect(graph.existsEdge(identifiableString(0, 'foo'), identifiableString(1, 'bar'))).toBe(false) - }) - - it('#edgesCount when there is no nodes', () => { - const graph = new Graph(dummyGetDependenciesQuery) + const node0 = new IdentifiableString(0, 'foo') + const node1 = new IdentifiableString(1, 'bar') + graph.addNodeAndReturnId(node0) + graph.addNodeAndReturnId(node1) + graph.addEdge(node0, node1) + graph.addEdge(node0, node1) - expect(graph.edgesCount()).toBe(0) - }) - - it('#edgesCount counts edges from all nodes', () => { - const graph = new Graph(dummyGetDependenciesQuery) - const node0 = identifiableString(0, 'bar1') - const node1 = identifiableString(1, 'bar2') - graph.addNode(node0) - graph.addNode(node1) - const node2 = identifiableString(2, 'first') - graph.addNode(node2) - graph.addEdge(node2, node0) - const node3 = identifiableString(2, 'second') - graph.addNode(node3) - graph.addEdge(node3, node0) - graph.addEdge(node3, node1) - - expect(graph.edgesCount()).toBe(3) - }) + expect(graph.adjacentNodes(node0)).toEqual(new Set([node1])) + }) - it('#topologicalSort for empty graph', () => { - const graph = new Graph(dummyGetDependenciesQuery) + it('throws error when the origin node is not present', () => { + const graph = new Graph(dummyDependencyQuery) + const node = new IdentifiableString(1, 'target') + graph.addNodeAndReturnId(node) - expect(graph.topSortWithScc().sorted).toEqual([]) - expect(graph.topSortWithScc().cycled).toEqual([]) - }) - - it('#topologicalSort node is included even if he is not connected to anything', () => { - const graph = new Graph(dummyGetDependenciesQuery) - const node = identifiableString(0, 'foo') - graph.addNode(node) - - expect(graph.topSortWithScc().sorted).toEqual([node]) - expect(graph.topSortWithScc().cycled).toEqual([]) - }) + expect(() => { + graph.addEdge(new IdentifiableString(0, 'origin'), node) + }).toThrowError(/Unknown node/) + }) - it('#topologicalSort for simple graph', () => { - const graph = new Graph(dummyGetDependenciesQuery) - const node0 = identifiableString(0, 'foo') - const node1 = identifiableString(1, 'bar') - graph.addNode(node0) - graph.addNode(node1) - graph.addEdge(node1, node0) + it('throws error when the target node is not present', () => { + const graph = new Graph(dummyDependencyQuery) + const node = new IdentifiableString(0, 'origin') + graph.addNodeAndReturnId(node) - expect(graph.topSortWithScc().sorted).toEqual([node1, node0]) - expect(graph.topSortWithScc().cycled).toEqual([]) + expect(() => { + graph.addEdge(node, new IdentifiableString(1, 'target')) + }).toThrowError(/Unknown node/) + }) }) - it('#topologicalSort for more complex graph', () => { - const graph = new Graph(dummyGetDependenciesQuery) - const node0 = identifiableString(0, 'x0') - const node1 = identifiableString(1, 'x1') - const node2 = identifiableString(2, 'x2') - const node3 = identifiableString(3, 'x3') - const node4 = identifiableString(4, 'x4') - graph.addNode(node0) - graph.addNode(node1) - graph.addNode(node2) - graph.addNode(node3) - graph.addNode(node4) - graph.addEdge(node0, node2) - graph.addEdge(node1, node2) - graph.addEdge(node2, node3) - graph.addEdge(node4, node3) - - expect(graph.topSortWithScc().sorted).toEqual([node0, node1, node2, node4, node3]) - expect(graph.topSortWithScc().cycled).toEqual([]) - }) + describe('removeEdge', () => { + it('throws error when source node does not exist', () => { + const graph = new Graph(dummyDependencyQuery) + const node0 = new IdentifiableString(0, 'x0') + const node1 = new IdentifiableString(1, 'x1') + graph.addNodeAndReturnId(node1) - it('#topologicalSort for not connected graph', () => { - const graph = new Graph(dummyGetDependenciesQuery) - const node0 = identifiableString(0, 'x0') - const node1 = identifiableString(1, 'x1') - const node2 = identifiableString(2, 'x2') - const node3 = identifiableString(3, 'x3') - graph.addNode(node0) - graph.addNode(node1) - graph.addNode(node2) - graph.addNode(node3) - graph.addEdge(node0, node2) - graph.addEdge(node1, node3) - - expect(graph.topSortWithScc().sorted).toEqual([node0, node1, node2, node3]) - expect(graph.topSortWithScc().cycled).toEqual([]) - }) + expect(() => graph.removeEdge(node0, node1)).toThrowError(/Unknown node/) + }) - it('#topologicalSort returns vertices on trivial cycle', () => { - const graph = new Graph(dummyGetDependenciesQuery) - const node0 = identifiableString(0, 'x0') - const node1 = identifiableString(1, 'x1') - graph.addNode(node0) - graph.addNode(node1) - graph.addEdge(node0, node1) - graph.addEdge(node1, node0) - - expect(graph.topSortWithScc().sorted).toEqual([]) - expect(new Set(graph.topSortWithScc().cycled)).toEqual(new Set([node0, node1])) - }) + it('throws error when target node does not exist', () => { + const graph = new Graph(dummyDependencyQuery) + const node0 = new IdentifiableString(0, 'x0') + const node1 = new IdentifiableString(1, 'x1') + graph.addNodeAndReturnId(node0) - it('#topologicalSort returns vertices on cycle', () => { - const graph = new Graph(dummyGetDependenciesQuery) - const node0 = identifiableString(0, 'x0') - const node1 = identifiableString(1, 'x1') - const node2 = identifiableString(2, 'x2') - graph.addNode(node0) - graph.addNode(node1) - graph.addNode(node2) - graph.addEdge(node0, node1) - graph.addEdge(node1, node2) - graph.addEdge(node1, node1) - - expect(graph.topSortWithScc().sorted).toEqual([node0, node2]) - expect(graph.topSortWithScc().cycled).toEqual([node1]) - }) + expect(() => graph.removeEdge(node0, node1)).toThrowError(/Unknown node/) + }) - it('#topologicalSort returns one-element cycle', () => { - const graph = new Graph(dummyGetDependenciesQuery) - const node = identifiableString(0, 'foo') - graph.addNode(node) - graph.addEdge(node, node) + it('throws error when edge does not exist', () => { + const graph = new Graph(dummyDependencyQuery) + const node0 = new IdentifiableString(0, 'x0') + const node1 = new IdentifiableString(1, 'x1') + graph.addNodeAndReturnId(node0) + graph.addNodeAndReturnId(node1) - expect(graph.topSortWithScc().sorted).toEqual([]) - expect(graph.topSortWithScc().cycled).toEqual([node]) - }) -}) + expect(() => graph.removeEdge(node0, node1)).toThrowError('Edge does not exist') + }) -describe('Graph#getTopologicallySortedSubgraphFrom', () => { - it('case without edges', () => { - const graph = new Graph(dummyGetDependenciesQuery) - const node0 = 'foo' - const node1 = 'bar' - graph.addNode(node0) - graph.addNode(node1) + it('removes an edge it it exists', () => { + const graph = new Graph(dummyDependencyQuery) + const node0 = new IdentifiableString(0, 'x0') + const node1 = new IdentifiableString(1, 'x1') - const fn = jasmine.createSpy().and.returnValue(true) - const fn2 = jasmine.createSpy() + graph.addNodeAndReturnId(node0) + graph.addNodeAndReturnId(node1) + expect(graphEdgesCount(graph)).toEqual(0) - graph.getTopSortedWithSccSubgraphFrom([node0], fn, fn2) + graph.addEdge(node0, node1) + expect(graphEdgesCount(graph)).toEqual(1) + expect(graph.existsEdge(node0, node1)).toBe(true) - expect(fn).toHaveBeenCalledTimes(1) - expect(fn.calls.argsFor(0)).toContain(node0) + graph.removeEdge(node0, node1) + expect(graphEdgesCount(graph)).toEqual(0) + expect(graph.existsEdge(node0, node1)).toBe(false) + }) }) - it('case with obvious edge', () => { - const graph = new Graph(dummyGetDependenciesQuery) - const node0 = 'foo' - const node1 = 'bar' + describe('existsEdge', () => { + it('returns true if edge is present in the graph', () => { + const graph = new Graph(dummyDependencyQuery) + const node0 = new IdentifiableString(0, 'foo') + const node1 = new IdentifiableString(1, 'bar') + graph.addNodeAndReturnId(node0) + graph.addNodeAndReturnId(node1) + graph.addEdge(node0, node1) + + expect(graph.existsEdge(node0, node1)).toBe(true) + }) + + it('returns false if edge is not present', () => { + const graph = new Graph(dummyDependencyQuery) + const node0 = new IdentifiableString(0, 'foo') + const node1 = new IdentifiableString(1, 'bar') + graph.addNodeAndReturnId(node0) + graph.addNodeAndReturnId(node1) + + expect(graph.existsEdge(node0, node1)).toBe(false) + }) + + it('returns false if nodes are not present in the graph', () => { + const graph = new Graph(dummyDependencyQuery) + + expect(graph.existsEdge(new IdentifiableString(0, 'foo'), new IdentifiableString(1, 'bar'))).toBe(false) + }) + }) + + describe('adjacentNodes', () => { + it('returns all target nodes adjacent to the given node', () => { + const graph = new Graph(dummyDependencyQuery) + + const node0 = new IdentifiableString(0, 'foo') + const node1 = new IdentifiableString(1, 'bar') + graph.addNodeAndReturnId(node0) + graph.addNodeAndReturnId(node1) + graph.addEdge(node0, node1) + + expect(graph.adjacentNodes(node0)).toEqual(new Set([node1])) + }) + + it('throws error if the source node is not present in the graph', () => { + const graph = new Graph(dummyDependencyQuery) + + const node0 = new IdentifiableString(0, 'foo') + const node1 = new IdentifiableString(1, 'bar') + graph.addNodeAndReturnId(node0) + graph.addNodeAndReturnId(node1) + graph.addEdge(node0, node1) + + expect(() => graph.adjacentNodes(new IdentifiableString(42, 'baz'))).toThrowError(/Unknown node/) + }) + }) + + describe('adjacentNodesCount', () => { + it('returns number of outgoing edges from a given node', () => { + const graph = new Graph(dummyDependencyQuery) + + const node0 = new IdentifiableString(0, 'foo') + const node1 = new IdentifiableString(1, 'bar') + graph.addNodeAndReturnId(node0) + graph.addNodeAndReturnId(node1) + graph.addEdge(node0, node1) + + expect(graph.adjacentNodesCount(node0)).toEqual(1) + }) + + it('throws error if the source node is not present in the graph', () => { + const graph = new Graph(dummyDependencyQuery) + + const node0 = new IdentifiableString(0, 'foo') + const node1 = new IdentifiableString(1, 'bar') + graph.addNodeAndReturnId(node0) + graph.addNodeAndReturnId(node1) + graph.addEdge(node0, node1) + + expect(() => graph.adjacentNodesCount(new IdentifiableString(42, 'baz'))).toThrowError(/Unknown node/) + }) + }) + + describe('topSortWithScc', () => { + it('works for empty graph', () => { + const graph = new Graph(dummyDependencyQuery) + + expect(graph.topSortWithScc().sorted).toEqual([]) + expect(graph.topSortWithScc().cycled).toEqual([]) + }) + + it('returns isolated vertices', () => { + const graph = new Graph(dummyDependencyQuery) + const node = new IdentifiableString(0, 'foo') + graph.addNodeAndReturnId(node) + + expect(graph.topSortWithScc().sorted).toEqual([node]) + expect(graph.topSortWithScc().cycled).toEqual([]) + }) + + it('returns vertices in order corresponding to the edge direction', () => { + const graph = new Graph(dummyDependencyQuery) + const node0 = new IdentifiableString(0, 'foo') + const node1 = new IdentifiableString(1, 'bar') + graph.addNodeAndReturnId(node0) + graph.addNodeAndReturnId(node1) + graph.addEdge(node1, node0) + + expect(graph.topSortWithScc().sorted).toEqual([node1, node0]) + expect(graph.topSortWithScc().cycled).toEqual([]) + }) + + it('works for 4-edges acyclic graph', () => { + const graph = new Graph(dummyDependencyQuery) + const node0 = new IdentifiableString(0, 'x0') + const node1 = new IdentifiableString(1, 'x1') + const node2 = new IdentifiableString(2, 'x2') + const node3 = new IdentifiableString(3, 'x3') + const node4 = new IdentifiableString(4, 'x4') + graph.addNodeAndReturnId(node0) + graph.addNodeAndReturnId(node1) + graph.addNodeAndReturnId(node2) + graph.addNodeAndReturnId(node3) + graph.addNodeAndReturnId(node4) + graph.addEdge(node0, node2) + graph.addEdge(node1, node2) + graph.addEdge(node2, node3) + graph.addEdge(node4, node3) + + expect(graph.topSortWithScc().sorted).toEqual([node0, node1, node2, node4, node3]) + expect(graph.topSortWithScc().cycled).toEqual([]) + }) + + it('works for a graph with multiple connected components', () => { + const graph = new Graph(dummyDependencyQuery) + const node0 = new IdentifiableString(0, 'x0') + const node1 = new IdentifiableString(1, 'x1') + const node2 = new IdentifiableString(2, 'x2') + const node3 = new IdentifiableString(3, 'x3') + graph.addNodeAndReturnId(node0) + graph.addNodeAndReturnId(node1) + graph.addNodeAndReturnId(node2) + graph.addNodeAndReturnId(node3) + graph.addEdge(node0, node2) + graph.addEdge(node1, node3) + + expect(graph.topSortWithScc().sorted).toEqual([node0, node1, node2, node3]) + expect(graph.topSortWithScc().cycled).toEqual([]) + }) + + it('detects cycles', () => { + const graph = new Graph(dummyDependencyQuery) + const node0 = new IdentifiableString(0, 'x0') + const node1 = new IdentifiableString(1, 'x1') + graph.addNodeAndReturnId(node0) + graph.addNodeAndReturnId(node1) + graph.addEdge(node0, node1) + graph.addEdge(node1, node0) + + expect(graph.topSortWithScc().sorted).toEqual([]) + expect(new Set(graph.topSortWithScc().cycled)).toEqual(new Set([node0, node1])) + }) + + it('detects 1-node cycles', () => { + const graph = new Graph(dummyDependencyQuery) + const node0 = new IdentifiableString(0, 'x0') + const node1 = new IdentifiableString(1, 'x1') + const node2 = new IdentifiableString(2, 'x2') + graph.addNodeAndReturnId(node0) + graph.addNodeAndReturnId(node1) + graph.addNodeAndReturnId(node2) + graph.addEdge(node0, node1) + graph.addEdge(node1, node2) + graph.addEdge(node1, node1) + + expect(graph.topSortWithScc().sorted).toEqual([node0, node2]) + expect(graph.topSortWithScc().cycled).toEqual([node1]) + }) + }) + + describe('getTopSortedWithSccSubgraphFrom', () => { + it('calls the operatingFunction callback for sorted nodes', () => { + const graph = new Graph(dummyDependencyQuery) + const node0 = 'foo' + const node1 = 'bar' + graph.addNodeAndReturnId(node0) + graph.addNodeAndReturnId(node1) + + const operatingFunction = jasmine.createSpy().and.returnValue(true) + const onCycle = jasmine.createSpy() + + graph.getTopSortedWithSccSubgraphFrom([node0], operatingFunction, onCycle) + + expect(operatingFunction).toHaveBeenCalledTimes(1) + expect(operatingFunction.calls.argsFor(0)).toContain(node0) + }) + + it('works for graph with an edge', () => { + const graph = new Graph(dummyDependencyQuery) + const node0 = 'foo' + const node1 = 'bar' + + graph.addNodeAndReturnId(node0) + graph.addNodeAndReturnId(node1) + graph.addEdge(node0, node1) + + const operatingFunction = jasmine.createSpy().and.returnValue(true) + const onCycle = jasmine.createSpy() + + graph.getTopSortedWithSccSubgraphFrom([node0], operatingFunction, onCycle) + + expect(operatingFunction).toHaveBeenCalledTimes(2) + expect(operatingFunction.calls.argsFor(0)).toContain(node0) + expect(operatingFunction.calls.argsFor(1)).toContain(node1) + }) + + it('omits nodes not reachable from the "modifiedNodes" array', () => { + const graph = new Graph(dummyDependencyQuery) + const node0 = 'foo' + const node1 = 'bar' + + graph.addNodeAndReturnId(node0) + graph.addNodeAndReturnId(node1) + graph.addEdge(node0, node1) + + const operatingFunction = jasmine.createSpy().and.returnValue(false) + const onCycle = jasmine.createSpy() + + graph.getTopSortedWithSccSubgraphFrom([node0], operatingFunction, onCycle) + + expect(operatingFunction).toHaveBeenCalledTimes(1) + expect(operatingFunction.calls.argsFor(0)).toContain(node0) + }) + + it('calls the operatingFunction for a node not included but reachable from the "modifiedNodes" array', () => { + const graph = new Graph(dummyDependencyQuery) + const nodes = ['foo', 'bar', 'baz'] + + nodes.forEach((n) => graph.addNodeAndReturnId(n)) + graph.addEdge(nodes[0], nodes[2]) + graph.addEdge(nodes[1], nodes[2]) + + const operatingFunction = jasmine.createSpy().and.callFake((node: string) => node === nodes[0]) + const onCycle = jasmine.createSpy() + + graph.getTopSortedWithSccSubgraphFrom([nodes[0], nodes[1]], operatingFunction, onCycle) + + expect(operatingFunction).toHaveBeenCalledTimes(3) + expect(operatingFunction.calls.argsFor(2)).toContain(nodes[2]) + }) + + it('calls onCycle callback for nodes that are on cycle', () => { + const graph = new Graph(dummyDependencyQuery) + const nodes = ['foo', 'c0', 'c1', 'c2'] - graph.addNode(node0) - graph.addNode(node1) - graph.addEdge(node0, node1) + nodes.forEach((n) => graph.addNodeAndReturnId(n)) + graph.addEdge(nodes[0], nodes[1]) + graph.addEdge(nodes[1], nodes[2]) + graph.addEdge(nodes[2], nodes[3]) + graph.addEdge(nodes[3], nodes[1]) - const fn = jasmine.createSpy().and.returnValue(true) - const fn2 = jasmine.createSpy() + const operatingFunction = jasmine.createSpy().and.returnValue(true) + const onCycle = jasmine.createSpy() + const cycled = graph.getTopSortedWithSccSubgraphFrom([nodes[0]], operatingFunction, onCycle).cycled - graph.getTopSortedWithSccSubgraphFrom([node0], fn, fn2) + expect(operatingFunction).toHaveBeenCalledTimes(1) + expect(onCycle).toHaveBeenCalledTimes(3) + expect(cycled).toEqual(['c0', 'c1', 'c2']) + }) - expect(fn).toHaveBeenCalledTimes(2) - expect(fn.calls.argsFor(0)).toContain(node0) - expect(fn.calls.argsFor(1)).toContain(node1) - }) + it('does not call operatingFunction callback for nodes that are on cycle', () => { + const graph = new Graph(dummyDependencyQuery) + const nodes = ['c0', 'c1', 'c2'] + nodes.forEach((n) => graph.addNodeAndReturnId(n)) + graph.addEdge(nodes[0], nodes[1]) + graph.addEdge(nodes[1], nodes[2]) + graph.addEdge(nodes[2], nodes[0]) - it('it doesnt call other if didnt change', () => { - const graph = new Graph(dummyGetDependenciesQuery) - const node0 = 'foo' - const node1 = 'bar' + const operatingFunction = jasmine.createSpy().and.returnValue(true) + const onCycle = jasmine.createSpy() + const cycled = graph.getTopSortedWithSccSubgraphFrom([nodes[0]], operatingFunction, onCycle).cycled - graph.addNode(node0) - graph.addNode(node1) - graph.addEdge(node0, node1) + expect(operatingFunction).not.toHaveBeenCalled() + expect(cycled).toEqual(['c0', 'c1', 'c2']) + }) - const fn = jasmine.createSpy().and.returnValue(false) - const fn2 = jasmine.createSpy() + it('detects a cycle consisting of nodes not included but reachable from the "modifiedNodes" array', () => { + const graph = new Graph(dummyDependencyQuery) + const nodes = ['foo', 'c0', 'c1', 'c2'] + nodes.forEach((n) => graph.addNodeAndReturnId(n)) + graph.addEdge(nodes[0], nodes[1]) + graph.addEdge(nodes[1], nodes[2]) + graph.addEdge(nodes[2], nodes[3]) + graph.addEdge(nodes[3], nodes[1]) - graph.getTopSortedWithSccSubgraphFrom([node0], fn, fn2) + const operatingFunction = jasmine.createSpy().and.returnValue(true) + const onCycle = jasmine.createSpy() + const cycled = graph.getTopSortedWithSccSubgraphFrom([nodes[0]], operatingFunction, onCycle).cycled - expect(fn).toHaveBeenCalledTimes(1) - expect(fn.calls.argsFor(0)).toContain(node0) + expect(operatingFunction).toHaveBeenCalledTimes(1) + expect(cycled).toEqual(['c0', 'c1', 'c2']) + }) }) - it('does call if some previous vertex marked as changed', () => { - const graph = new Graph(dummyGetDependenciesQuery) - const nodes = ['foo', 'bar', 'baz'] + describe('markNodeAsVolatile', () => { + it('adds a node to volatile nodes array', () => { + const graph = new Graph(dummyDependencyQuery) + const node = new IdentifiableString(0, 'foo') - nodes.forEach((n) => graph.addNode(n)) - graph.addEdge(nodes[0], nodes[2]) - graph.addEdge(nodes[1], nodes[2]) + graph.addNodeAndReturnId(node) + graph.markNodeAsVolatile(node) - const fn = jasmine.createSpy().and.callFake((node: string) => node === nodes[0]) - const fn2 = jasmine.createSpy() + expect(graph.getDirtyAndVolatileNodes()).toEqual([node]) + }) - graph.getTopSortedWithSccSubgraphFrom([nodes[0], nodes[1]], fn, fn2) + it('does nothing if node is not in a graph', () => { + const graph = new Graph(dummyDependencyQuery) + const node = new IdentifiableString(0, 'foo') - expect(fn).toHaveBeenCalledTimes(3) - expect(fn.calls.argsFor(2)).toContain(nodes[2]) + graph.markNodeAsVolatile(node) + expect(graph.getDirtyAndVolatileNodes()).toEqual([]) + }) }) - it('returns cycled vertices', () => { - const graph = new Graph(dummyGetDependenciesQuery) - const nodes = ['foo', 'c0', 'c1', 'c2'] + describe('markNodeAsChangingWithStructure', () => { + it('adds a node to special nodes structural changes array', () => { + const graph = new Graph(dummyDependencyQuery) + const node = new IdentifiableString(0, 'foo') - nodes.forEach((n) => graph.addNode(n)) - graph.addEdge(nodes[0], nodes[1]) - graph.addEdge(nodes[1], nodes[2]) - graph.addEdge(nodes[2], nodes[3]) - graph.addEdge(nodes[3], nodes[1]) + graph.addNodeAndReturnId(node) + graph.markNodeAsChangingWithStructure(node) + graph.markChangingWithStructureNodesAsDirty() - const fn = jasmine.createSpy().and.returnValue(true) - const fn2 = jasmine.createSpy() - const cycled = graph.getTopSortedWithSccSubgraphFrom([nodes[0]], fn, fn2).cycled + expect(graph.getDirtyAndVolatileNodes()).toEqual([node]) + }) - expect(fn).toHaveBeenCalledTimes(1) - expect(cycled).toEqual(['c0', 'c1', 'c2']) - }) + it('does nothing if node is not in a graph', () => { + const graph = new Graph(dummyDependencyQuery) + const node = new IdentifiableString(0, 'foo') - it('doesnt call first one of the given vertices if its on cycle', () => { - const graph = new Graph(dummyGetDependenciesQuery) - const nodes = ['c0', 'c1', 'c2'] - nodes.forEach((n) => graph.addNode(n)) - graph.addEdge(nodes[0], nodes[1]) - graph.addEdge(nodes[1], nodes[2]) - graph.addEdge(nodes[2], nodes[0]) + graph.markNodeAsChangingWithStructure(node) + graph.markChangingWithStructureNodesAsDirty() - const fn = jasmine.createSpy().and.returnValue(true) - const fn2 = jasmine.createSpy() - const cycled = graph.getTopSortedWithSccSubgraphFrom([nodes[0]], fn, fn2).cycled - - expect(fn).not.toHaveBeenCalled() - expect(cycled).toEqual(['c0', 'c1', 'c2']) + expect(graph.getDirtyAndVolatileNodes()).toEqual([]) + }) }) - it('returns cycled vertices even if they were not tried to be computed', () => { - const graph = new Graph(dummyGetDependenciesQuery) - const nodes = ['foo', 'c0', 'c1', 'c2'] - nodes.forEach((n) => graph.addNode(n)) - graph.addEdge(nodes[0], nodes[1]) - graph.addEdge(nodes[1], nodes[2]) - graph.addEdge(nodes[2], nodes[3]) - graph.addEdge(nodes[3], nodes[1]) - - const fn = jasmine.createSpy().and.returnValue(true) - const fn2 = jasmine.createSpy() - const cycled = graph.getTopSortedWithSccSubgraphFrom([nodes[0]], fn, fn2).cycled - - expect(fn).toHaveBeenCalledTimes(1) - expect(cycled).toEqual(['c0', 'c1', 'c2']) - }) -}) - -describe('Graph cruds', () => { - it('#removeEdge not existing edge', () => { - const graph = new Graph(dummyGetDependenciesQuery) - const node0 = identifiableString(0, 'x0') - const node1 = identifiableString(1, 'x1') - graph.addNode(node0) - graph.addNode(node1) - - expect(() => graph.removeEdge(node0, node1)).toThrowError('Edge does not exist') - }) - - it('#removeEdge removes edge from graph', () => { - const graph = new Graph(dummyGetDependenciesQuery) - const node0 = identifiableString(0, 'x0') - const node1 = identifiableString(1, 'x1') - - graph.addNode(node0) - graph.addNode(node1) - - graph.addEdge(node0, node1) - expect(graph.edgesCount()).toEqual(1) - expect(graph.existsEdge(node0, node1)).toBe(true) - - graph.removeEdge(node0, node1) - expect(graph.edgesCount()).toEqual(0) - expect(graph.existsEdge(node0, node1)).toBe(false) - }) + describe('markNodeAsInfiniteRange', () => { + it('adds a node to the infinite ranges array', () => { + const graph = new Graph(dummyDependencyQuery) + const node = new IdentifiableString(0, 'foo') - it('#removeIncomingEdges removes all edges incoming to given node', () => { - const graph = new Graph(dummyGetDependenciesQuery) - const node0 = identifiableString(0, 'x0') - const node1 = identifiableString(1, 'x1') - const node2 = identifiableString(1, 'x2') + graph.addNodeAndReturnId(node) + graph.markNodeAsInfiniteRange(node) - graph.addNode(node0) - graph.addNode(node1) - graph.addNode(node2) + expect(graph.getInfiniteRanges().map(({ node }) => node)).toEqual([node]) + }) - graph.addEdge(node1, node0) - graph.addEdge(node2, node0) - expect(graph.edgesCount()).toEqual(2) - expect(graph.existsEdge(node1, node0)).toBe(true) - expect(graph.existsEdge(node2, node0)).toBe(true) + it('does nothing if node is not in a graph', () => { + const graph = new Graph(dummyDependencyQuery) + const node = new IdentifiableString(0, 'foo') - graph.removeIncomingEdges(node0) - expect(graph.edgesCount()).toEqual(0) + graph.markNodeAsInfiniteRange(node) + expect(graph.getInfiniteRanges()).toEqual([]) + }) }) }) diff --git a/test/named-expressions.spec.ts b/test/named-expressions.spec.ts index a5a0524c4..877eea907 100644 --- a/test/named-expressions.spec.ts +++ b/test/named-expressions.spec.ts @@ -695,7 +695,7 @@ describe('Named expressions - evaluation', () => { engine.setCellContents(adr('A1'), '=FOO+10') - const fooVertex = engine.dependencyGraph.fetchNamedExpressionVertex('FOO', 0) + const fooVertex = engine.dependencyGraph.fetchNamedExpressionVertex('FOO', 0).vertex const a1 = engine.dependencyGraph.fetchCell(adr('A1')) expect(engine.graph.existsEdge(fooVertex, a1)).toBe(true) expect(engine.getCellValue(adr('A1'))).toEqual(52) @@ -716,7 +716,7 @@ describe('Named expressions - evaluation', () => { engine.addNamedExpression('FOO', '=42') - const fooVertex = engine.dependencyGraph.fetchNamedExpressionVertex('FOO', 0) + const fooVertex = engine.dependencyGraph.fetchNamedExpressionVertex('FOO', 0).vertex const a1 = engine.dependencyGraph.fetchCell(adr('A1')) expect(engine.graph.existsEdge(fooVertex, a1)).toBe(true) expect(engine.getCellValue(adr('A1'))).toEqual(42) @@ -739,7 +739,7 @@ describe('Named expressions - evaluation', () => { engine.setCellContents(adr('A1'), null) - const fooVertex = engine.dependencyGraph.fetchNamedExpressionVertex('FOO', 0) + const fooVertex = engine.dependencyGraph.fetchNamedExpressionVertex('FOO', 0).vertex expect(engine.graph.adjacentNodes(fooVertex).size).toBe(0) }) @@ -761,7 +761,7 @@ describe('Named expressions - evaluation', () => { engine.setCellContents(adr('A1'), '=FOO+10') - const localFooVertex = engine.dependencyGraph.fetchNamedExpressionVertex('FOO', 0)! + const localFooVertex = engine.dependencyGraph.fetchNamedExpressionVertex('FOO', 0).vertex const globalFooVertex = engine.dependencyGraph.fetchCell(engine.dependencyGraph.namedExpressions.namedExpressionForScope('FOO')!.address) const a1 = engine.dependencyGraph.fetchCell(adr('A1')) expect(engine.graph.existsEdge(localFooVertex, a1)).toBe(true) @@ -938,8 +938,8 @@ describe('Named expressions - cross scope', () => { expect(engine.getCellValue(adr('B1', 0))).toBe(null) expect(engine.getCellValue(adr('B1', 1))).toEqual('bar') // ensure edges are correct - const sourceScopeNEVertex = engine.dependencyGraph.fetchNamedExpressionVertex('expr', 0) - const targetScopeNEVertex = engine.dependencyGraph.fetchNamedExpressionVertex('expr', 1) + const sourceScopeNEVertex = engine.dependencyGraph.fetchNamedExpressionVertex('expr', 0).vertex + const targetScopeNEVertex = engine.dependencyGraph.fetchNamedExpressionVertex('expr', 1).vertex const targetFormulaVertex = engine.dependencyGraph.getCell(adr('B1', 1))! expect(engine.dependencyGraph.existsEdge(sourceScopeNEVertex, targetFormulaVertex)).toBe(false) expect(engine.dependencyGraph.existsEdge(targetScopeNEVertex, targetFormulaVertex)).toBe(true) @@ -963,8 +963,8 @@ describe('Named expressions - cross scope', () => { expect(engine.getCellValue(adr('B1', 0))).toBe(null) expect(engine.getCellValue(adr('B1', 1))).toEqual('bar') // ensure edges are correct - const sourceScopeNEVertex = engine.dependencyGraph.fetchNamedExpressionVertex('expr', 0) - const targetScopeNEVertex = engine.dependencyGraph.fetchNamedExpressionVertex('expr', 1) + const sourceScopeNEVertex = engine.dependencyGraph.fetchNamedExpressionVertex('expr', 0).vertex + const targetScopeNEVertex = engine.dependencyGraph.fetchNamedExpressionVertex('expr', 1).vertex const targetFormulaVertex = engine.dependencyGraph.getCell(adr('B1', 1))! expect(engine.dependencyGraph.existsEdge(sourceScopeNEVertex, targetFormulaVertex)).toBe(false) expect(engine.dependencyGraph.existsEdge(targetScopeNEVertex, targetFormulaVertex)).toBe(true) @@ -988,8 +988,8 @@ describe('Named expressions - cross scope', () => { expect(engine.getCellValue(adr('B1', 0))).toEqual('foo') expect(engine.getCellValue(adr('B1', 1))).toEqual('bar') // ensure edges are correct - const sourceScopeNEVertex = engine.dependencyGraph.fetchNamedExpressionVertex('expr', 0) - const targetScopeNEVertex = engine.dependencyGraph.fetchNamedExpressionVertex('expr', 1) + const sourceScopeNEVertex = engine.dependencyGraph.fetchNamedExpressionVertex('expr', 0).vertex + const targetScopeNEVertex = engine.dependencyGraph.fetchNamedExpressionVertex('expr', 1).vertex const targetFormulaVertex = engine.dependencyGraph.getCell(adr('B1', 1))! expect(engine.dependencyGraph.existsEdge(sourceScopeNEVertex, targetFormulaVertex)).toBe(false) expect(engine.dependencyGraph.existsEdge(targetScopeNEVertex, targetFormulaVertex)).toBe(true) diff --git a/test/testUtils.ts b/test/testUtils.ts index 966451a0d..a7b929401 100644 --- a/test/testUtils.ts +++ b/test/testUtils.ts @@ -3,7 +3,7 @@ import {AbsoluteCellRange, AbsoluteColumnRange, AbsoluteRowRange} from '../src/A import {CellError, SimpleCellAddress, simpleCellAddress} from '../src/Cell' import {Config} from '../src/Config' import {DateTimeHelper} from '../src/DateTimeHelper' -import {ArrayVertex, FormulaCellVertex, RangeVertex} from '../src/DependencyGraph' +import {ArrayVertex, FormulaCellVertex, Graph, RangeVertex} from '../src/DependencyGraph' import {ErrorMessage} from '../src/error-message' import {defaultStringifyDateTime} from '../src/format/format' import {complex} from '../src/interpreter/ArithmeticHelper' @@ -71,7 +71,7 @@ export const verifyRangesInSheet = (engine: HyperFormula, sheet: number, ranges: const rangeVerticesInMapping = Array.from(engine.rangeMapping.rangesInSheet(sheet)) .map((vertex) => rangeAddr(vertex.range)) - const rangeVerticesInGraph = Array.from(engine.graph.nodes.values()).filter(vertex => vertex instanceof RangeVertex) + const rangeVerticesInGraph = [ ...engine.graph.getNodes()].filter(vertex => vertex instanceof RangeVertex) .map(vertex => rangeAddr((vertex as RangeVertex).range)) expectNoDuplicates(rangeVerticesInGraph) @@ -258,3 +258,23 @@ export function resetSpy(spy: any): void { export function expectCellValueToEqualDate(engine: HyperFormula, cellAddress: SimpleCellAddress, expectedDateString: string) { expect(dateNumberToString(engine.getCellValue(cellAddress), new Config())).toEqual(expectedDateString) } + +/** + * Returns number of edges in graph + * + */ +export function graphEdgesCount(graph: Graph): number { + return (graph as any).nodesSparseArray.reduce((acc: number, node: T, id: number) => + node ? acc + ((graph as any).fixEdgesArrayForNode(id) as number[]).length : acc + , 0) +} + +export function graphReversedAdjacentNodes(graph: Graph, node: T): T[] { + const id = (graph as any).nodesIds.get(node) + + return (graph as any).nodesSparseArray.reduce((acc: number[], sourceNode: T, sourceId: number) => + sourceNode && (graph as any).edgesSparseArray[sourceId].includes(id) + ? [ ...acc, sourceId ] + : acc + , []).map((resultId: number) => (graph as any).nodesSparseArray[resultId]) +} diff --git a/test/volatile-functions.spec.ts b/test/volatile-functions.spec.ts index f0be919f1..2082558a5 100644 --- a/test/volatile-functions.spec.ts +++ b/test/volatile-functions.spec.ts @@ -32,8 +32,8 @@ describe('Interpreter - function RAND', () => { engine.setCellContents(adr('A1'), '=RAND()') - const a1 = engine.addressMapping.getCell(adr('A1')) - expect(engine.dependencyGraph.volatileVertices()).toEqual(new Set([a1])) + const a1 = engine.addressMapping.getCell(adr('A1'))! + expect(engine.dependencyGraph.verticesToRecompute()).toEqual([a1]) }) it('volatile vertices should not be recomputed after removing from graph', () => { @@ -43,7 +43,7 @@ describe('Interpreter - function RAND', () => { engine.setCellContents(adr('A1'), '35') - expect(engine.dependencyGraph.verticesToRecompute()).toEqual(new Set()) + expect(engine.dependencyGraph.verticesToRecompute()).toEqual([]) }) it('volatile formula after moving is still volatile', () => { @@ -53,7 +53,7 @@ describe('Interpreter - function RAND', () => { engine.moveCells(AbsoluteCellRange.spanFrom(adr('A1'), 1, 1), adr('A2')) - const a2 = engine.addressMapping.getCell(adr('A2')) - expect(engine.dependencyGraph.volatileVertices()).toEqual(new Set([a2])) + const a2 = engine.addressMapping.getCell(adr('A2'))! + expect(engine.dependencyGraph.verticesToRecompute()).toEqual([a2]) }) })