Refactor by moving methods out of lib.go into more specific packages

client/client_test.go

@@ -18,6 +18,7 @@ import (
 	"github.com/google/uuid"
 	"gorm.io/gorm"
 
+	"github.com/ddworken/hishtory/client/cmd"
 	"github.com/ddworken/hishtory/client/data"
 	"github.com/ddworken/hishtory/client/hctx"
 	"github.com/ddworken/hishtory/client/lib"
@@ -540,7 +541,7 @@ func installFromHead(t *testing.T, tester shellTester) (string, string) {
 
 func installFromPrev(t *testing.T, tester shellTester) (string, string) {
 	defer testutils.BackupAndRestoreEnv("HISHTORY_FORCE_CLIENT_VERSION")()
-	dd, err := lib.GetDownloadData(makeTestOnlyContextWithFakeConfig())
+	dd, err := cmd.GetDownloadData(makeTestOnlyContextWithFakeConfig())
 	require.NoError(t, err)
 	pv, err := shared.ParseVersionString(dd.Version)
 	require.NoError(t, err)
@@ -553,7 +554,7 @@ func installFromPrev(t *testing.T, tester shellTester) (string, string) {
 }
 
 func updateToRelease(t *testing.T, tester shellTester) string {
-	dd, err := lib.GetDownloadData(makeTestOnlyContextWithFakeConfig())
+	dd, err := cmd.GetDownloadData(makeTestOnlyContextWithFakeConfig())
 	require.NoError(t, err)
 
 	// Update
@@ -1089,7 +1090,7 @@ func testInstallViaPythonScriptChild(t *testing.T, tester shellTester) {
 	userSecret := matches[1]
 
 	// Test the status subcommand
-	downloadData, err := lib.GetDownloadData(makeTestOnlyContextWithFakeConfig())
+	downloadData, err := cmd.GetDownloadData(makeTestOnlyContextWithFakeConfig())
 	require.NoError(t, err)
 	out = tester.RunInteractiveShell(t, `hishtory status`)
 	expectedOut := fmt.Sprintf("hiSHtory: %s\nEnabled: true\nSecret Key: %s\nCommit Hash: ", downloadData.Version, userSecret)

client/cmd/install.go

@@ -19,6 +19,7 @@ import (
 	"github.com/ddworken/hishtory/client/hctx"
 	"github.com/ddworken/hishtory/client/lib"
 	"github.com/ddworken/hishtory/shared"
+	"github.com/google/uuid"
 	"github.com/spf13/cobra"
 )
 
@@ -79,7 +80,7 @@ var initCmd = &cobra.Command{
 		if len(args) > 0 {
 			secretKey = args[0]
 		}
-		lib.CheckFatalError(lib.Setup(secretKey, *offlineInit))
+		lib.CheckFatalError(setup(secretKey, *offlineInit))
 		if os.Getenv("HISHTORY_SKIP_INIT_IMPORT") == "" {
 			fmt.Println("Importing existing shell history...")
 			ctx := hctx.MakeContext()
@@ -169,7 +170,7 @@ func install(secretKey string, offline bool) error {
 	_, err = hctx.GetConfig()
 	if err != nil {
 		// No config, so set up a new installation
-		return lib.Setup(secretKey, offline)
+		return setup(secretKey, offline)
 	}
 	err = handleDbUpgrades(hctx.MakeContext())
 	if err != nil {
@@ -538,6 +539,65 @@ func stripLines(filePath, lines string) error {
 	return os.WriteFile(filePath, []byte(ret), 0644)
 }
 
+func setup(userSecret string, isOffline bool) error {
+	if userSecret == "" {
+		userSecret = uuid.Must(uuid.NewRandom()).String()
+	}
+	fmt.Println("Setting secret hishtory key to " + string(userSecret))
+
+	// Create and set the config
+	var config hctx.ClientConfig
+	config.UserSecret = userSecret
+	config.IsEnabled = true
+	config.DeviceId = uuid.Must(uuid.NewRandom()).String()
+	config.ControlRSearchEnabled = true
+	config.IsOffline = isOffline
+	err := hctx.SetConfig(&config)
+	if err != nil {
+		return fmt.Errorf("failed to persist config to disk: %w", err)
+	}
+
+	// Drop all existing data
+	db, err := hctx.OpenLocalSqliteDb()
+	if err != nil {
+		return err
+	}
+	db.Exec("DELETE FROM history_entries")
+
+	// Bootstrap from remote date
+	if config.IsOffline {
+		return nil
+	}
+	ctx := hctx.MakeContext()
+	registerPath := "/api/v1/register?user_id=" + data.UserId(userSecret) + "&device_id=" + config.DeviceId
+	if os.Getenv("HISHTORY_TEST") != "" {
+		registerPath += "&is_integration_test_device=true"
+	}
+	_, err = lib.ApiGet(ctx, registerPath)
+	if err != nil {
+		return fmt.Errorf("failed to register device with backend: %w", err)
+	}
+
+	respBody, err := lib.ApiGet(ctx, "/api/v1/bootstrap?user_id="+data.UserId(userSecret)+"&device_id="+config.DeviceId)
+	if err != nil {
+		return fmt.Errorf("failed to bootstrap device from the backend: %w", err)
+	}
+	var retrievedEntries []*shared.EncHistoryEntry
+	err = json.Unmarshal(respBody, &retrievedEntries)
+	if err != nil {
+		return fmt.Errorf("failed to load JSON response: %w", err)
+	}
+	for _, entry := range retrievedEntries {
+		decEntry, err := data.DecryptHistoryEntry(userSecret, *entry)
+		if err != nil {
+			return fmt.Errorf("failed to decrypt history entry from server: %w", err)
+		}
+		lib.AddToDbIfNew(db, decEntry)
+	}
+
+	return nil
+}
+
 func init() {
 	rootCmd.AddCommand(installCmd)
 	rootCmd.AddCommand(initCmd)

client/cmd/install_test.go

@@ -0,0 +1,60 @@
+package cmd
+
+import (
+	"os"
+	"path"
+	"testing"
+
+	"github.com/ddworken/hishtory/client/data"
+	"github.com/ddworken/hishtory/client/hctx"
+	"github.com/ddworken/hishtory/shared/testutils"
+	"github.com/stretchr/testify/require"
+)
+
+func TestSetup(t *testing.T) {
+	defer testutils.BackupAndRestore(t)()
+	defer testutils.RunTestServer()()
+
+	homedir, err := os.UserHomeDir()
+	require.NoError(t, err)
+	if _, err := os.Stat(path.Join(homedir, data.GetHishtoryPath(), data.CONFIG_PATH)); err == nil {
+		t.Fatalf("hishtory secret file already exists!")
+	}
+	require.NoError(t, setup("", false))
+	if _, err := os.Stat(path.Join(homedir, data.GetHishtoryPath(), data.CONFIG_PATH)); err != nil {
+		t.Fatalf("hishtory secret file does not exist after Setup()!")
+	}
+	data, err := os.ReadFile(path.Join(homedir, data.GetHishtoryPath(), data.CONFIG_PATH))
+	require.NoError(t, err)
+	if len(data) < 10 {
+		t.Fatalf("hishtory secret has unexpected length: %d", len(data))
+	}
+	config := hctx.GetConf(hctx.MakeContext())
+	if config.IsOffline != false {
+		t.Fatalf("hishtory config should have been offline")
+	}
+}
+
+func TestSetupOffline(t *testing.T) {
+	defer testutils.BackupAndRestore(t)()
+	defer testutils.RunTestServer()()
+
+	homedir, err := os.UserHomeDir()
+	require.NoError(t, err)
+	if _, err := os.Stat(path.Join(homedir, data.GetHishtoryPath(), data.CONFIG_PATH)); err == nil {
+		t.Fatalf("hishtory secret file already exists!")
+	}
+	require.NoError(t, setup("", true))
+	if _, err := os.Stat(path.Join(homedir, data.GetHishtoryPath(), data.CONFIG_PATH)); err != nil {
+		t.Fatalf("hishtory secret file does not exist after Setup()!")
+	}
+	data, err := os.ReadFile(path.Join(homedir, data.GetHishtoryPath(), data.CONFIG_PATH))
+	require.NoError(t, err)
+	if len(data) < 10 {
+		t.Fatalf("hishtory secret has unexpected length: %d", len(data))
+	}
+	config := hctx.GetConf(hctx.MakeContext())
+	if config.IsOffline != true {
+		t.Fatalf("hishtory config should have been offline, actual=%#v", string(data))
+	}
+}

client/cmd/saveHistoryEntry_test.go

@@ -8,7 +8,6 @@ import (
 	"time"
 
 	"github.com/ddworken/hishtory/client/hctx"
-	"github.com/ddworken/hishtory/client/lib"
 	"github.com/ddworken/hishtory/shared/testutils"
 	"github.com/stretchr/testify/require"
 )
@@ -16,7 +15,7 @@ import (
 func TestBuildHistoryEntry(t *testing.T) {
 	defer testutils.BackupAndRestore(t)()
 	defer testutils.RunTestServer()()
-	require.NoError(t, lib.Setup("", false))
+	require.NoError(t, setup("", false))
 
 	// Test building an actual entry for bash
 	entry, err := buildHistoryEntry(hctx.MakeContext(), []string{"unused", "saveHistoryEntry", "bash", "120", " 123  ls /foo  ", "1641774958"})
@@ -109,7 +108,7 @@ func TestBuildHistoryEntryWithTimestampStripping(t *testing.T) {
 	defer testutils.BackupAndRestoreEnv("HISTTIMEFORMAT")()
 	defer testutils.BackupAndRestore(t)()
 	defer testutils.RunTestServer()()
-	require.NoError(t, lib.Setup("", false))
+	require.NoError(t, setup("", false))
 
 	testcases := []struct {
 		input, histtimeformat, expectedCommand string

client/cmd/update.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"bytes"
 	"context"
+	"encoding/json"
 	"fmt"
 	"io"
 	"net/http"
@@ -28,9 +29,22 @@ var updateCmd = &cobra.Command{
 	},
 }
 
+func GetDownloadData(ctx context.Context) (shared.UpdateInfo, error) {
+	respBody, err := lib.ApiGet(ctx, "/api/v1/download")
+	if err != nil {
+		return shared.UpdateInfo{}, fmt.Errorf("failed to download update info: %w", err)
+	}
+	var downloadData shared.UpdateInfo
+	err = json.Unmarshal(respBody, &downloadData)
+	if err != nil {
+		return shared.UpdateInfo{}, fmt.Errorf("failed to parse update info: %w", err)
+	}
+	return downloadData, nil
+}
+
 func update(ctx context.Context) error {
 	// Download the binary
-	downloadData, err := lib.GetDownloadData(ctx)
+	downloadData, err := GetDownloadData(ctx)
 	if err != nil {
 		return err
 	}

client/lib/lib.go

@@ -52,66 +52,6 @@ var GitCommit string = "Unknown"
 // Funnily enough, 256KB actually wasn't enough. See https://github.com/ddworken/hishtory/issues/93
 var maxSupportedLineLengthForImport = 512_000
 
-// TODO: move this function to install.go
-func Setup(userSecret string, isOffline bool) error {
-	if userSecret == "" {
-		userSecret = uuid.Must(uuid.NewRandom()).String()
-	}
-	fmt.Println("Setting secret hishtory key to " + string(userSecret))
-
-	// Create and set the config
-	var config hctx.ClientConfig
-	config.UserSecret = userSecret
-	config.IsEnabled = true
-	config.DeviceId = uuid.Must(uuid.NewRandom()).String()
-	config.ControlRSearchEnabled = true
-	config.IsOffline = isOffline
-	err := hctx.SetConfig(&config)
-	if err != nil {
-		return fmt.Errorf("failed to persist config to disk: %w", err)
-	}
-
-	// Drop all existing data
-	db, err := hctx.OpenLocalSqliteDb()
-	if err != nil {
-		return err
-	}
-	db.Exec("DELETE FROM history_entries")
-
-	// Bootstrap from remote date
-	if config.IsOffline {
-		return nil
-	}
-	ctx := hctx.MakeContext()
-	registerPath := "/api/v1/register?user_id=" + data.UserId(userSecret) + "&device_id=" + config.DeviceId
-	if os.Getenv("HISHTORY_TEST") != "" {
-		registerPath += "&is_integration_test_device=true"
-	}
-	_, err = ApiGet(ctx, registerPath)
-	if err != nil {
-		return fmt.Errorf("failed to register device with backend: %w", err)
-	}
-
-	respBody, err := ApiGet(ctx, "/api/v1/bootstrap?user_id="+data.UserId(userSecret)+"&device_id="+config.DeviceId)
-	if err != nil {
-		return fmt.Errorf("failed to bootstrap device from the backend: %w", err)
-	}
-	var retrievedEntries []*shared.EncHistoryEntry
-	err = json.Unmarshal(respBody, &retrievedEntries)
-	if err != nil {
-		return fmt.Errorf("failed to load JSON response: %w", err)
-	}
-	for _, entry := range retrievedEntries {
-		decEntry, err := data.DecryptHistoryEntry(userSecret, *entry)
-		if err != nil {
-			return fmt.Errorf("failed to decrypt history entry from server: %w", err)
-		}
-		AddToDbIfNew(db, decEntry)
-	}
-
-	return nil
-}
-
 func AddToDbIfNew(db *gorm.DB, entry data.HistoryEntry) {
 	tx := db.Where("local_username = ?", entry.LocalUsername)
 	tx = tx.Where("hostname = ?", entry.Hostname)
@@ -234,10 +174,6 @@ func DisplayResults(ctx context.Context, results []*data.HistoryEntry, numResult
 	return nil
 }
 
-func IsEnabled(ctx context.Context) (bool, error) {
-	return hctx.GetConf(ctx).IsEnabled, nil
-}
-
 func CheckFatalError(err error) {
 	if err != nil {
 		_, filename, line, _ := runtime.Caller(1)
@@ -553,19 +489,6 @@ func getServerHostname() string {
 	return "https://api.hishtory.dev"
 }
 
-func GetDownloadData(ctx context.Context) (shared.UpdateInfo, error) {
-	respBody, err := ApiGet(ctx, "/api/v1/download")
-	if err != nil {
-		return shared.UpdateInfo{}, fmt.Errorf("failed to download update info: %w", err)
-	}
-	var downloadData shared.UpdateInfo
-	err = json.Unmarshal(respBody, &downloadData)
-	if err != nil {
-		return shared.UpdateInfo{}, fmt.Errorf("failed to parse update info: %w", err)
-	}
-	return downloadData, nil
-}
-
 func httpClient() *http.Client {
 	return &http.Client{}
 }

client/lib/lib_test.go

@@ -3,7 +3,6 @@ package lib
 import (
 	"fmt"
 	"os"
-	"path"
 	"reflect"
 	"testing"
 	"time"
@@ -21,53 +20,6 @@ func TestMain(m *testing.M) {
 	os.Setenv("HISHTORY_TEST", "1")
 }
 
-func TestSetup(t *testing.T) {
-	defer testutils.BackupAndRestore(t)()
-	defer testutils.RunTestServer()()
-
-	homedir, err := os.UserHomeDir()
-	require.NoError(t, err)
-	if _, err := os.Stat(path.Join(homedir, data.GetHishtoryPath(), data.CONFIG_PATH)); err == nil {
-		t.Fatalf("hishtory secret file already exists!")
-	}
-	require.NoError(t, Setup("", false))
-	if _, err := os.Stat(path.Join(homedir, data.GetHishtoryPath(), data.CONFIG_PATH)); err != nil {
-		t.Fatalf("hishtory secret file does not exist after Setup()!")
-	}
-	data, err := os.ReadFile(path.Join(homedir, data.GetHishtoryPath(), data.CONFIG_PATH))
-	require.NoError(t, err)
-	if len(data) < 10 {
-		t.Fatalf("hishtory secret has unexpected length: %d", len(data))
-	}
-	config := hctx.GetConf(hctx.MakeContext())
-	if config.IsOffline != false {
-		t.Fatalf("hishtory config should have been offline")
-	}
-}
-
-func TestSetupOffline(t *testing.T) {
-	defer testutils.BackupAndRestore(t)()
-	defer testutils.RunTestServer()()
-
-	homedir, err := os.UserHomeDir()
-	require.NoError(t, err)
-	if _, err := os.Stat(path.Join(homedir, data.GetHishtoryPath(), data.CONFIG_PATH)); err == nil {
-		t.Fatalf("hishtory secret file already exists!")
-	}
-	require.NoError(t, Setup("", true))
-	if _, err := os.Stat(path.Join(homedir, data.GetHishtoryPath(), data.CONFIG_PATH)); err != nil {
-		t.Fatalf("hishtory secret file does not exist after Setup()!")
-	}
-	data, err := os.ReadFile(path.Join(homedir, data.GetHishtoryPath(), data.CONFIG_PATH))
-	require.NoError(t, err)
-	if len(data) < 10 {
-		t.Fatalf("hishtory secret has unexpected length: %d", len(data))
-	}
-	config := hctx.GetConf(hctx.MakeContext())
-	if config.IsOffline != true {
-		t.Fatalf("hishtory config should have been offline, actual=%#v", string(data))
-	}
-}
 func TestPersist(t *testing.T) {
 	defer testutils.BackupAndRestore(t)()
 	require.NoError(t, hctx.InitConfig())
@@ -168,32 +120,6 @@ func TestSearch(t *testing.T) {
 	}
 }
 
-func TestAddToDbIfNew(t *testing.T) {
-	// Set up
-	defer testutils.BackupAndRestore(t)()
-	require.NoError(t, hctx.InitConfig())
-	db := hctx.GetDb(hctx.MakeContext())
-
-	// Add duplicate entries
-	entry1 := testutils.MakeFakeHistoryEntry("ls /foo")
-	AddToDbIfNew(db, entry1)
-	AddToDbIfNew(db, entry1)
-	entry2 := testutils.MakeFakeHistoryEntry("ls /foo")
-	AddToDbIfNew(db, entry2)
-	AddToDbIfNew(db, entry2)
-	AddToDbIfNew(db, entry1)
-
-	// Check there should only be two entries
-	var entries []data.HistoryEntry
-	result := db.Find(&entries)
-	if result.Error != nil {
-		t.Fatal(result.Error)
-	}
-	if len(entries) != 2 {
-		t.Fatalf("entries has an incorrect length: %d, entries=%#v", len(entries), entries)
-	}
-}
-
 func TestChunks(t *testing.T) {
 	testcases := []struct {
 		input     []int