Fix cached candidate server api error-handling bug in sdk update (#749)

* Fix cached candidate server api error-handling bug
* Simplify code in update scripts

Author: rgoldberg

src/main/bash/sdkman-update.sh

@@ -21,26 +21,11 @@ function __sdk_update() {
 	__sdkman_echo_debug "Using candidates endpoint: $candidates_uri"
 
 	local fresh_candidates_csv=$(__sdkman_secure_curl_with_timeouts "$candidates_uri")
-	local detect_html="$(echo "$fresh_candidates" | tr '[:upper:]' '[:lower:]' | grep 'html')"
-
-	local fresh_candidates=("")
-	local cached_candidates=("")
-
-	if [[ "$zsh_shell" == 'true' ]]; then
-		fresh_candidates=( ${(s:,:)fresh_candidates_csv} )
-		cached_candidates=( ${(s:,:)SDKMAN_CANDIDATES_CSV} )
-	else
-		OLD_IFS="$IFS"
-		IFS=","
-		fresh_candidates=(${fresh_candidates_csv})
-		cached_candidates=(${SDKMAN_CANDIDATES_CSV})
-		IFS="$OLD_IFS"
-	fi
 
 	__sdkman_echo_debug "Local candidates: $SDKMAN_CANDIDATES_CSV"
 	__sdkman_echo_debug "Fetched candidates: $fresh_candidates_csv"
 
-	if [[ -n "$fresh_candidates_csv" && -z "$detect_html" ]]; then
+	if [[ -n "${fresh_candidates_csv}" ]] && ! grep -iq 'html' <<< "${fresh_candidates_csv}"; then
 		# legacy bash workaround
 		if [[ "$bash_shell" == 'true' && "$BASH_VERSINFO" -lt 4 ]]; then
 			__sdkman_legacy_bash_message
@@ -50,9 +35,15 @@ function __sdk_update() {
 
 		__sdkman_echo_debug "Fresh and cached candidate lengths: ${#fresh_candidates_csv} ${#SDKMAN_CANDIDATES_CSV}"
 
-		local combined_candidates diff_candidates
+		local fresh_candidates combined_candidates diff_candidates
+
+		if [[ "${zsh_shell}" == 'true' ]]; then
+			fresh_candidates=(${(s:,:)fresh_candidates_csv})
+		else
+			IFS=',' read -a fresh_candidates <<< "${fresh_candidates_csv}"
+		fi
 
-		combined_candidates=("${fresh_candidates[@]}" "${cached_candidates[@]}")
+		combined_candidates=("${fresh_candidates[@]}" "${SDKMAN_CANDIDATES[@]}")
 
 		diff_candidates=($(printf $'%s\n' "${combined_candidates[@]}" | sort | uniq -u))
 
@@ -65,7 +56,7 @@ function __sdk_update() {
 				__sdkman_echo_green "\nAdding new candidates(s): ${delta[*]}"
 			fi
 
-			delta=("${cached_candidates[@]}" "${diff_candidates[@]}")
+			delta=("${SDKMAN_CANDIDATES[@]}" "${diff_candidates[@]}")
 			delta=($(printf $'%s\n' "${delta[@]}" | sort | uniq -d))
 			if ((${#delta[@]})); then
 				__sdkman_echo_green "\nRemoving obsolete candidates(s): ${delta[*]}"

src/test/groovy/sdkman/specs/CandidatesCacheUpdateFailureSpec.groovy

@@ -0,0 +1,37 @@
+package sdkman.specs
+
+import sdkman.support.SdkmanEnvSpecification
+
+class CandidatesCacheUpdateFailureSpec extends SdkmanEnvSpecification {
+
+	static final String CANDIDATES_API = "http://localhost:8080/2"
+
+	static final String BROADCAST_API_LATEST_ID_ENDPOINT = "$CANDIDATES_API/broadcast/latest/id"
+	static final String CANDIDATES_ALL_ENDPOINT = "$CANDIDATES_API/candidates/all"
+
+	File candidatesCache
+
+	def setup() {
+		candidatesCache = new File("${sdkmanDotDirectory}/var", "candidates")
+		curlStub.primeWith(BROADCAST_API_LATEST_ID_ENDPOINT, "echo dbfb025be9f97fda2052b5febcca0155")
+				.primeWith(CANDIDATES_ALL_ENDPOINT, "echo html")
+		sdkmanBashEnvBuilder.withConfiguration("sdkman_debug_mode", "true")
+	}
+
+	void "should not update candidates if error downloading candidate list"() {
+		given:
+		bash = sdkmanBashEnvBuilder
+				.withCandidates([])
+				.build()
+
+		and:
+		bash.start()
+
+		when:
+		bash.execute("source $bootstrapScript")
+		bash.execute("sdk update")
+
+		then:
+		!bash.output.contains('Fresh and cached candidate lengths')
+	}
+}