Merge pull request #10 from yohamta/issue-9

Fix #9

internal/admin/handlers/job.go

@@ -373,7 +373,7 @@ func buildLog(logs []*models.StatusFile) *Log {
 }
 
 func getPathParameter(r *http.Request) (string, error) {
-	re := regexp.MustCompile("/([^/\\?]+)/?$")
+	re := regexp.MustCompile("/jobs/([^/\\?]+)/?$")
 	m := re.FindStringSubmatch(r.URL.Path)
 	if len(m) < 2 {
 		return "", fmt.Errorf("invalid URL")

internal/admin/handlers/list.go

@@ -7,6 +7,7 @@ import (
 	"path/filepath"
 
 	"github.com/yohamta/jobctl/internal/controller"
+	"github.com/yohamta/jobctl/internal/utils"
 )
 
 type jobListResponse struct {
@@ -91,7 +92,9 @@ func getGetListParameter(r *http.Request) *jobListParameter {
 
 func listGroups(dir string) ([]*group, error) {
 	ret := []*group{}
-
+	if !utils.FileExists(dir) {
+		return ret, nil
+	}
 	fis, err := ioutil.ReadDir(dir)
 	if err != nil || fis == nil {
 		log.Printf("%v", err)

internal/admin/handlers/web/templates/index.gohtml

@@ -87,7 +87,7 @@
     );
   }
   function JobItem({ job, group }) {
-    const url = encodeURI("/" + job.File + "?group=" + group)
+    const url = encodeURI("/jobs/" + job.File + "?group=" + group)
     return (<tr>
       <td className="has-text-weight-semibold"><a href={url}>{job.File}</a></td>
       <td><span class="tag has-text-weight-semibold" style={tagColorMapping["Job"]}>Job</span></td>
@@ -108,7 +108,7 @@
     )
   }
   function GroupItemBack({ group }) {
-    const url = encodeURI("/")
+    const url = encodeURI("/jobs/")
     return (<tr>
       <td className="has-text-weight-semibold"><a href={url}>../ (upper group)</a></td>
       <td><span className="tag has-text-weight-semibold" style={tagColorMapping["Group"]}>Group</span></td>
@@ -121,7 +121,7 @@
     </tr>)
   }
   function GroupItem({ group }) {
-    const url = encodeURI("/?group=" + group.Name)
+    const url = encodeURI("/jobs/?group=" + group.Name)
     return (<tr>
       <td className="has-text-weight-semibold"><a href={url}>{group.Name}</a></td>
       <td><span className="tag has-text-weight-semibold" style={tagColorMapping["Group"]}>Group</span></td>

internal/admin/http.go

@@ -5,7 +5,6 @@ import (
 	"log"
 	"net"
 	"net/http"
-	"time"
 )
 
 type server struct {
@@ -77,7 +76,6 @@ func (svr *server) handleShutdown(w http.ResponseWriter, r *http.Request) {
 	log.Println("received shutdown request")
 	w.Write([]byte("shutting down the jobctl server...\n"))
 	go func() {
-		time.Sleep(time.Millisecond * 3000)
 		svr.Shutdown()
 	}()
 }

internal/admin/http_test.go

@@ -0,0 +1,89 @@
+package admin_test
+
+import (
+	"fmt"
+	"io/ioutil"
+	"net"
+	"net/http"
+	"os"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	"github.com/yohamta/jobctl/internal/admin"
+)
+
+func TestHttpServerStartShutdown(t *testing.T) {
+	dir, err := ioutil.TempDir("", "test_http_server")
+	require.NoError(t, err)
+	os.RemoveAll(dir)
+
+	host := "127.0.0.1"
+	port := findPort()
+	server := admin.NewServer(&admin.Config{
+		Host: host,
+		Port: port,
+	})
+
+	go func() {
+		err := server.Serve()
+		require.Equal(t, http.ErrServerClosed, err)
+	}()
+
+	time.Sleep(time.Millisecond * 300)
+
+	resp, err := http.Get(fmt.Sprintf("http://%s:%s", host, port))
+	require.NoError(t, err)
+	require.Equal(t, "200 OK", resp.Status)
+
+	server.Shutdown()
+
+	resp, err = http.Get(fmt.Sprintf("http://%s:%s", host, port))
+	require.Error(t, err)
+}
+
+func TestHttpServerShutdownWithAPI(t *testing.T) {
+	dir, err := ioutil.TempDir("", "test_http_server")
+	require.NoError(t, err)
+	os.RemoveAll(dir)
+
+	host := "127.0.0.1"
+	port := findPort()
+	server := admin.NewServer(&admin.Config{
+		Host: host,
+		Port: port,
+		Jobs: dir,
+	})
+
+	go func() {
+		err := server.Serve()
+		require.Equal(t, http.ErrServerClosed, err)
+	}()
+
+	time.Sleep(time.Millisecond * 300)
+
+	resp, err := http.Get(fmt.Sprintf("http://%s:%s", host, port))
+	require.NoError(t, err)
+	require.Equal(t, "200 OK", resp.Status)
+
+	resp, err = http.Post(fmt.Sprintf("http://%s:%s/shutdown", host, port), "", nil)
+	require.NoError(t, err)
+	require.Equal(t, "200 OK", resp.Status)
+
+	time.Sleep(time.Millisecond * 1000)
+
+	resp, err = http.Get(fmt.Sprintf("http://%s:%s", host, port))
+	require.Error(t, err)
+}
+
+func findPort() string {
+	ln, err := net.Listen("tcp", ":0")
+	if err != nil {
+		panic(err)
+	}
+	port := ln.Addr().(*net.TCPAddr).Port
+	if err := ln.Close(); err != nil {
+		panic(err)
+	}
+	return fmt.Sprintf("%d", port)
+}

internal/admin/routes.go

@@ -19,13 +19,18 @@ func defaultRoutes(cfg *Config) []*route {
 				JobsDir: cfg.Jobs,
 			},
 		)},
-		{http.MethodGet, `^/([^/]+)$`, handlers.HandleGetJob(
+		{http.MethodGet, `^/jobs/?$`, handlers.HandleGetList(
+			&handlers.JobListHandlerConfig{
+				JobsDir: cfg.Jobs,
+			},
+		)},
+		{http.MethodGet, `^/jobs/([^/]+)$`, handlers.HandleGetJob(
 			&handlers.JobHandlerConfig{
 				JobsDir:            cfg.Jobs,
 				LogEncodingCharset: cfg.LogEncodingCharset,
 			},
 		)},
-		{http.MethodPost, `^/([^/]+)$`, handlers.HandlePostJobAction(
+		{http.MethodPost, `^/jobs/([^/]+)$`, handlers.HandlePostJobAction(
 			&handlers.PostJobHandlerConfig{
 				JobsDir: cfg.Jobs,
 				Bin:     cfg.Command,

internal/controller/controller.go

@@ -16,6 +16,7 @@ import (
 	"github.com/yohamta/jobctl/internal/models"
 	"github.com/yohamta/jobctl/internal/scheduler"
 	"github.com/yohamta/jobctl/internal/sock"
+	"github.com/yohamta/jobctl/internal/utils"
 )
 
 type Controller interface {
@@ -28,8 +29,12 @@ type Controller interface {
 }
 
 func GetJobList(dir string) (jobs []*Job, errs []string, err error) {
-	ret := []*Job{}
+	jobs = []*Job{}
 	errs = []string{}
+	if !utils.FileExists(dir) {
+		errs = append(errs, fmt.Sprintf("invalid jobs directory: %s", dir))
+		return
+	}
 	fis, err := ioutil.ReadDir(dir)
 	if err != nil {
 		log.Printf("%v", err)
@@ -46,9 +51,9 @@ func GetJobList(dir string) (jobs []*Job, errs []string, err error) {
 				continue
 			}
 		}
-		ret = append(ret, job)
+		jobs = append(jobs, job)
 	}
-	return ret, errs, nil
+	return jobs, errs, nil
 }
 
 var _ Controller = (*controller)(nil)

internal/controller/job.go

@@ -1,11 +1,13 @@
 package controller
 
 import (
+	"fmt"
 	"path/filepath"
 
 	"github.com/yohamta/jobctl/internal/config"
 	"github.com/yohamta/jobctl/internal/models"
 	"github.com/yohamta/jobctl/internal/scheduler"
+	"github.com/yohamta/jobctl/internal/utils"
 )
 
 type Job struct {
@@ -18,6 +20,9 @@ type Job struct {
 }
 
 func FromConfig(file string) (*Job, error) {
+	if !utils.FileExists(file) {
+		return nil, fmt.Errorf("file not found: %s", file)
+	}
 	return fromConfig(file, false)
 }
 

internal/sock/client.go

@@ -8,17 +8,23 @@ import (
 	"net"
 	"net/http"
 	"time"
+
+	"github.com/yohamta/jobctl/internal/utils"
 )
 
 var ErrTimeout = fmt.Errorf("unix socket timeout")
-var ErrConnectionRefused = fmt.Errorf("connection failed")
-var timeout = time.Millisecond * 500
+var ErrConnectionRefused = fmt.Errorf("unix socket connection failed")
+var ErrFileNotExist = fmt.Errorf("unix socket file does not exit")
+var timeout = time.Millisecond * 3000
 
 type Client struct {
 	Addr string
 }
 
 func (cl *Client) Request(method, url string) (string, error) {
+	if !utils.FileExists(cl.Addr) {
+		return "", fmt.Errorf("%w: %s", ErrFileNotExist, cl.Addr)
+	}
 	conn, err := net.DialTimeout("unix", cl.Addr, timeout)
 	if err != nil {
 		if err.(net.Error).Timeout() {

internal/sock/client_test.go

@@ -32,7 +32,7 @@ func TestDialTimeout(t *testing.T) {
 		&sock.Config{
 			Addr: f.Name(),
 			HandlerFunc: func(w http.ResponseWriter, r *http.Request) {
-				time.Sleep(time.Second * 1)
+				time.Sleep(time.Second * 3100)
 				w.WriteHeader(http.StatusOK)
 				w.Write([]byte("OK"))
 			},

internal/sock/server_test.go

@@ -1,6 +1,7 @@
 package sock_test
 
 import (
+	"errors"
 	"io/ioutil"
 	"net/http"
 	"os"
@@ -29,8 +30,8 @@ func TestMain(m *testing.M) {
 	os.Exit(code)
 }
 
-func TestStartServer(t *testing.T) {
-	tmpFile, err := ioutil.TempFile("", "test_server_start")
+func TestStartAndShutdownServer(t *testing.T) {
+	tmpFile, err := ioutil.TempFile("", "test_server_start_shutdown")
 	require.NoError(t, err)
 	defer os.Remove(tmpFile.Name())
 
@@ -53,11 +54,17 @@ func TestStartServer(t *testing.T) {
 
 	go func() {
 		err = unixServer.Serve(listen)
-		require.NoError(t, err)
+		assert.True(t, errors.Is(sock.ErrServerRequestedShutdown, err))
 	}()
 
 	time.Sleep(time.Second * 1)
 
 	ret, err := client.Request(http.MethodPost, "/")
 	assert.Equal(t, ret, "OK")
+
+	unixServer.Shutdown()
+
+	time.Sleep(time.Millisecond * 100)
+	_, err = client.Request(http.MethodPost, "/")
+	assert.True(t, errors.Is(err, sock.ErrFileNotExist))
 }