Skip to content

Commit 2b60221

Browse files
authored
fix counter-intuitive union choose coalesce and repeat path tracking (#362)
* fix counter-intuitive path tracking of union, choose, coalesce and repeat.
1 parent 84d1530 commit 2b60221

File tree

4 files changed

+61
-10
lines changed

4 files changed

+61
-10
lines changed

traversal-tests/src/test/scala/overflowdb/traversal/PathTraversalTests.scala

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ class PathTraversalTests extends AnyWordSpec with ExampleGraphSetup {
159159
case "L1" => _.out // -> L2
160160
case "R1" => _.repeat(_.out)(_.maxDepth(3)) // -> R4
161161
}.property(Name).path.toSetMutable shouldBe Set(
162-
Seq(r1, r4, "R4"),
162+
Seq(r1, r2, r3, r4, "R4"),
163163
Seq(l1, l2, "L2")
164164
)
165165
}
@@ -177,6 +177,18 @@ class PathTraversalTests extends AnyWordSpec with ExampleGraphSetup {
177177
traversalInvoked shouldBe false
178178
}
179179

180+
"union" in {
181+
centerTrav.enablePathTracking.union(_.out.out).path.toSetMutable shouldBe Set(
182+
Seq(center, l1, l2),
183+
Seq(center, r1, r2)
184+
)
185+
//we can hide internal steps from path-tracking
186+
centerTrav.enablePathTracking.union(t => Traversal.from(t.out.out).iterator).path.toSetMutable shouldBe Set(
187+
Seq(center, l2),
188+
Seq(center, r2)
189+
)
190+
}
191+
180192
}
181193
}
182194

traversal-tests/src/test/scala/overflowdb/traversal/RepeatTraversalTests.scala

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,13 @@ class RepeatTraversalTests extends AnyWordSpec with ExampleGraphSetup {
381381
r1.start.enablePathTracking.repeat(_.out.out)(_.maxDepth(2)).l shouldBe Seq(r5)
382382
r1.start.enablePathTracking.repeat(_.out.out)(_.maxDepth(2)).path.head shouldBe List(r1, r2, r3, r4, r5)
383383
}
384+
"should not forget steps preceding the repeat" in {
385+
centerTrav.enablePathTracking.followedBy.repeat(_.followedBy.followedBy)(_.maxDepth(1)).path.toSetMutable shouldBe Set(
386+
Seq(center, l1, l2, l3),
387+
Seq(center, r1, r2, r3)
388+
)
389+
}
390+
384391
}
385392

386393
}

