From 850ed1565666f9382b6160a60de8d3bd92b0250e Mon Sep 17 00:00:00 2001 From: Naftoli Gugenheim Date: Fri, 3 Aug 2018 07:04:44 -0400 Subject: [PATCH] Refactoring to support parsing choices safely - Sayable.Group, to preserve a grouping - Make Choice an ADT, and move it to top level - Move assigning logic into ChoiceMenu and move it to top level - Factor out choiceMenuSayable - Tested ParseChoices --- core/src/main/scala/simpleivr/Choice.scala | 23 +++++++ .../src/main/scala/simpleivr/ChoiceMenu.scala | 23 +++++++ .../main/scala/simpleivr/DefaultSayer.scala | 2 + .../src/main/scala/simpleivr/IvrChoices.scala | 66 ++++++------------- core/src/main/scala/simpleivr/Sayable.scala | 10 +++ .../scala/simpleivr/ChoiceMenuTests.scala | 23 +++++++ .../test/scala/simpleivr/DummySayables.scala | 4 +- .../scala/simpleivr/IvrChoicesTests.scala | 35 ---------- .../InteractionIvrCommandInterpreter.scala | 33 +--------- .../simpleivr/testing/ParseChoices.scala | 33 ++++++++++ .../simpleivr/testing/ParseChoiceTests.scala | 33 ++++++++++ 11 files changed, 173 insertions(+), 112 deletions(-) create mode 100644 core/src/main/scala/simpleivr/Choice.scala create mode 100644 core/src/main/scala/simpleivr/ChoiceMenu.scala create mode 100644 core/src/test/scala/simpleivr/ChoiceMenuTests.scala delete mode 100644 core/src/test/scala/simpleivr/IvrChoicesTests.scala create mode 100644 testing/src/main/scala/simpleivr/testing/ParseChoices.scala create mode 100644 testing/src/test/scala/simpleivr/testing/ParseChoiceTests.scala diff --git a/core/src/main/scala/simpleivr/Choice.scala b/core/src/main/scala/simpleivr/Choice.scala new file mode 100644 index 0000000..c39399b --- /dev/null +++ b/core/src/main/scala/simpleivr/Choice.scala @@ -0,0 +1,23 @@ +package simpleivr + +sealed trait Choice[+A] { + def maybeDtmf: Option[DTMF] + def label: Sayable + def value: A + def map[B](f: A => B): Choice[B] +} + +object Choice { + case class Unassigned[+A](label: Sayable, value: A) extends Choice[A] { + override def maybeDtmf = None + override def map[B](f: A => B) = copy(value = f(value)) + } + case class Assigned[+A](dtmf: DTMF, label: Sayable, value: A) extends Choice[A] { + override def maybeDtmf = Some(dtmf) + override def map[B](f: A => B) = copy(value = f(value)) + } + def apply[A](label: Sayable, value: A): Choice[A] = Unassigned(label, value) + def apply[A](dtmf: DTMF, label: Sayable, value: A): Choice[A] = Assigned(dtmf, label, value) + + def unapply[A](choice: Choice[A]) = Some((choice.maybeDtmf, choice.label, choice.value)) +} diff --git a/core/src/main/scala/simpleivr/ChoiceMenu.scala b/core/src/main/scala/simpleivr/ChoiceMenu.scala new file mode 100644 index 0000000..700bef5 --- /dev/null +++ b/core/src/main/scala/simpleivr/ChoiceMenu.scala @@ -0,0 +1,23 @@ +package simpleivr + +case class ChoiceMenu[A](title: Sayable, choices: Seq[Choice[A]]) { + private def assignNums(choices: List[Choice[A]], nums: List[DTMF]): List[Choice.Assigned[A]] = choices match { + case Nil => Nil + case (c @ Choice.Assigned(dtmf, _, _)) :: rest => c :: assignNums(rest, nums.filter(_ != dtmf)) + case Choice.Unassigned(label, value) :: rest => + if (nums.nonEmpty) + Choice.Assigned(nums.head, label, value) :: assignNums(rest, nums.tail) + else { + println("ERROR: No num available for choices: " + choices) + assignNums(rest, nums) + } + } + + lazy val assigned = + assignNums(choices.toList, DTMF.digits.toList.filterNot(n => choices exists (_.maybeDtmf contains n))) +} + +object ChoiceMenu { + def apply[A](title: Sayable, dummy: Null = null)(choices: Choice[A]*): ChoiceMenu[A] = + new ChoiceMenu(title, choices) +} diff --git a/core/src/main/scala/simpleivr/DefaultSayer.scala b/core/src/main/scala/simpleivr/DefaultSayer.scala index 29cfba6..0e7b4e2 100644 --- a/core/src/main/scala/simpleivr/DefaultSayer.scala +++ b/core/src/main/scala/simpleivr/DefaultSayer.scala @@ -32,6 +32,8 @@ class DefaultSayer(interp: IvrCommand.Interpreter[IO], interruptDtmfs: Set[DTMF] .flatMap(play) } + override def group(sayable: Sayable) = apply(sayable) + override def many(sayables: List[Sayable]): IO[Option[DTMF]] = sayables match { case Nil => IO.pure(None) diff --git a/core/src/main/scala/simpleivr/IvrChoices.scala b/core/src/main/scala/simpleivr/IvrChoices.scala index 383e2f9..578d8f4 100644 --- a/core/src/main/scala/simpleivr/IvrChoices.scala +++ b/core/src/main/scala/simpleivr/IvrChoices.scala @@ -9,14 +9,6 @@ class IvrChoices(sayables: Sayables) extends Ivr(sayables) { import sayables._ - case class Choice[+A](key: Option[DTMF], label: Sayable, value: A) { - def map[B](f: A => B): Choice[B] = Choice(key, label, f(value)) - } - object Choice { - def apply[A](label: Sayable, value: A): Choice[A] = new Choice(None, label, value) - def apply[A](key: DTMF, label: Sayable, value: A): Choice[A] = new Choice(Some(key), label, value) - } - def paginated[T](maximum: Int, choices: List[Choice[T]], fixedChoices: List[Choice[T]]): List[Choice[IvrStep[T]]] = { def doPage(page: List[Choice[T]], rest: List[Choice[T]], back: List[Choice[T]]): List[Choice[IvrStep[T]]] = { def askNextPage: IvrStep[T] = Free.defer { @@ -47,57 +39,39 @@ class IvrChoices(sayables: Sayables) extends Ivr(sayables) { doPage(choices.take(maximum - 1), choices.drop(maximum - 1), Nil) } - def assignNums[A](choices: List[Choice[A]]): List[Choice[A]] = { - val unused = DTMF.digits.toList.filterNot(n => choices exists (_.key contains n)) - println("assignNums: unused: " + unused.mkString) - for (c <- choices) println(s" ${c.key} / ${c.label}") - assignNums[A](choices, unused) - } - def assignNums[A](choices: List[Choice[A]], nums: List[DTMF]): List[Choice[A]] = choices match { - case Nil => Nil - case (c @ Choice(Some(n), _, _)) :: rest => c :: assignNums(rest, nums.filter(_ != n)) - case c :: rest => - if (nums.nonEmpty) - c.copy(key = Some(nums.head)) :: assignNums(rest, nums.tail) - else { - println("ERROR: No num available for choices: " + choices) - c :: rest - } - } - - case class ChoiceMenu[A](title: Sayable, choices: Seq[Choice[A]]) - object ChoiceMenu { - def apply[A](title: Sayable, dummy: Null = null)(choices: Choice[A]*): ChoiceMenu[A] = - new ChoiceMenu(title, choices) - } - sealed class SayChoice(val func: (DTMF, Sayable) => Sayable) object SayChoice { case object LabelFirst extends SayChoice((key, label) => label & `Press` & dtmfWord(key)) case object LabelLast extends SayChoice((key, label) => `Press` & dtmfWord(key) & label) } + def choiceMenuSayable[A](choiceMenu: ChoiceMenu[A], pauseMs: Int, sayChoice: SayChoice) = + choiceMenu.title & + Sayable.Many( + choiceMenu.assigned.map { + case Choice.Assigned(key, label, _) => + Sayable.Group(Pause(pauseMs) & sayChoice.func(key, label)) + } + ) + protected def defaultAskChoicePauseMs: Int = 750 protected def defaultAskChoiceSayChoice: SayChoice = SayChoice.LabelLast def askChoice[A](choiceMenu: ChoiceMenu[A], pauseMs: Int = defaultAskChoicePauseMs, sayChoice: SayChoice = defaultAskChoiceSayChoice): IvrStep[A] = { - val menu = assignNums(choiceMenu.choices.toList) - val menuMsgs = menu.collect { - case Choice(Some(key), label, _) => Pause(pauseMs) & sayChoice.func(key, label) - } - if (menuMsgs.length < menu.length) - Console.err.println(s"ERROR: Not all menu choices have keys in ${choiceMenu.title}: $menu") - - def loop: IvrStep[A] = sayAndGetDigit(choiceMenu.title & Sayable.Many(menuMsgs)) flatMap { - case None => IvrStep.say(`Please make a selection` & Pause(pauseMs)) *> loop - case Some(c) => - menu.find(_.key.contains(c)) match { - case Some(choice) => IvrStep(choice.value) - case None => IvrStep.say(`That is not one of the choices.` & Pause(pauseMs)) *> loop + val menuSayable = choiceMenuSayable(choiceMenu, pauseMs, sayChoice) + + def loop: IvrStep[A] = + sayAndGetDigit(menuSayable) + .flatMap { + case None => IvrStep.say(`Please make a selection` & Pause(pauseMs)) *> loop + case Some(c) => + choiceMenu.assigned.find(_.maybeDtmf.contains(c)) match { + case Some(choice) => IvrStep(choice.value) + case None => IvrStep.say(`That is not one of the choices.` & Pause(pauseMs)) *> loop + } } - } loop } diff --git a/core/src/main/scala/simpleivr/Sayable.scala b/core/src/main/scala/simpleivr/Sayable.scala index 84bf026..56da800 100644 --- a/core/src/main/scala/simpleivr/Sayable.scala +++ b/core/src/main/scala/simpleivr/Sayable.scala @@ -18,6 +18,7 @@ sealed trait Sayable { final def toSingles: Seq[Sayable.Single] = this match { case single: Sayable.Single => Seq(single) case Sayable.Many(sayables) => sayables.flatMap(_.toSingles) + case Sayable.Group(sayable) => sayable.toSingles } override def toString = toSingles @@ -30,10 +31,17 @@ sealed trait Sayable { } object Sayable { sealed trait Single extends Sayable + case class Group(sayable: Sayable) extends Sayable case class Many(sayables: Seq[Sayable]) extends Sayable val Empty: Sayable = Many(Nil) + def apply(sayables: Seq[Sayable]) = sayables match { + case Seq() => Empty + case Seq(single) => single + case many => Many(many) + } + def unapplySeq(sayable: Sayable): Option[Seq[Any]] = Some(sayable.toSingles.map { case speak: Speaks#Speak => speak.msg case s => s @@ -44,12 +52,14 @@ object Sayable { case Pause(ms) => pause(ms) case Play(path) => play(path) case s: Speaks#Speak => speak(s) + case Group(s) => group(s) case Many(sayables) => many(sayables.toList) } def pause(ms: Int): A def play(path: AudioPath): A def speak(spk: Speaks#Speak): A + def group(sayable: Sayable): A def many(sayables: List[Sayable]): A } } diff --git a/core/src/test/scala/simpleivr/ChoiceMenuTests.scala b/core/src/test/scala/simpleivr/ChoiceMenuTests.scala new file mode 100644 index 0000000..2595c72 --- /dev/null +++ b/core/src/test/scala/simpleivr/ChoiceMenuTests.scala @@ -0,0 +1,23 @@ +package simpleivr + +import org.scalatest.{FunSuite, Matchers} + + +class ChoiceMenuTests extends FunSuite with Matchers { + test("assigned") { + def defined(key: DTMF) = Choice(key, Sayable.Empty, ()) + + def auto = Choice(Sayable.Empty, ()) + + val choices = List(defined(DTMF.`1`), auto, defined(DTMF.`2`), auto, auto, auto, auto, auto, auto, auto) + + val choiceMenu = ChoiceMenu(Sayable.Empty, choices) + + val actual = choiceMenu.assigned.map(_.dtmf) + + val expected = + List(DTMF.`1`, DTMF.`3`, DTMF.`2`, DTMF.`4`, DTMF.`5`, DTMF.`6`, DTMF.`7`, DTMF.`8`, DTMF.`9`, DTMF.`0`) + + actual shouldBe expected + } +} diff --git a/core/src/test/scala/simpleivr/DummySayables.scala b/core/src/test/scala/simpleivr/DummySayables.scala index ab19802..cd89509 100644 --- a/core/src/test/scala/simpleivr/DummySayables.scala +++ b/core/src/test/scala/simpleivr/DummySayables.scala @@ -3,7 +3,9 @@ package simpleivr import java.nio.file.Files -object DummySayables extends Sayables({ +class DummySayables extends Sayables({ val dir = Files.createTempDirectory("simpleivr_DummySayables") new LocalAudioFileBackend(dir, dir.toString) }) + +object DummySayables extends DummySayables diff --git a/core/src/test/scala/simpleivr/IvrChoicesTests.scala b/core/src/test/scala/simpleivr/IvrChoicesTests.scala deleted file mode 100644 index 507e775..0000000 --- a/core/src/test/scala/simpleivr/IvrChoicesTests.scala +++ /dev/null @@ -1,35 +0,0 @@ -package simpleivr - -import org.scalatest.{FunSuite, Matchers} - - -class IvrChoicesTests extends FunSuite with Matchers { - test("assignNums") { - val ivrChoices = new IvrChoices(DummySayables) - import ivrChoices._ - - def defined(key: DTMF) = Choice(key, Sayable.Empty, ()) - - def auto = Choice(Sayable.Empty, ()) - - val actual = - assignNums(List(defined(DTMF.`1`), auto, defined(DTMF.`2`), auto, auto, auto, auto, auto, auto, auto)) - .map(_.key) - - val expected = - List( - Some(DTMF.`1`), - Some(DTMF.`3`), - Some(DTMF.`2`), - Some(DTMF.`4`), - Some(DTMF.`5`), - Some(DTMF.`6`), - Some(DTMF.`7`), - Some(DTMF.`8`), - Some(DTMF.`9`), - Some(DTMF.`0`) - ) - - actual shouldBe expected - } -} diff --git a/testing/src/main/scala/simpleivr/testing/InteractionIvrCommandInterpreter.scala b/testing/src/main/scala/simpleivr/testing/InteractionIvrCommandInterpreter.scala index 5de67e1..b90cd2c 100644 --- a/testing/src/main/scala/simpleivr/testing/InteractionIvrCommandInterpreter.scala +++ b/testing/src/main/scala/simpleivr/testing/InteractionIvrCommandInterpreter.scala @@ -1,10 +1,10 @@ package simpleivr.testing -import simpleivr.{DTMF, Pause, Sayable} +import simpleivr.{DTMF, Sayable} class InteractionIvrCommandInterpreter(var interactions: List[Interaction], override val callerId: String = null) - (checkIgnored: Int => Unit) extends NullIvrCommandInterpreter { + (checkIgnored: Int => Unit) extends NullIvrCommandInterpreter with ParseChoices { private[this] var ignoredCount = 0 private def modHead[A](pf: PartialFunction[Interaction, (A, Option[Interaction])]): Option[A] = { @@ -40,36 +40,11 @@ class InteractionIvrCommandInterpreter(var interactions: List[Interaction], over case Interaction.Press(Nil) => None -> None } - private object digit { - def unapply(sayable: Sayable): Option[DTMF] = sayable match { - case Sayable("one") => Some(DTMF.`1`) - case Sayable("two") => Some(DTMF.`2`) - case Sayable("three") => Some(DTMF.`3`) - case Sayable("four") => Some(DTMF.`4`) - case Sayable("five") => Some(DTMF.`5`) - case Sayable("six") => Some(DTMF.`6`) - case Sayable("seven") => Some(DTMF.`7`) - case Sayable("eight") => Some(DTMF.`8`) - case Sayable("nine") => Some(DTMF.`9`) - case Sayable("zero") => Some(DTMF.`0`) - case Sayable("pound") => Some(DTMF.`#`) - case Sayable("star") => Some(DTMF.*) - } - } - - private def toChoices(sayables: List[Sayable.Single]): List[(DTMF, Sayable)] = - Util.spans(sayables) { case Pause(_) => true case _ => false } - .flatMap { - case Sayable("Press") +: digit(d) +: label => Some(d -> Sayable.Many(label)) - case label :+ Sayable("Press") :+ digit(d) => Some(d -> Sayable.Many(label)) - case other => None - } - override def waitForDigit(timeout: Int): Option[DTMF] = modHead(takeInput()).flatten override def say(sayable: Sayable, interruptDtmfs: Set[DTMF]): Option[DTMF] = { object choice { def unapply(text: String): Option[DTMF] = - toChoices(sayable.toSingles.toList) + parseChoices(sayable) .find(_._2.toString().contains(text)) .map(_._1) } @@ -86,5 +61,3 @@ class InteractionIvrCommandInterpreter(var interactions: List[Interaction], over 0 } } - - diff --git a/testing/src/main/scala/simpleivr/testing/ParseChoices.scala b/testing/src/main/scala/simpleivr/testing/ParseChoices.scala new file mode 100644 index 0000000..eff8d3a --- /dev/null +++ b/testing/src/main/scala/simpleivr/testing/ParseChoices.scala @@ -0,0 +1,33 @@ +package simpleivr.testing + +import simpleivr.Sayable.{Group, Many, Single} +import simpleivr.{DTMF, Pause, Sayable} + + +trait ParseChoices { + private object digit { + def unapply(sayable: Sayable): Option[DTMF] = sayable match { + case Sayable("one") => Some(DTMF.`1`) + case Sayable("two") => Some(DTMF.`2`) + case Sayable("three") => Some(DTMF.`3`) + case Sayable("four") => Some(DTMF.`4`) + case Sayable("five") => Some(DTMF.`5`) + case Sayable("six") => Some(DTMF.`6`) + case Sayable("seven") => Some(DTMF.`7`) + case Sayable("eight") => Some(DTMF.`8`) + case Sayable("nine") => Some(DTMF.`9`) + case Sayable("zero") => Some(DTMF.`0`) + case Sayable("pound") => Some(DTMF.`#`) + case Sayable("star") => Some(DTMF.*) + } + } + + def parseChoices(sayable: Sayable): List[(DTMF, Sayable)] = + sayable match { + case s: Single => Nil + case Many(Pause(_) +: Sayable("Press") +: digit(d) +: label) => List(d -> Sayable(label)) + case Many(Pause(_) +: label :+ Sayable("Press") :+ digit(d)) => List(d -> Sayable(label)) + case Many(sayables) => sayables.flatMap(parseChoices).toList + case Group(s) => parseChoices(s) + } +} diff --git a/testing/src/test/scala/simpleivr/testing/ParseChoiceTests.scala b/testing/src/test/scala/simpleivr/testing/ParseChoiceTests.scala new file mode 100644 index 0000000..fd6b909 --- /dev/null +++ b/testing/src/test/scala/simpleivr/testing/ParseChoiceTests.scala @@ -0,0 +1,33 @@ +package simpleivr.testing + +import org.scalatest.FunSuite +import simpleivr._ + + +class ParseChoiceTests extends FunSuite with ParseChoices { + test("parseChoices") { + object S extends DummySayables { + val `do one thing`, `do something else` = speak + } + + val ivrChoices = new IvrChoices(S) + import ivrChoices._ + + val choices = + List( + Choice.Assigned(DTMF.`1`, S.`do one thing`, ()), + Choice.Assigned(DTMF.`2`, S.`do something else`, ()) + ) + + val choiceMenu = ChoiceMenu(Sayable.Empty, choices) + val pauseMs = 750 + val labelFirstParsed = parseChoices(choiceMenuSayable(choiceMenu, pauseMs, SayChoice.LabelFirst)) + println(">>") + val labelLastParsed = parseChoices(choiceMenuSayable(choiceMenu, pauseMs, SayChoice.LabelLast)) + println("<<") + + assert(labelFirstParsed == labelLastParsed) + + assert(labelFirstParsed == choices.map(c => c.dtmf -> c.label)) + } +}