From 20a5cf4c08491aa49ecc385c643237d8adec5a9c Mon Sep 17 00:00:00 2001
From: Nikhil Saraf <1028334+nikhilsaraf@users.noreply.github.com>
Date: Fri, 17 Apr 2020 01:30:36 +0530
Subject: [PATCH] Kelp UI: Fix basepath and use pingURL to ping server in
 tailFile before redirect (#404)

* 1 - show error hint if base unix and native paths are different

* 2 - catch cycles in initialization of kelpos.go

* 3 - fix instance of calling String() on an OSPath

* 4 - incorporate pingURL into tailFileHTML

* 5 - keep debug mode for electron enabled

* 6 - add check for kelp server to be run from root kelp directory in local mode

* 7 - always execute commands from the working directory

* 8 - use only binary directory as basepth in both native and unix formats, eliminating ambiguity

* 9 - working directory command to OS should be specified in Native form

* 10 - fix getBinaryDirectoryUnix by invoking toUnixFilepath before returning

* 11 - kelpBinPath -> kelpBinName

* 12 - added comments related to fix of using kelp binary name instead of absolute path
---
 cmd/server_amd64.go                           |  27 ++++-
 gui/backend/api_server.go                     |  21 +++-
 gui/backend/start_bot.go                      |   3 +
 .../fs_bin_gen/gui/filesystem_vfsdata_dev.go  |   4 +-
 support/kelpos/kelpos.go                      |  19 +++-
 support/kelpos/ospath.go                      |  55 ++++++++--
 support/kelpos/ospath_test.go                 | 102 ++++++++++++++++--
 support/kelpos/process.go                     |   5 +-
 8 files changed, 204 insertions(+), 32 deletions(-)

diff --git a/cmd/server_amd64.go b/cmd/server_amd64.go
index e3b4a6fc1..41ff59f3b 100644
--- a/cmd/server_amd64.go
+++ b/cmd/server_amd64.go
@@ -35,6 +35,7 @@ import (
 	"github.com/stellar/kelp/support/networking"
 	"github.com/stellar/kelp/support/prefs"
 	"github.com/stellar/kelp/support/sdk"
+	"github.com/stellar/kelp/support/utils"
 )
 
 const kelpPrefsDirectory = ".kelp"
@@ -89,6 +90,17 @@ func init() {
 		isLocalMode := env == envDev
 		isLocalDevMode := isLocalMode && *options.dev
 		kos := kelpos.GetKelpOS()
+		if isLocalMode {
+			wd, e := os.Getwd()
+			if e != nil {
+				panic(errors.Wrap(e, "could not get working directory"))
+			}
+			if filepath.Base(wd) != "kelp" {
+				e := fmt.Errorf("need to invoke from the root 'kelp' directory")
+				utils.PrintErrorHintf(e.Error())
+				panic(e)
+			}
+		}
 
 		var logFilepath *kelpos.OSPath
 		if !isLocalDevMode {
@@ -517,7 +529,7 @@ func openElectron(trayIconPath *kelpos.OSPath, url string) {
 			AppIconDefaultPath: "resources/kelp-icon@2x.png",
 			AcceptTCPTimeout:   time.Minute * 2,
 		},
-		Debug: false,
+		Debug: true,
 		Windows: []*bootstrap.Window{&bootstrap.Window{
 			Homepage: url,
 			Options: &astilectron.WindowOptions{
@@ -697,10 +709,21 @@ const tailFileHTML = `<!-- taken from http://www.davejennifer.com/computerjunk/j
 						if (ajax.status == 200 || ajax.status == 206) {
 							if (ajax.responseText.includes("READY_STRING")) {
 								var redirectURL = "REDIRECT_URL";
+								var pingURL = "PING_URL";
 								document.getElementById("theEnd").innerHTML = "<br/><br/><b>redirecting to " + redirectURL + " ...</b><br/><br/>";
 								document.getElementById("theEnd").scrollIntoView();
+
 								// sleep for 2 seconds so the user sees that we are being redirected
-								setTimeout(() => { window.location.href = redirectURL; }, 2000)
+								setTimeout(() => {
+									var ajaxPing = new XMLHttpRequest();
+									ajaxPing.open("GET", pingURL, true);
+									ajaxPing.onreadystatechange = function () {
+										if ((ajaxPing.readyState == 4) && (ajaxPing.status == 200)) {
+											window.location.href = redirectURL;
+										}
+									}
+									ajaxPing.send(null);
+								}, 2000)
 							}
 						}
 					}// ready state 4
diff --git a/gui/backend/api_server.go b/gui/backend/api_server.go
index 8a875e2c0..dd757e1c0 100644
--- a/gui/backend/api_server.go
+++ b/gui/backend/api_server.go
@@ -7,6 +7,7 @@ import (
 	"log"
 	"net/http"
 	"os"
+	"path/filepath"
 
 	"github.com/stellar/go/clients/horizonclient"
 	"github.com/stellar/kelp/support/kelpos"
@@ -15,7 +16,7 @@ import (
 // APIServer is an instance of the API service
 type APIServer struct {
 	basepath          *kelpos.OSPath
-	kelpBinPath       *kelpos.OSPath
+	kelpBinName       string
 	configsDir        *kelpos.OSPath
 	logsDir           *kelpos.OSPath
 	kos               *kelpos.KelpOS
@@ -42,7 +43,7 @@ func MakeAPIServer(
 	noHeaders bool,
 	quitFn func(),
 ) (*APIServer, error) {
-	kelpBinPath := basepath.Join(os.Args[0])
+	kelpBinName := filepath.Base(os.Args[0])
 	configsDir := basepath.Join("ops", "configs")
 	logsDir := basepath.Join("ops", "logs")
 
@@ -53,7 +54,7 @@ func MakeAPIServer(
 
 	return &APIServer{
 		basepath:              basepath,
-		kelpBinPath:           kelpBinPath,
+		kelpBinName:           kelpBinName,
 		configsDir:            configsDir,
 		logsDir:               logsDir,
 		kos:                   kos,
@@ -119,12 +120,22 @@ func (s *APIServer) writeJsonWithLog(w http.ResponseWriter, v interface{}, doLog
 }
 
 func (s *APIServer) runKelpCommandBlocking(namespace string, cmd string) ([]byte, error) {
-	cmdString := fmt.Sprintf("%s %s", s.kelpBinPath.Unix(), cmd)
+	// There is a weird issue on windows where the absolute path for the kelp binary does not work on the release GUI
+	// version because of the unzipped directory name but it will work on the released cli version or if we change the
+	// name of the folder in which the GUI version is unzipped.
+	// To avoid these issues we only invoke with the binary name as opposed to the absolute path that contains the
+	// directory name. see start_bot.go for some experimentation with absolute and relative paths
+	cmdString := fmt.Sprintf("./%s %s", s.kelpBinName, cmd)
 	return s.kos.Blocking(namespace, cmdString)
 }
 
 func (s *APIServer) runKelpCommandBackground(namespace string, cmd string) (*kelpos.Process, error) {
-	cmdString := fmt.Sprintf("%s %s", s.kelpBinPath.Unix(), cmd)
+	// There is a weird issue on windows where the absolute path for the kelp binary does not work on the release GUI
+	// version because of the unzipped directory name but it will work on the released cli version or if we change the
+	// name of the folder in which the GUI version is unzipped.
+	// To avoid these issues we only invoke with the binary name as opposed to the absolute path that contains the
+	// directory name. see start_bot.go for some experimentation with absolute and relative paths
+	cmdString := fmt.Sprintf("./%s %s", s.kelpBinName, cmd)
 	return s.kos.Background(namespace, cmdString)
 }
 
diff --git a/gui/backend/start_bot.go b/gui/backend/start_bot.go
index 725b50449..69d54dbe2 100644
--- a/gui/backend/start_bot.go
+++ b/gui/backend/start_bot.go
@@ -51,6 +51,9 @@ func (s *APIServer) doStartBot(botName string, strategy string, iterations *uint
 	// - native absolute paths did work on windows!
 	// - unix absolute path did not work on windows
 	//
+	// (see api_server.go#runKelpCommandBlocking and #runKelpCommandBackground for information that may be related to why
+	// absolute paths did not work)
+	//
 	// The above experimentation makes unix relative paths the most common format so we will use that to start new bots
 	//
 	// Note that on windows it could use the native windows naming scheme (C:\ etc.) but in the linux subsystem on windows
diff --git a/scripts/fs_bin_gen/gui/filesystem_vfsdata_dev.go b/scripts/fs_bin_gen/gui/filesystem_vfsdata_dev.go
index 60f5040d9..28d71b4d2 100644
--- a/scripts/fs_bin_gen/gui/filesystem_vfsdata_dev.go
+++ b/scripts/fs_bin_gen/gui/filesystem_vfsdata_dev.go
@@ -5,8 +5,8 @@ import (
 	"path/filepath"
 )
 
-// build dir is ../gui/web/build because we run from the bin directory in dev mode
-var guiBuildDir = filepath.Join("..", "gui", "web", "build")
+// build dir is gui/web/build because we run from the root kelp directory in dev mode
+var guiBuildDir = filepath.Join("gui", "web", "build")
 
 // file system for GUI
 var FS = http.Dir(guiBuildDir)
diff --git a/support/kelpos/kelpos.go b/support/kelpos/kelpos.go
index dd9bb3e78..cb70b1c15 100644
--- a/support/kelpos/kelpos.go
+++ b/support/kelpos/kelpos.go
@@ -1,6 +1,7 @@
 package kelpos
 
 import (
+	"fmt"
 	"io"
 	"os/exec"
 	"sync"
@@ -10,6 +11,7 @@ import (
 
 // KelpOS is a struct that manages all subprocesses started by this Kelp process
 type KelpOS struct {
+	workingBinDir       *OSPath
 	processes           map[string]Process
 	processLock         *sync.Mutex
 	bots                map[string]*BotInstance
@@ -33,11 +35,17 @@ type Process struct {
 var singleton *KelpOS
 
 func init() {
+	path, e := MakeOsPathBase()
+	if e != nil {
+		panic(e)
+	}
+
 	singleton = &KelpOS{
-		processes:   map[string]Process{},
-		processLock: &sync.Mutex{},
-		bots:        map[string]*BotInstance{},
-		botLock:     &sync.Mutex{},
+		workingBinDir: path,
+		processes:     map[string]Process{},
+		processLock:   &sync.Mutex{},
+		bots:          map[string]*BotInstance{},
+		botLock:       &sync.Mutex{},
 	}
 }
 
@@ -49,5 +57,8 @@ type BotInstance struct {
 
 // GetKelpOS gets the singleton instance
 func GetKelpOS() *KelpOS {
+	if singleton == nil {
+		panic(fmt.Errorf("there is a cycle stemming from the init() method since singleton was nil"))
+	}
 	return singleton
 }
diff --git a/support/kelpos/ospath.go b/support/kelpos/ospath.go
index 631389b27..aa737e9be 100644
--- a/support/kelpos/ospath.go
+++ b/support/kelpos/ospath.go
@@ -4,9 +4,12 @@ import (
 	"fmt"
 	"log"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"runtime/debug"
 	"strings"
+
+	"github.com/stellar/kelp/support/utils"
 )
 
 // OSPath encapsulates the pair of the native path (i.e. windows or unix) and the unix path
@@ -40,18 +43,24 @@ func makeOSPath(native string, unix string, isRel bool) *OSPath {
 
 // MakeOsPathBase is a factory method for the OSPath struct based on the current binary's directory
 func MakeOsPathBase() (*OSPath, error) {
-	currentDirUnslashed, e := getCurrentDirUnix()
-	if e != nil {
-		return nil, fmt.Errorf("could not get current directory: %s", e)
-	}
 	binaryDirectoryNative, e := getBinaryDirectoryNative()
 	if e != nil {
 		return nil, fmt.Errorf("could not get binary directory: %s", e)
 	}
-	ospath := makeOSPath(binaryDirectoryNative, currentDirUnslashed, false)
+
+	binaryDirectoryUnix, e := getBinaryDirectoryUnix(binaryDirectoryNative)
+	if e != nil {
+		return nil, fmt.Errorf("could not get binary directory unix: %s", e)
+	}
+
+	// use the binary directory for the base path since UI executables will be run from $PWD whereas the binary can be located
+	// in a different path (for example $PWD/Kelp.app/Contents/MacOS/ for darwin)
+	ospath := makeOSPath(binaryDirectoryNative, binaryDirectoryUnix, false)
 
 	if filepath.Base(ospath.Native()) != filepath.Base(ospath.Unix()) {
-		return nil, fmt.Errorf("ran from directory (%s) but need to run from the same directory as the location of the binary (%s), cd over to the location of the binary", ospath.Unix(), ospath.Native())
+		errorStr := fmt.Sprintf("ran from directory (%s) but need to run from the same directory as the location of the binary (%s), cd over to the location of the binary\n", ospath.Unix(), ospath.Native())
+		utils.PrintErrorHintf(errorStr)
+		return nil, fmt.Errorf(errorStr)
 	}
 
 	return ospath, nil
@@ -125,11 +134,37 @@ func (o *OSPath) RelFromPath(basepath *OSPath) (*OSPath, error) {
 	return makeOSPath(newRelNative, newRelUnix, true), nil
 }
 
-func getCurrentDirUnix() (string, error) {
-	kos := GetKelpOS()
-	outputBytes, e := kos.Blocking("pwd", "pwd")
+// getBinaryDirectoryUnix takes the workingDirUnix and adds to it the relative path from
+// workingDirNative to binaryDirectoryNative in unix form.
+func getBinaryDirectoryUnix(binaryDirectoryNative string) (string, error) {
+	wdUnix, e := getWorkingDirUnix()
+	if e != nil {
+		return "", fmt.Errorf("could not fetch working directory unix: %s", e)
+	}
+
+	wdNative, e := os.Getwd()
+	if e != nil {
+		return "", fmt.Errorf("could not fetch working directory native: %s", e)
+	}
+
+	// on windows the native format is not unix-based so we need to convert from C:\ vs /mnt/c notation. This method will
+	// standardize conversions across OSes but results in a nop for darwin/unix since they use unix paths as their native format
+	return convertNativePathToUnix(wdNative, binaryDirectoryNative, wdUnix)
+}
+
+func convertNativePathToUnix(baseNative string, targetNative string, baseUnix string) (string, error) {
+	relBaseToTarget, e := filepath.Rel(baseNative, targetNative)
+	if e != nil {
+		return "", fmt.Errorf("could not fetch relative path from baseNative (%s) to targetNative (%s): %s", baseNative, targetNative, e)
+	}
+
+	return toUnixFilepath(filepath.Join(baseUnix, toUnixFilepath(relBaseToTarget))), nil
+}
+
+func getWorkingDirUnix() (string, error) {
+	outputBytes, e := exec.Command("bash", "-c", "pwd").Output()
 	if e != nil {
-		return "", fmt.Errorf("could not fetch current directory: %s", e)
+		return "", fmt.Errorf("could not run raw command 'bash -c pwd': %s", e)
 	}
 	return strings.TrimSpace(string(outputBytes)), nil
 }
diff --git a/support/kelpos/ospath_test.go b/support/kelpos/ospath_test.go
index b4687b0ee..b862cc510 100644
--- a/support/kelpos/ospath_test.go
+++ b/support/kelpos/ospath_test.go
@@ -27,14 +27,7 @@ func TestOSPath(t *testing.T) {
 	for _, k := range testCases {
 		t.Run(k.basePathNative, func(t *testing.T) {
 			// early exit if running on a disallowed platform to avoid false negatives
-			isValid := false
-			for _, allowedGoos := range k.runGoos {
-				if runtime.GOOS == allowedGoos {
-					isValid = true
-					break
-				}
-			}
-			if !isValid {
+			if !checkGoosAllowed(k.runGoos) {
 				return
 			}
 
@@ -80,3 +73,96 @@ func TestOSPath(t *testing.T) {
 		})
 	}
 }
+
+func TestConvertNativePathToUnix(t *testing.T) {
+	testCases := []struct {
+		name           string
+		runGoos        []string
+		baseNative     string
+		targetNative   string
+		baseUnix       string
+		wantTargetUnix string
+	}{
+		{
+			name:           "unix_forward",
+			runGoos:        []string{"linux", "darwin"},
+			baseNative:     "/Users/a/test",
+			targetNative:   "/Users/a/test/b",
+			baseUnix:       "/Users/a/test",
+			wantTargetUnix: "/Users/a/test/b",
+		}, {
+			name:           "unix_forward_trailslash",
+			runGoos:        []string{"linux", "darwin"},
+			baseNative:     "/Users/a/test/",
+			targetNative:   "/Users/a/test/b/",
+			baseUnix:       "/Users/a/test/",
+			wantTargetUnix: "/Users/a/test/b",
+		}, {
+			name:           "unix_backward",
+			runGoos:        []string{"linux", "darwin"},
+			baseNative:     "/Users/a/test",
+			targetNative:   "/Users/a",
+			baseUnix:       "/Users/a/test",
+			wantTargetUnix: "/Users/a",
+		}, {
+			name:           "unix_backward_trailslash",
+			runGoos:        []string{"linux", "darwin"},
+			baseNative:     "/Users/a/test/",
+			targetNative:   "/Users/a/",
+			baseUnix:       "/Users/a/test/",
+			wantTargetUnix: "/Users/a",
+		}, {
+			name:           "windows_forward",
+			runGoos:        []string{"windows"},
+			baseNative:     "C:\\Users\\a\\test",
+			targetNative:   "C:\\Users\\a\\test\\b",
+			baseUnix:       "/Users/a/test",
+			wantTargetUnix: "/Users/a/test/b",
+		}, {
+			name:           "windows_forward_trailslash",
+			runGoos:        []string{"windows"},
+			baseNative:     "C:\\Users\\a\\test\\",
+			targetNative:   "C:\\Users\\a\\test\\b\\",
+			baseUnix:       "/Users/a/test/",
+			wantTargetUnix: "/Users/a/test/b",
+		}, {
+			name:           "windows_backward",
+			runGoos:        []string{"windows"},
+			baseNative:     "C:\\Users\\a\\test",
+			targetNative:   "C:\\Users\\a",
+			baseUnix:       "/Users/a/test",
+			wantTargetUnix: "/Users/a",
+		}, {
+			name:           "windows_backward_trailslash",
+			runGoos:        []string{"windows"},
+			baseNative:     "C:\\Users\\a\\test\\",
+			targetNative:   "C:\\Users\\a\\",
+			baseUnix:       "/Users/a/test/",
+			wantTargetUnix: "/Users/a",
+		},
+	}
+
+	for _, k := range testCases {
+		t.Run(k.name, func(t *testing.T) {
+			// early exit if running on a disallowed platform to avoid false negatives
+			if !checkGoosAllowed(k.runGoos) {
+				return
+			}
+
+			targetUnix, e := convertNativePathToUnix(k.baseNative, k.targetNative, k.baseUnix)
+			if !assert.NoError(t, e) {
+				return
+			}
+			assert.Equal(t, k.wantTargetUnix, targetUnix)
+		})
+	}
+}
+
+func checkGoosAllowed(runGoos []string) bool {
+	for _, allowedGoos := range runGoos {
+		if runtime.GOOS == allowedGoos {
+			return true
+		}
+	}
+	return false
+}
diff --git a/support/kelpos/process.go b/support/kelpos/process.go
index 3fd845028..062b3626f 100644
--- a/support/kelpos/process.go
+++ b/support/kelpos/process.go
@@ -96,6 +96,9 @@ func (kos *KelpOS) Blocking(namespace string, cmd string) ([]byte, error) {
 // Background runs the provided bash command in the background and registers the command
 func (kos *KelpOS) Background(namespace string, cmd string) (*Process, error) {
 	c := exec.Command("bash", "-c", cmd)
+	// always execute commands from the working directory (specify as native since underlying OS handles it)
+	c.Dir = kos.workingBinDir.Native()
+	log.Printf("process.Background is executing command: '%s' from directory '%s'", c.String(), c.Dir)
 
 	stdinWriter, e := c.StdinPipe()
 	if e != nil {
@@ -176,7 +179,7 @@ func (kos *KelpOS) RegisteredProcesses() []string {
 func (kos *KelpOS) Mkdir(dirPath *OSPath) error {
 	_, e := kos.Blocking("mkdir", fmt.Sprintf("mkdir -p %s", dirPath.Unix()))
 	if e != nil {
-		return fmt.Errorf("error running mkdir command for dir (%s): %s\n", dirPath, e)
+		return fmt.Errorf("error running mkdir command for dir (%s): %s\n", dirPath.AsString(), e)
 	}
 	return nil
 }