Repository: TBD54566975/ssi-service
Pull request #450 (2023-05-19 23:21:48)

Add integration test for webhook and fix broken middleware setup. 
The test added failed before, and passed after. 

codecov-commenter: ## [Codecov](https://app.codecov.io/gh/TBD54566975/ssi-service/pull/450?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) Report
> Merging [#450](https://app.codecov.io/gh/TBD54566975/ssi-service/pull/450?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) (96278ac) into [main](https://app.codecov.io/gh/TBD54566975/ssi-service/commit/12083e75bcac0d8e18b03bd4d7957fe83873a964?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) (12083e7) will **decrease** coverage by `0.06%`.
> The diff coverage is `0.00%`.

```diff
@@            Coverage Diff             @@
##             main     #450      +/-   ##
==========================================
- Coverage   21.46%   21.41%   -0.06%     
==========================================
  Files          49       49              
  Lines        5375     5388      +13     
==========================================
  Hits         1154     1154              
- Misses       4037     4050      +13     
  Partials      184      184              
```


| [Impacted Files](https://app.codecov.io/gh/TBD54566975/ssi-service/pull/450?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) | Coverage Δ | |
|---|---|---|
| [integration/common.go](https://app.codecov.io/gh/TBD54566975/ssi-service/pull/450?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975#diff-aW50ZWdyYXRpb24vY29tbW9uLmdv) | `1.57% <0.00%> (-0.07%)` | :arrow_down: |


decentralgabe: looks like this is failing in the integration test `/home/runner/work/ssi-service/ssi-service/integration/didweb_integration_test.go:50`

we may want to set the logs in the `integration` pkg to `debug` instead of info it's pretty hard to see what's going on

Patch:
diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml
index 2e562a0..c018ed1 100644
--- a/.github/workflows/integration.yml
+++ b/.github/workflows/integration.yml
@@ -32,16 +32,18 @@ jobs:
     - name: End To End Test
       run: mage integration
 
-    - name: Save Container Logs to File
+    - name: Save Container Logs to Directory
       if: failure()
-      run: docker logs ssi > ssi_logs.txt
+      run: |
+        docker logs ssi > log/ssi_logs.txt
+        docker cp ssi:/app/log .
 
     - name: Upload Logs as Artifact
       if: failure()
       uses: actions/upload-artifact@v3
       with:
         name: logs
-        path: ssi_logs.txt
+        path: log/*
 
     - name: Stop containers
       if: always()
diff --git a/cmd/ssiservice/main.go b/cmd/ssiservice/main.go
index 8eb764f..afb5ba4 100644
--- a/cmd/ssiservice/main.go
+++ b/cmd/ssiservice/main.go
@@ -5,6 +5,7 @@ import (
 	"expvar"
 	"os"
 	"os/signal"
+	"strconv"
 	"syscall"
 	"time"
 
@@ -200,8 +201,9 @@ func configureLogger(level, location string) *os.File {
 	logrus.SetOutput(os.Stdout)
 
 	// set logs config from config file
+	now := time.Now()
 	if location != "" {
-		logFile := location + "/" + config.ServiceName + "-" + time.Now().Format(time.RFC3339) + ".log"
+		logFile := location + "/" + config.ServiceName + "-" + now.Format(time.DateOnly) + "-" + strconv.FormatInt(now.Unix(), 10) + ".log"
 		file, err := os.OpenFile(logFile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666)
 		if err != nil {
 			logrus.WithError(err).Warn("failed to create logs file, using default stdout")
diff --git a/config/dev.toml b/config/dev.toml
index 883d704..eeb8c8f 100644
--- a/config/dev.toml
+++ b/config/dev.toml
@@ -16,7 +16,7 @@ read_timeout = 5000000000
 write_timeout = 5000000000
 shutdown_timeout = 5000000000
 
-log_location = "log"
+log_location = ""
 # options: trace, debug, info, warn, error, fatal, panic
 log_level = "debug"
 
diff --git a/pkg/server/framework/server.go b/pkg/server/framework/server.go
index 27b324e..7203e4c 100644
--- a/pkg/server/framework/server.go
+++ b/pkg/server/framework/server.go
@@ -82,9 +82,6 @@ func NewHTTPServer(cfg config.ServerConfig, shutdown chan os.Signal, mws gin.Han
 // Handle sets a handler function for a given HTTP method and path pair
 // to the server mux.
 func (s *Server) Handle(method string, path string, handler Handler, middleware ...gin.HandlerFunc) {
-	// add the middleware to the router
-	s.router.Use(middleware...)
-
 	// request handler function
 	h := func(c *gin.Context) {
 		requestState := RequestState{
@@ -126,8 +123,12 @@ func (s *Server) Handle(method string, path string, handler Handler, middleware
 		}
 	}
 
+	hs := make(gin.HandlersChain, 0, len(middleware)+1)
+	hs = append(hs, middleware...)
+	hs = append(hs, h)
+
 	// add the handler to the router
-	s.router.Handle(method, path, h)
+	s.router.Handle(method, path, hs...)
 }
 
 // SignalShutdown is used to gracefully shut down the server when an integrity issue is identified.
diff --git a/pkg/service/webhook/service.go b/pkg/service/webhook/service.go
index 5036f3c..910da98 100644
--- a/pkg/service/webhook/service.go
+++ b/pkg/service/webhook/service.go
@@ -235,5 +235,5 @@ func (s Service) post(ctx context.Context, url string, json string) error {
 }
 
 func is2xxResponse(statusCode int) bool {
-	return statusCode/100 != 2
+	return statusCode/100 == 2
 }


Test patch:
diff --git a/pkg/server/server_webhook_test.go b/pkg/server/server_webhook_test.go
index d1a4ddf..61f6897 100644
--- a/pkg/server/server_webhook_test.go
+++ b/pkg/server/server_webhook_test.go
@@ -1,19 +1,131 @@
 package server
 
 import (
+	"bytes"
 	"context"
+	"fmt"
+	"io"
+	"net"
 	"net/http"
 	"net/http/httptest"
+	"os"
+	"strconv"
 	"testing"
+	"time"
 
 	"github.com/goccy/go-json"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-
+	"github.com/tbd54566975/ssi-service/config"
 	"github.com/tbd54566975/ssi-service/pkg/server/router"
 	"github.com/tbd54566975/ssi-service/pkg/service/webhook"
+	"github.com/tbd54566975/ssi-service/pkg/storage"
 )
 
+func freePort() string {
+	listener, err := net.Listen("tcp", "localhost:0")
+	if err != nil {
+		fmt.Println("Failed to listen:", err)
+		return ""
+	}
+	defer func(listener net.Listener) {
+		_ = listener.Close()
+	}(listener)
+	return strconv.Itoa(listener.Addr().(*net.TCPAddr).Port)
+}
+
+func TestSimpleWebhook(t *testing.T) {
+	ch := make(chan struct{}, 10)
+	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		ch <- struct{}{}
+	}))
+	defer testServer.Close()
+
+	shutdown := make(chan os.Signal, 1)
+	serviceConfig, err := config.LoadConfig("")
+	assert.NoError(t, err)
+
+	serviceConfig.Server.APIHost = "0.0.0.0:" + freePort()
+	name := tempBoltFileName(t)
+	serviceConfig.Services.StorageOptions = append(serviceConfig.Services.StorageOptions, storage.Option{
+		ID:     "boltdb-filepath-option",
+		Option: name,
+	})
+
+	server, err := NewSSIServer(shutdown, *serviceConfig)
+	assert.NoError(t, err)
+
+	go func() {
+		require.ErrorIs(t, server.ListenAndServe(), http.ErrServerClosed)
+	}()
+
+	require.Eventually(t, isHealthy(t, server), 30*time.Second, 100*time.Millisecond)
+
+	webhookRequest := router.CreateWebhookRequest{
+		Noun: "DID",
+		Verb: "Create",
+		URL:  testServer.URL,
+	}
+	requestData, err := json.Marshal(webhookRequest)
+	assert.NoError(t, err)
+
+	put(t, server, "/v1/webhooks", requestData)
+
+	createRequest := []byte(`{
+		"keyType":"Ed25519",
+		"options": {
+			"didWebId": "did:web:tbd.website"
+		}
+	}`)
+	put(t, server, "/v1/dids/web", createRequest)
+
+	// Check that exactly one call was received after 2 seconds.
+	select {
+	case <-ch:
+	case <-time.After(2 * time.Second):
+		assert.Fail(t, "should receive at least 1 message")
+	}
+	select {
+	case <-ch:
+		assert.Fail(t, "should not receive more than 1 message")
+	case <-time.After(2 * time.Second):
+	}
+
+	assert.NoError(t, server.Close())
+}
+
+func tempBoltFileName(t *testing.T) string {
+	file, err := os.CreateTemp("", "bolt")
+	require.NoError(t, err)
+	name := file.Name()
+	t.Cleanup(func() {
+		_ = file.Close()
+		_ = os.Remove(name)
+	})
+	return name
+}
+
+func isHealthy(t *testing.T, server *SSIServer) func() bool {
+	return func() bool {
+		resp, err := http.Get("http://" + server.Addr + "/health")
+		require.NoError(t, err)
+		return resp.StatusCode == 200
+	}
+}
+
+func put(t *testing.T, server *SSIServer, endpoint string, data []byte) {
+	request, err := http.NewRequest(http.MethodPut, "http://"+server.Addr+endpoint, bytes.NewReader(data))
+	assert.NoError(t, err)
+	request.Header.Set("Content-Type", "application/json")
+	res, err := http.DefaultClient.Do(request)
+	assert.NoError(t, err)
+	assert.Equal(t, 2, res.StatusCode/100)
+	body, err := io.ReadAll(res.Body)
+	assert.NoError(t, err)
+
+	assert.NotEmpty(t, string(body))
+}
+
 func TestWebhookAPI(t *testing.T) {
 	t.Run("CreateWebhook returns error when missing request", func(tt *testing.T) {
 		db := setupTestDB(tt)
@@ -191,9 +303,9 @@ func TestWebhookAPI(t *testing.T) {
 
 		webhookService := testWebhookService(tt, db)
 
-		webhook, err := webhookService.GetWebhook(context.Background(), webhook.GetWebhookRequest{Noun: "Credential", Verb: "Create"})
+		wh, err := webhookService.GetWebhook(context.Background(), webhook.GetWebhookRequest{Noun: "Credential", Verb: "Create"})
 		assert.ErrorContains(tt, err, "webhook does not exist")
-		assert.Nil(tt, webhook)
+		assert.Nil(tt, wh)
 	})
 
 	t.Run("GetWebhook Returns Webhook That Does Exist", func(tt *testing.T) {