refactor(jira): share todos' Jira client for the time service

services/todos/internal/jira/client.go → internal/jira/client.go

@@ -1,5 +1,6 @@
-// Package jira finds the user's open tickets on a Jira Cloud site, through Jira's REST API v3
-// and a scoped API token (docs/plan.md §2, decisions 103 and 121).
+// Package jira reads the user's tickets on a Jira Cloud site, through Jira's REST API v3 and a
+// scoped API token: it finds the open tickets and fetches tickets by key, for each service that
+// the Jira settings set up (docs/plan.md §2, decisions 103, 121, 133 and 180).
 package jira
 
 import (

services/todos/internal/jira/client_test.go → internal/jira/client_test.go

@@ -7,7 +7,7 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/codedogapp/bakhyard/services/todos/internal/jira"
+	"github.com/codedogapp/bakhyard/internal/jira"
 )
 
 func TestSearchAsksForTheOpenTickets(t *testing.T) {

internal/jira/env.go

@@ -0,0 +1,51 @@
+package jira
+
+import (
+	"context"
+	"fmt"
+	"slices"
+
+	"github.com/codedogapp/bakhyard/internal/env"
+	"github.com/codedogapp/bakhyard/internal/secrets"
+)
+
+// The settings of the Jira site (docs/setup.md). Jira is off when none is set.
+const (
+	siteVar  = "BAKHYARD_JIRA_SITE"
+	apiVar   = "BAKHYARD_JIRA_API"
+	emailVar = "BAKHYARD_JIRA_EMAIL"
+)
+
+// tokenAccount is the Keychain account of the Jira API token, and the name of its file.
+const tokenAccount = "jira-token"
+
+// FromEnv returns the client of the Jira site in the settings that getenv reads, with the API
+// token that readSecret reads, or nil when none of the Jira settings is set.
+func FromEnv(getenv func(string) string, readSecret secrets.Reader) (*Client, error) {
+	settings := []string{siteVar, apiVar, emailVar}
+	if !slices.ContainsFunc(settings, func(name string) bool { return getenv(name) != "" }) {
+		return nil, nil
+	}
+
+	site, err := env.SecureURL(getenv, siteVar)
+	if err != nil {
+		return nil, err
+	}
+
+	address, err := env.SecureURL(getenv, apiVar)
+	if err != nil {
+		return nil, err
+	}
+
+	email, err := env.Required(getenv, emailVar)
+	if err != nil {
+		return nil, err
+	}
+
+	token, err := readSecret(context.Background(), tokenAccount)
+	if err != nil {
+		return nil, fmt.Errorf("read the Jira token: %w", err)
+	}
+
+	return New(Settings{Site: site, API: address, Email: email, Token: token}), nil
+}

internal/jira/env_test.go

@@ -0,0 +1,130 @@
+package jira_test
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/codedogapp/bakhyard/internal/jira"
+	"github.com/codedogapp/bakhyard/internal/secrets"
+)
+
+// The Jira site and REST API in the settings of most tests.
+const (
+	siteURL = "https://acme.atlassian.net"
+	apiURL  = "https://api.atlassian.com/ex/jira/cloud-id"
+)
+
+func TestFromEnvSearchesTheJiraInTheSettings(t *testing.T) {
+	client, err := jira.FromEnv(envOf(settingsAt(standIn(t))),
+		secretsWith(map[string]string{"jira-token": token}))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	got, err := client.Search(t.Context())
+	if err != nil || len(got) != 1 || got[0].URL != siteURL+"/browse/PAY-1" {
+		t.Errorf("got %v, %v, want PAY-1 on %s", got, err, siteURL)
+	}
+}
+
+func TestFromEnvIsOffWithoutTheSettings(t *testing.T) {
+	client, err := jira.FromEnv(envOf(nil), secretsWith(nil))
+	if client != nil || err != nil {
+		t.Errorf("got %v, %v, want no client and no error", client, err)
+	}
+}
+
+func TestFromEnvRejects(t *testing.T) {
+	for _, test := range []rejection{
+		{map[string]string{"BAKHYARD_JIRA_SITE": siteURL}, "BAKHYARD_JIRA_API is not set"},
+		{map[string]string{"BAKHYARD_JIRA_API": apiURL}, "BAKHYARD_JIRA_SITE is not set"},
+		{map[string]string{"BAKHYARD_JIRA_EMAIL": email}, "BAKHYARD_JIRA_SITE is not set"},
+		{settingsWith("BAKHYARD_JIRA_SITE", ""), "BAKHYARD_JIRA_SITE is not set"},
+		{settingsWith("BAKHYARD_JIRA_API", ""), "BAKHYARD_JIRA_API is not set"},
+		{settingsWith("BAKHYARD_JIRA_EMAIL", ""), "BAKHYARD_JIRA_EMAIL is not set"},
+		// internal/env tests which URLs pass; these show that both of Jira's URLs must.
+		notHTTPS("BAKHYARD_JIRA_SITE", "http://acme.atlassian.net"),
+		notHTTPS("BAKHYARD_JIRA_API", "http://example.com"),
+	} {
+		// Without the token, so the settings must be checked before it's read.
+		client, err := jira.FromEnv(envOf(test.settings), secretsWith(nil))
+		if client != nil || err == nil || err.Error() != test.want {
+			t.Errorf("got %v, %v, want the error %s", client, err, test.want)
+		}
+	}
+}
+
+func TestFromEnvNeedsTheToken(t *testing.T) {
+	client, err := jira.FromEnv(envOf(settingsAt(apiURL)), secretsWith(nil))
+
+	want := "read the Jira token: no secret jira-token"
+	if client != nil || err == nil || err.Error() != want {
+		t.Errorf("got %v, %v, want the error %s", client, err, want)
+	}
+}
+
+// standIn returns the URL of a stand-in for Jira's REST API that has one open ticket, PAY-1, for
+// email with token.
+func standIn(t *testing.T) string {
+	t.Helper()
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		user, password, ok := r.BasicAuth()
+		if !ok || user != email || password != token {
+			w.WriteHeader(http.StatusUnauthorized)
+
+			return
+		}
+
+		answer(t, w, last(issue("PAY-1", "Pay", "To Do", "new")))
+	}))
+	t.Cleanup(server.Close)
+
+	return server.URL
+}
+
+// settingsAt returns the Jira settings, with the REST API at address.
+func settingsAt(address string) map[string]string {
+	return map[string]string{
+		"BAKHYARD_JIRA_SITE": siteURL, "BAKHYARD_JIRA_API": address, "BAKHYARD_JIRA_EMAIL": email,
+	}
+}
+
+// settingsWith returns the Jira settings, with name set to value.
+func settingsWith(name, value string) map[string]string {
+	settings := settingsAt(apiURL)
+	settings[name] = value
+
+	return settings
+}
+
+// rejection is a row of TestFromEnvRejects: the settings, and the error they give.
+type rejection struct {
+	settings map[string]string
+	want     string
+}
+
+// notHTTPS returns the rejection of value in the setting name as not an https URL.
+func notHTTPS(name, value string) rejection {
+	return rejection{settingsWith(name, value), name + `: "` + value + `" is not an https URL`}
+}
+
+// envOf returns an environment that holds only settings.
+func envOf(settings map[string]string) func(string) string {
+	return func(name string) string { return settings[name] }
+}
+
+// secretsWith returns a Reader of the secrets in items.
+func secretsWith(items map[string]string) secrets.Reader {
+	return func(_ context.Context, name string) (string, error) {
+		secret, ok := items[name]
+		if !ok {
+			return "", fmt.Errorf("no secret %s", name)
+		}
+
+		return secret, nil
+	}
+}

