From 519ea2fbd0a1451d7b2fb2c9e0e5679491d747a6 Mon Sep 17 00:00:00 2001
From: Miki Leskinen <miki.leskinen@reaktor.fi>
Date: Fri, 10 Jan 2014 20:51:07 +0200
Subject: [PATCH] Remove everything related to isValidValue from Bacon.retry

Similar effect can (and should) be implemented in the client code,
by transforming the source stream as suggested by @jonifreeman:

Bacon.retry({source: someSource.flatMap(function(x) { return x === "FOO" ? Bacon.error("invalid value") : Bacon.once(x) })})
---
 spec/BaconSpec.coffee | 55 +++++++++++++------------------------------
 src/Bacon.coffee      | 22 ++++-------------
 2 files changed, 20 insertions(+), 57 deletions(-)

diff --git a/spec/BaconSpec.coffee b/spec/BaconSpec.coffee
index 009f72a81..fc2ad8563 100644
--- a/spec/BaconSpec.coffee
+++ b/spec/BaconSpec.coffee
@@ -2598,6 +2598,15 @@ describe "combineTemplate", ->
     expect(Bacon.combineTemplate({ thing: Bacon.never(), const: "a" }).toString()).to.equal("Bacon.combineTemplate({thing:Bacon.never(),const:a})")
 
 describe "Bacon.retry", ->
+  describe "does not retry after value", ->
+    expectStreamEvents(
+      ->
+        calls = 0
+        source = ->
+          calls += 1
+          Bacon.once({calls})
+        Bacon.retry({source, retries: 2})
+      [calls: 1])
   describe "retries to run the source stream given number of times until it yields a value", ->
     expectStreamEvents(
       ->
@@ -2608,25 +2617,12 @@ describe "Bacon.retry", ->
             Bacon.error()
           else
             Bacon.once({calls})
-        Bacon.retry
-          source: source
-          retries: 2
+        Bacon.retry({source, retries: 5})
       [calls: 3])
   describe "does not change source stream characteristics", ->
     expectStreamEvents(
       -> Bacon.retry(source: -> Bacon.fromArray([3, 1, 2, 1, 3]).skipDuplicates().take(2))
       [3, 1])
-  describe "retries after invalid value", ->
-    expectStreamEvents(
-      ->
-        calls = 0
-        source = ->
-          calls += 1
-          Bacon.once({calls})
-        isValidValue = ({calls}) ->
-          calls > 3
-        Bacon.retry {source, isValidValue, retries: 999}
-      [calls: 4])
   describe "retries after retryable error", ->
     expectStreamEvents(
       ->
@@ -2636,7 +2632,7 @@ describe "Bacon.retry", ->
           Bacon.error({calls})
         isRetryable = ({calls}) ->
           calls < 2
-        Bacon.retry {source, isRetryable, retries: 999}
+        Bacon.retry({source, isRetryable, retries: 5})
       [error(calls: 2)]) # TODO: assert error content
   describe "yields error when no retries left", ->
     expectStreamEvents(
@@ -2647,40 +2643,21 @@ describe "Bacon.retry", ->
           Bacon.error({calls})
         Bacon.retry {source, retries: 2}
       [error(calls: 3)]) # TODO: assert error content
-  describe "yields error when all values are invalid", ->
-    expectStreamEvents(
-      ->
-        calls = 0
-        source = ->
-          calls += 1
-          Bacon.once({calls})
-        isValidValue = -> false
-        Bacon.retry {source, isValidValue, retries: 2}
-      [error(calls: 3)]) # TODO: assert error content
-  describe "survives undefined error", ->
-    expectStreamEvents(
-      -> Bacon.retry(source: -> Bacon.error())
-      [error()])
-  it "allows interval by context", (done) ->
+  it "allows specifying interval by context for each retry", (done) ->
     calls = 0
     contexts = []
     source = ->
       calls += 1
-      if calls < 2
-        Bacon.error({calls})
-      else
-        Bacon.once({calls})
+      Bacon.error({calls})
     interval = (context) ->
       contexts.push(context)
       1
-    isValidValue = ({calls}) ->
-      calls > 2
-    Bacon.retry({source, interval, isValidValue, retries: 3}).onValue (value) ->
+    Bacon.retry({source, interval, retries: 2}).onError (err) ->
       expect(contexts).to.deep.equal [
         {error: {calls: 1}, retriesDone: 0}
-        {value: {calls: 2}, retriesDone: 1}
+        {error: {calls: 2}, retriesDone: 1}
       ]
-      expect(value).to.deep.equal {calls: 3}
+      expect(err).to.deep.equal {calls: 3}
       done()
   it "throws exception if 'source' option is not a function", ->
     expect(-> Bacon.retry(source: "ugh")).to.throw "'source' option has to be a function"
diff --git a/src/Bacon.coffee b/src/Bacon.coffee
index 891a4d4b5..fc2708d30 100644
--- a/src/Bacon.coffee
+++ b/src/Bacon.coffee
@@ -244,32 +244,18 @@ Bacon.retry = (options) ->
   retries = options.retries || 0
   maxRetries = options.maxRetries || retries
   interval = options.interval || -> 0
-  isValidValue = options.isValidValue || -> true
   isRetryable = options.isRetryable || -> true
 
   retry = (context) ->
-    context.retriesDone = maxRetries - retries
-    nextAttemptOptions = {source, retries: retries - 1, maxRetries, interval, isValidValue, isRetryable}
+    nextAttemptOptions = {source, retries: retries - 1, maxRetries, interval, isRetryable}
     Bacon.later(interval(context)).filter(false).concat(Bacon.retry(nextAttemptOptions))
 
-  fromValue = (v) ->
-    if isValidValue(v)
-      Bacon.once(v)
-    else if retries > 0
-      retry(value: v)
-    else
-      Bacon.error({noRetriesLeft: true, value: v})
-
-  fromError = (e) ->
-    if e?.noRetriesLeft
-      Bacon.error(e)
-    else if isRetryable(e) && retries > 0
-      retry(error: e)
+  source().flatMapError (e) ->
+    if isRetryable(e) && retries > 0
+      retry(error: e, retriesDone: maxRetries - retries)
     else
       Bacon.error(e)
 
-  source().flatMap(fromValue).flatMapError(fromError)
-
 
 eventIdCounter = 0