using redis to store fetched state to ensure no versions are skipped

Author: dennisjbell

assets/check

@@ -1,7 +1,7 @@
 #!/bin/bash
 # vim: set ft=sh
 
-set -ex
+set -e
 
 exec 3>&1 # make stdout available as fd 3 for the result
 exec 1>&2 # redirect all output to stderr for logging
@@ -87,13 +87,18 @@ get_branch_refs() {
   local branch_ref
   local branch
 
+  local ancestry_prefix="${redis_prefix:+"${redis_prefix}:"}ancestry:"
+  local fetched_prefix="${redis_prefix:+"${redis_prefix}:"}fetched:"
+
+
   # Get the redis connection options
   if [ -n "$redis_host" ] ; then
     db_opt="${redis_dbnum:+" -n $redis_dbnum"}"
     pw_opt="${redis_password:+" -a '$redis_password'"}"
-    key="${redis_prefix:+"${redis_prefix}-"}${last_refs}"
+    ancestry_key="${ancestry_prefix}${last_refs%% *}"
+    fetched_key="${fetched_prefix}${last_refs%% *}"
 
-    historic_branches="$(eval "redis-cli -h '${redis_host}'${db_opt}${pw_opt} get '$key'" | jq -Rr .)"
+    historic_branches="$(eval "redis-cli -h '${redis_host}'${db_opt}${pw_opt} get '$ancestry_key'" | jq -Rr .)"
     case $historic_branches in
       *"NOAUTH Authentication required"* )
         echo "Could not authenticate on redis server: $historic_branches" >&2