traversal/src/main/scala/overflowdb/traversal/PathAwareTraversal.scala

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,42 @@ class PathAwareTraversal[+A](val elementsWithPath: IterableOnce[(A, Vector[Any])
4646
override def dedupBy(fun: A => Any): Traversal[A] =
4747
new PathAwareTraversal(elementsWithPathIterator.distinctBy(x => fun(x._1)))
4848

49+
override def union[B](traversals: (Traversal[A] => Traversal[B])*): Traversal[B] =
50+
new PathAwareTraversal(elementsWithPathIterator.flatMap { case (a, p) =>
51+
traversals.iterator.flatMap { inner =>
52+
inner(new PathAwareTraversal(Iterator.single((a, p)))) match {
53+
case stillPathAware: PathAwareTraversal[B] => stillPathAware.elementsWithPathIterator
54+
//do we really want to allow the following, or is it an error?
55+
case notPathAware => notPathAware.iterator.map { (b: B) => (b, p.appended(a)) }
56+
}
57+
}
58+
})
59+
60+
61+
override def choose[BranchOn >: Null, NewEnd]
62+
(on: Traversal[A] => Traversal[BranchOn])
63+
(options: PartialFunction[BranchOn, Traversal[A] => Traversal[NewEnd]]): Traversal[NewEnd] =
64+
new PathAwareTraversal(elementsWithPathIterator.flatMap { case (a, p) =>
65+
val branchOnValue: BranchOn = on(Traversal.fromSingle(a)).headOption.getOrElse(null)
66+
options.applyOrElse(branchOnValue, (failState: BranchOn) => ((unused: Traversal[A]) => Traversal.empty[NewEnd])).apply(new PathAwareTraversal(Iterator.single((a, p)))) match {
67+
case stillPathAware: PathAwareTraversal[NewEnd] => stillPathAware.elementsWithPathIterator
68+
//do we really want to allow the following, or is it an error?
69+
case notPathAware => notPathAware.iterator.map { (b: NewEnd) => (b, p.appended(a)) }
70+
}
71+
})
72+
73+
override def coalesce[NewEnd](options: (Traversal[A] => Traversal[NewEnd])*): Traversal[NewEnd] =
74+
new PathAwareTraversal(elementsWithPathIterator.flatMap { case (a, p) =>
75+
options.iterator.map { inner =>
76+
inner(new PathAwareTraversal(Iterator.single((a, p)))) match {
77+
case stillPathAware: PathAwareTraversal[NewEnd] => stillPathAware.elementsWithPathIterator
78+
//do we really want to allow the following, or is it an error?
79+
case notPathAware => notPathAware.iterator.map { (b: NewEnd) => (b, p.appended(a)) }
80+
}
81+
}.find(_.nonEmpty).getOrElse(Iterator.empty)
82+
})
83+
84+
4985
// TODO add type safety once we're on dotty, similar to gremlin-scala's as/label steps with typelevel append?
5086
override def path: Traversal[Vector[Any]] =
5187
new Traversal(elementsWithPathIterator.map {
@@ -82,8 +118,8 @@ class PathAwareTraversal[+A](val elementsWithPath: IterableOnce[(A, Vector[Any])
82118
val behaviour = behaviourBuilder(new RepeatBehaviour.Builder[B]).build
83119
val _repeatTraversal = repeatTraversal.asInstanceOf[Traversal[B] => Traversal[B]] //this cast usually :tm: safe, because `B` is a supertype of `A`
84120
val repeat0: B => PathAwareTraversal[B] = PathAwareRepeatStep(_repeatTraversal, behaviour)
85-
new PathAwareTraversal(iterator.flatMap { a =>
86-
repeat0(a).elementsWithPath
121+
new PathAwareTraversal(elementsWithPathIterator.flatMap { case (a,p) =>
122+
repeat0(a).elementsWithPathIterator.map{case (b, pp) => (b, p ++ pp)}
87123
})
88124
}
89125

traversal/src/main/scala/overflowdb/traversal/Traversal.scala

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -231,15 +231,11 @@ class Traversal[+A](elements: IterableOnce[A])
231231
*/
232232
@Doc(info = "allows to implement conditional semantics: if, if/else, if/elseif, if/elseif/else, ...")
233233
def choose[BranchOn >: Null, NewEnd]
234-
(on: Traversal[A] => Traversal[BranchOn])
235-
(options: PartialFunction[BranchOn, Traversal[A] => Traversal[NewEnd]]): Traversal[NewEnd] =
234+
(on: Traversal[A] => Traversal[BranchOn])
235+
(options: PartialFunction[BranchOn, Traversal[A] => Traversal[NewEnd]]): Traversal[NewEnd] =
236236
flatMap { (a: A) =>
237237
val branchOnValue: BranchOn = on(Traversal.fromSingle(a)).headOption.getOrElse(null)
238-
if (options.isDefinedAt(branchOnValue)) {
239-
options(branchOnValue)(Traversal.fromSingle(a))
240-
} else {
241-
Traversal.empty
242-
}
238+
options.applyOrElse(branchOnValue, (fail: BranchOn) => ((unused: Traversal[A]) => Traversal.empty[NewEnd])).apply(Traversal.fromSingle(a))
243239
}
244240

245241
/** Branch step: evaluates the provided traversals in order and returns the first traversal that emits at least one element.

0 commit comments

Comments
 (0)