services/todos/internal/jira/fail_test.go → internal/jira/fail_test.go

@@ -8,7 +8,7 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/codedogapp/bakhyard/services/todos/internal/jira"
+	"github.com/codedogapp/bakhyard/internal/jira"
 )
 
 func TestSearchFailsOnABadAnswer(t *testing.T) {

services/todos/internal/jira/fake_test.go → internal/jira/fake_test.go

@@ -8,7 +8,7 @@ import (
 	"sync"
 	"testing"
 
-	"github.com/codedogapp/bakhyard/services/todos/internal/jira"
+	"github.com/codedogapp/bakhyard/internal/jira"
 )
 
 const (

services/todos/internal/jira/fetch_test.go → internal/jira/fetch_test.go

@@ -8,7 +8,7 @@ import (
 	"slices"
 	"testing"
 
-	"github.com/codedogapp/bakhyard/services/todos/internal/jira"
+	"github.com/codedogapp/bakhyard/internal/jira"
 )
 
 func TestFetchAsksForTheTickets(t *testing.T) {

services/todos/internal/jira/fetchfail_test.go → internal/jira/fetchfail_test.go

@@ -8,7 +8,7 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/codedogapp/bakhyard/services/todos/internal/jira"
+	"github.com/codedogapp/bakhyard/internal/jira"
 )
 
 func TestFetchFailsOnABadAnswer(t *testing.T) {

services/todos/internal/api/api.go

@@ -8,7 +8,7 @@ import (
 
 	"github.com/go-chi/chi/v5"
 
-	"github.com/codedogapp/bakhyard/services/todos/internal/jira"
+	"github.com/codedogapp/bakhyard/services/todos/internal/tickets"
 	"github.com/codedogapp/bakhyard/services/todos/internal/todo"
 )
 
@@ -19,14 +19,14 @@ type list struct {
 
 // Register serves the todos in store on router: GET /todos lists them, POST /todos adds one,
 // PATCH /todos/{id} changes one and DELETE /todos/{id} deletes one. GET /todos/tickets answers
-// what tickets has found in Jira, nil when Jira isn't set up. GET /todos/dismissals lists the
+// what poller has found in Jira, nil when Jira isn't set up. GET /todos/dismissals lists the
 // tickets dismissed from the inbox, and PUT /todos/dismissals/{key} dismisses one. Other methods
 // get 405, and logger gets the errors that the database runs into.
-func Register(router chi.Router, store *todo.Store, tickets *jira.Poller, logger *slog.Logger) {
+func Register(router chi.Router, store *todo.Store, poller *tickets.Poller, logger *slog.Logger) {
 	router.Get("/todos", listTodos(store, logger))
 	router.Post("/todos", addTodo(store, logger))
 	// Always there, so that chi doesn't take the path for /todos/{id}, which has no GET.
-	router.Get("/todos/tickets", listTickets(tickets))
+	router.Get("/todos/tickets", listTickets(poller))
 	router.Get("/todos/dismissals", listDismissals(store, logger))
 	router.Put("/todos/dismissals/{key}", dismissTicket(store, logger))
 	router.Patch("/todos/{id}", changeTodo(store, logger))

services/todos/internal/api/tickets.go

@@ -3,12 +3,12 @@ package api
 import (
 	"net/http"
 
-	"github.com/codedogapp/bakhyard/services/todos/internal/jira"
+	"github.com/codedogapp/bakhyard/services/todos/internal/tickets"
 )
 
 // listTickets returns the handler of GET /todos/tickets, which answers what poller has found in
 // Jira, once its first poll has ended. Without Jira, poller is nil and the client gets a 404.
-func listTickets(poller *jira.Poller) http.HandlerFunc {
+func listTickets(poller *tickets.Poller) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		if poller == nil {
 			http.Error(w, "Jira is not set up", http.StatusNotFound)

services/todos/internal/api/tickets_test.go

@@ -11,13 +11,14 @@ import (
 
 	"github.com/go-chi/chi/v5"
 
+	"github.com/codedogapp/bakhyard/internal/jira"
 	"github.com/codedogapp/bakhyard/services/todos/internal/api"
-	"github.com/codedogapp/bakhyard/services/todos/internal/jira"
+	"github.com/codedogapp/bakhyard/services/todos/internal/tickets"
 )
 
 // A bubble's clock starts at midnight UTC on 1 January 2000, when the poller searches first.
 func TestGetTicketsAnswersWhatThePollerFound(t *testing.T) {
-	tickets := []jira.Ticket{
+	open := []jira.Ticket{
 		{
 			Key: "PAY-2", Summary: `Answer "Re: <the plan>"`, Status: "In Review",
 			Category: jira.InProgress, URL: "https://acme.example/browse/PAY-2",
@@ -40,7 +41,7 @@ func TestGetTicketsAnswersWhatThePollerFound(t *testing.T) {
 		want    string
 	}{
 		{
-			tickets, nil, nil,
+			open, nil, nil,
 			`{"tickets":[{"key":"PAY-2","summary":"Answer \"Re: <the plan>\"",` +
 				`"status":"In Review","category":"in-progress",` +
 				`"url":"https://acme.example/browse/PAY-2"},` +
@@ -98,7 +99,7 @@ func withJira(
 
 	fake := fakeJira{search: search, done: done}
 
-	poller := jira.NewPoller(fake, fake.linked, slog.New(slog.DiscardHandler))
+	poller := tickets.NewPoller(fake, fake.linked, slog.New(slog.DiscardHandler))
 	go poller.Run(t.Context())
 
 	router := chi.NewRouter()

services/todos/internal/attention/attention.go

@@ -8,7 +8,8 @@ import (
 	"strconv"
 
 	attentionv1 "github.com/codedogapp/bakhyard/internal/gen/bakhyard/attention/v1"
-	"github.com/codedogapp/bakhyard/services/todos/internal/jira"
+	"github.com/codedogapp/bakhyard/internal/jira"
+	"github.com/codedogapp/bakhyard/services/todos/internal/tickets"
 	"github.com/codedogapp/bakhyard/services/todos/internal/todo"
 )
 
@@ -17,18 +18,18 @@ import (
 type Lister struct {
 	list      func(context.Context) ([]todo.Todo, error)
 	dismissed func(context.Context) ([]string, error)
-	tickets   *jira.Poller
+	poller    *tickets.Poller
 }
 
 // NewLister returns a lister of the todos that list reads, such as a Store's List, and of the Jira
 // inbox, which leaves out the tickets that dismissed reads, such as a Store's Dismissed. It learns
-// from tickets which tickets are open and which are done; tickets is nil when Jira isn't set up.
+// from poller which tickets are open and which are done; poller is nil when Jira isn't set up.
 func NewLister(
 	list func(context.Context) ([]todo.Todo, error),
 	dismissed func(context.Context) ([]string, error),
-	tickets *jira.Poller,
+	poller *tickets.Poller,
 ) *Lister {
-	return &Lister{list: list, dismissed: dismissed, tickets: tickets}
+	return &Lister{list: list, dismissed: dismissed, poller: poller}
 }
 
 // ListItems lists the Jira inbox's tickets, then, in the list's order, the todos left to do that
@@ -67,14 +68,14 @@ func (l *Lister) ListItems(
 
 // found returns what the latest Jira poll to succeed found, once the first poll has ended, and
 // nothing when Jira isn't set up.
-func (l *Lister) found(ctx context.Context) (jira.Found, error) {
-	if l.tickets == nil {
-		return jira.Found{}, nil
+func (l *Lister) found(ctx context.Context) (tickets.Found, error) {
+	if l.poller == nil {
+		return tickets.Found{}, nil
 	}
 
-	latest, err := l.tickets.Latest(ctx)
+	latest, err := l.poller.Latest(ctx)
 	if err != nil {
-		return jira.Found{}, err
+		return tickets.Found{}, err
 	}
 
 	return latest.Found, nil