@@ -105,6 +110,14 @@ get_branch_refs() {
         ;;
     esac
 
+    # Check if the last references was fetched
+    last_ref_fetched="$(eval "redis-cli -h '${redis_host}'${db_opt}${pw_opt} --raw get '$fetched_key'" | jq -Rr .)"
+    if [ -n "$last_refs" -a "$last_ref_fetched" != "true" ]
+    then
+      echo nothing new >&2
+      return 0
+    fi
+
     last_refs="${last_refs}${historic_branches:+" $historic_branches"}"
   fi
 
@@ -150,12 +163,18 @@ INLINE
           echo "$results" | awk '{print $1 ":" "'"$changed_branch${remaining_branches:+" $remaining_branches"}"'" }'
         else
           for ref in $results; do
-            key="${redis_prefix:+"${redis_prefix}-"}${ref}:${changed_branch}"
-            redis_result="$(eval "redis-cli -h '${redis_host}'${db_opt}${pw_opt} set '${key}' '$remaining_branches'")"
+            ancestry_key="${ancestry_prefix}${ref}:${changed_branch}"
+            fetched_key="${fetched_prefix}${ref}:${changed_branch}"
+            redis_result="$(eval "redis-cli -h '${redis_host}'${db_opt}${pw_opt} set '${ancestry_key}' '$remaining_branches'")"
             if [ "${redis_result}" != "OK" ] ; then
               echo "Could not store historic branches: $redis_result" >&2
               exit 1
             fi
+            redis_result="$(eval "redis-cli -h '${redis_host}'${db_opt}${pw_opt} set '${fetched_key}' 'false'")"
+            if [ "${redis_result}" != "OK" ] ; then
+              echo "Could not record ref not fetched: $redis_result" >&2
+              exit 1
+            fi
           done
 
           echo  "$results" | awk '{print $1 ":" "'"$changed_branch"'" }'

assets/in

@@ -32,6 +32,14 @@ depth=$(jq -r '(.params.depth // 0)' < $payload)
 fetch=$(jq -r '(.params.fetch // [])[]' < $payload)
 submodules=$(jq -r '(.params.submodules // "all")' < $payload)
 
+## Redis stuff to support cleaner multibranch wait-for-previous behaviour
+redis_host=$(jq -r '.source.redis.host // ""' < $payload)
+if [ -n "$redis_host" ] ; then
+  redis_password=$(jq -r '.source.redis.password // ""' < $payload)
+  redis_dbnum=$(jq -r '.source.redis.db_number // "0"' < $payload)
+  redis_prefix=$(jq -r '.source.redis.prefix // ""' < $payload)
+fi
+
 [[ -z "$fetch" ]] && single_branch_flag="--single-branch " || single_branch_flag=''
 
 if [ -z "$uri" ]
@@ -101,6 +109,18 @@ do
   fi
 done
 
+if [ -n "$redis_host" ]
+then
+  db_opt="${redis_dbnum:+" -n $redis_dbnum"}"
+  pw_opt="${redis_password:+" -a '$redis_password'"}"
+  key="${redis_prefix:+"${redis_prefix}:"}fetched:${ref}:${branch}"
+  redis_result="$(eval "redis-cli -h '${redis_host}'${db_opt}${pw_opt} set '${key}' 'true'")"
+  if [ "${redis_result}" != "OK" ] ; then
+    echo "Could not record that ref has been fetched: $redis_result" >&2
+    exit 1
+  fi
+fi
+
 new_ref="$(echo "$(git rev-parse HEAD)$ref_extras" | jq -R .)"
 jq -n "{
   version: {ref: $new_ref},

test/check_multi.sh

@@ -220,15 +220,25 @@ it_can_find_successive_branches_with_multiple_commits_with_redis() {
   echo "ref7:another $ref7"
   echo "ref8:another $ref8"
 
+  test_check uri $repo branches '.*' from "$ref1:master $ref2:bogus $ref3:another" redis "testing" | jq -e "
+    . == []
+  "
+
   if [ "$ref6" \< "$ref8" ] ; then
 
     echo "master is least commit"
+
+    echo "---------> TEST M1"
+    set_ref_fetched "testing" $ref1:master
     test_check uri $repo branches '.*' from "$ref1:master $ref2:bogus $ref3:another" redis "testing" | jq -e "
       . == [
         {ref: $(echo "$ref5:master" | jq -R .)},
         {ref: $(echo "$ref6:master" | jq -R .)}
       ]
     "
+
+    echo "---------> TEST M2"
+    set_ref_fetched "testing" $ref6:master
     test_check uri $repo branches '.*' from "$ref6:master" redis "testing" | jq -e "
       . == [
         {ref: $(echo "$ref4:another" | jq -R .)},
@@ -237,6 +247,8 @@ it_can_find_successive_branches_with_multiple_commits_with_redis() {
       ]
     "
 
+    echo "---------> TEST M3"
+    set_ref_fetched "testing" $ref8:another
     test_check uri $repo branches '.*' from "$ref8:another" redis "testing" | jq -e "
       . == []
     "
@@ -249,19 +261,29 @@ it_can_find_successive_branches_with_multiple_commits_with_redis() {
 
   else
     echo "another is least commit"
+
+    echo "---------> TEST A1"
+
+    set_ref_fetched "different-key" $ref1:master
     test_check uri $repo branches '.*' from "$ref1:master $ref2:bogus $ref3:another" redis "different-key" | jq -e "
       . == [
         {ref: $(echo "$ref4:another" | jq -R .)},
         {ref: $(echo "$ref7:another" | jq -R .)},
         {ref: $(echo "$ref8:another" | jq -R .)}
       ]
     "
+
+    echo "---------> TEST A2"
+    set_ref_fetched "different-key" $ref8:another
     test_check uri $repo branches '.*' from "$ref8:another" redis "different-key"| jq -e "
       . == [
         {ref: $(echo "$ref5:master" | jq -R .)},
         {ref: $(echo "$ref6:master" | jq -R .)}
       ]
     "
+
+    echo "---------> TEST A3"
+    set_ref_fetched "different-key" $ref6:master
     test_check uri $repo branches '.*' from "$ref6:master" redis "different-key" | jq -e "
       . == []
     "

test/get.sh

@@ -205,6 +205,46 @@ it_can_get_from_url_at_multibranch_ref() {
 
 }
 
+it_can_get_from_url_at_multibranch_ref_with_redis() {
+
+  set +u
+  if [ "$ENABLE_REDIS_TESTS" != "TRUE" ] ; then
+    echo "Skipping Redis tests because \$ENABLE_REDIS_TESTS not set to 'TRUE'"
+    return 0
+  elif [ "$(redis-cli ping)" != "PONG" ] ; then
+    echo "Skipping Redis tests because redis not installed or not reachable"
+    return 0
+  fi
+  set -u
+
+  local repo=$(init_repo)
+  local ref1=$(make_commit_to_branch $repo branch-a)
+  local ref2=$(make_commit_to_branch $repo branch-b)
+
+  local dest=$TMPDIR/destination
+
+  test_get $dest uri $repo ref "$ref1:branch-a" redis "testing" | jq -e "
+    .version == {ref: $(echo "$ref1:branch-a" | jq -R .)}
+  "
+
+  test_ref_fetched "testing" $ref2:branch-a
+
+  test -e $dest/some-file
+  test "$(git -C $dest rev-parse HEAD)" = $ref1
+
+  rm -rf $dest
+
+  test_get $dest uri $repo ref "$ref2:branch-b" redis "testing" | jq -e "
+    .version == {ref: $(echo "$ref2:branch-b" | jq -R .)}
+  "
+
+  test_ref_fetched "testing" $ref2:branch-b
+
+  test -e $dest/some-file
+  test "$(git -C $dest rev-parse HEAD)" = $ref2
+
+}
+
 run it_can_get_from_url
 run it_can_get_from_url_at_ref
 run it_can_get_from_url_from_a_multibranch_ref
@@ -214,3 +254,4 @@ run it_can_get_multiple_branches_using_fetch
 run it_honors_the_depth_flag
 run it_honors_the_depth_flag_for_submodules
 run it_can_get_from_url_at_multibranch_ref
+run it_can_get_from_url_at_multibranch_ref_with_redis

test/helpers.sh

@@ -282,6 +282,17 @@ test_get() {
         }")"
         shift;;
 
+      "redis" )
+        addition="$(jq -n "{
+          source: {
+            redis: {
+              host: $(echo "localhost" | jq -R '.'),
+              prefix: $(echo "$1" | jq -R '.')
+            }
+          }
+        }")"
+        shift;;
+
       * )
         if [ -z ${destination+is_set} ] ; then
           destination=$arg
@@ -308,9 +319,9 @@ test_get() {
 }
 
 test_redis() {
-  result="$(redis-cli get "${1:+${1}-}$2")"
+  result="$(redis-cli get "${1:+${1}:}ancestry:$2")"
   if [ "$result" != "$3" ] ; then
-    echo "Expected redis-cli get \"${1:+${1}-}$2\" to return:"
+    echo "Expected redis-cli get \"${1:+${1}:}$2\" to return:"
     echo "$3"
     echo ""
     echo "Instead, it returned:"
@@ -320,6 +331,24 @@ test_redis() {
   fi
 }
 
+set_ref_fetched() {
+  redis-cli set "${1:+${1}:}fetched:$2" "${3:-true}"
+}
+
+test_ref_fetched() {
+  expected="${3:-true}"
+  result="$(redis-cli --raw get "${1:+${1}:}fetched:$2")"
+  if [ "$result" != "${expected}" ] ; then
+    echo "Expected redis-cli get \"${1:+${1}:}fetched:$2\" to return:"
+    echo "${expected}"
+    echo ""
+    echo "Instead, it returned:"
+    echo "$result"
+    echo ""
+    return 1
+  fi
+}
+
 test_put() {
   echo ''
 }