status codes for discovery client (#3513)

* status codes for discovery client

* pr feedback

* pr feedback

discovery/api/server/client/http.go

@@ -21,7 +21,6 @@ package client
 import (
 	"bytes"
 	"context"
-	"crypto/tls"
 	"encoding/json"
 	"fmt"
 	"github.com/nuts-foundation/go-did/vc"
@@ -35,9 +34,9 @@ import (
 )
 
 // New creates a new DefaultHTTPClient.
-func New(strictMode bool, timeout time.Duration, tlsConfig *tls.Config) *DefaultHTTPClient {
+func New(timeout time.Duration) *DefaultHTTPClient {
 	return &DefaultHTTPClient{
-		client: client.NewWithTLSConfig(timeout, tlsConfig),
+		client: client.New(timeout),
 	}
 }
 

discovery/api/server/client/http_test.go

@@ -40,7 +40,7 @@ func TestHTTPInvoker_Register(t *testing.T) {
 	t.Run("ok", func(t *testing.T) {
 		handler := &testHTTP.Handler{StatusCode: http.StatusCreated}
 		server := httptest.NewServer(handler)
-		client := New(false, time.Minute, server.TLS)
+		client := New(time.Minute)
 
 		err := client.Register(context.Background(), server.URL, vp)
 
@@ -51,7 +51,7 @@ func TestHTTPInvoker_Register(t *testing.T) {
 	})
 	t.Run("non-ok with problem details", func(t *testing.T) {
 		server := httptest.NewServer(&testHTTP.Handler{StatusCode: http.StatusBadRequest, ResponseData: `{"title":"missing credentials", "status":400, "detail":"could not resolve DID"}`})
-		client := New(false, time.Minute, server.TLS)
+		client := New(time.Minute)
 
 		err := client.Register(context.Background(), server.URL, vp)
 
@@ -61,7 +61,7 @@ func TestHTTPInvoker_Register(t *testing.T) {
 	})
 	t.Run("non-ok other", func(t *testing.T) {
 		server := httptest.NewServer(&testHTTP.Handler{StatusCode: http.StatusNotFound, ResponseData: `not found`})
-		client := New(false, time.Minute, server.TLS)
+		client := New(time.Minute)
 
 		err := client.Register(context.Background(), server.URL, vp)
 
@@ -84,7 +84,7 @@ func TestHTTPInvoker_Get(t *testing.T) {
 			"timestamp": 1,
 		}
 		server := httptest.NewServer(handler)
-		client := New(false, time.Minute, server.TLS)
+		client := New(time.Minute)
 
 		presentations, seed, timestamp, err := client.Get(context.Background(), server.URL, 0)
 
@@ -102,7 +102,7 @@ func TestHTTPInvoker_Get(t *testing.T) {
 			"timestamp": 1,
 		}
 		server := httptest.NewServer(handler)
-		client := New(false, time.Minute, server.TLS)
+		client := New(time.Minute)
 
 		presentations, _, timestamp, err := client.Get(context.Background(), server.URL, 1)
 
@@ -120,7 +120,7 @@ func TestHTTPInvoker_Get(t *testing.T) {
 			writer.Write([]byte("{}"))
 		}
 		server := httptest.NewServer(http.HandlerFunc(handler))
-		client := New(false, time.Minute, server.TLS)
+		client := New(time.Minute)
 
 		_, _, _, err := client.Get(context.Background(), server.URL, 0)
 
@@ -130,7 +130,7 @@ func TestHTTPInvoker_Get(t *testing.T) {
 	t.Run("server returns invalid status code", func(t *testing.T) {
 		handler := &testHTTP.Handler{StatusCode: http.StatusInternalServerError, ResponseData: `{"title":"internal server error", "status":500, "detail":"db not found"}`}
 		server := httptest.NewServer(handler)
-		client := New(false, time.Minute, server.TLS)
+		client := New(time.Minute)
 
 		_, _, _, err := client.Get(context.Background(), server.URL, 0)
 
@@ -142,7 +142,7 @@ func TestHTTPInvoker_Get(t *testing.T) {
 		handler := &testHTTP.Handler{StatusCode: http.StatusOK}
 		handler.ResponseData = "not json"
 		server := httptest.NewServer(handler)
-		client := New(false, time.Minute, server.TLS)
+		client := New(time.Minute)
 
 		_, _, _, err := client.Get(context.Background(), server.URL, 0)
 

discovery/api/v1/api.go

@@ -121,6 +121,9 @@ func (w *Wrapper) ActivateServiceForSubject(ctx context.Context, request Activat
 func (w *Wrapper) DeactivateServiceForSubject(ctx context.Context, request DeactivateServiceForSubjectRequestObject) (DeactivateServiceForSubjectResponseObject, error) {
 	err := w.Client.DeactivateServiceForSubject(ctx, request.ServiceID, request.SubjectID)
 	if err != nil {
+		if errors.Is(err, discovery.ErrPresentationRegistrationFailed) {
+			return DeactivateServiceForSubject202JSONResponse{Reason: err.Error()}, nil
+		}
 		return nil, err
 	}
 	return DeactivateServiceForSubject200Response{}, nil
@@ -132,19 +135,24 @@ func (w *Wrapper) GetServices(_ context.Context, _ GetServicesRequestObject) (Ge
 }
 
 func (w *Wrapper) GetServiceActivation(ctx context.Context, request GetServiceActivationRequestObject) (GetServiceActivationResponseObject, error) {
-	response := GetServiceActivation200JSONResponse{
-		Status: ServiceStatusActive,
-	}
+	response := GetServiceActivation200JSONResponse{}
 	activated, presentations, err := w.Client.GetServiceActivation(ctx, request.ServiceID, request.SubjectID)
 	if err != nil {
 		if !errors.As(err, &discovery.RegistrationRefreshError{}) {
 			return nil, err
 		}
-		response.Status = ServiceStatusError
+		response.Status = to.Ptr(ServiceStatusError)
 		response.Error = to.Ptr(err.Error())
 	}
 	response.Activated = activated
-	response.Vp = &presentations
+	if activated && response.Status == nil {
+		// only set if not already set to ServiceStatusError
+		response.Status = to.Ptr(ServiceStatusActive)
+	}
+	if presentations != nil {
+		// if presentations is nil this would add `"vp":null` to the response
+		response.Vp = &presentations
+	}
 
 	return response, nil
 }

discovery/api/v1/api_test.go

@@ -108,6 +108,19 @@ func TestWrapper_DeactivateServiceForSubject(t *testing.T) {
 		assert.NoError(t, err)
 		assert.IsType(t, DeactivateServiceForSubject200Response{}, response)
 	})
+	t.Run("server error", func(t *testing.T) {
+		test := newMockContext(t)
+		expectedErr := errors.Join(discovery.ErrPresentationRegistrationFailed, errors.New("custom error"))
+		test.client.EXPECT().DeactivateServiceForSubject(gomock.Any(), serviceID, subjectID).Return(expectedErr)
+
+		response, err := test.wrapper.DeactivateServiceForSubject(nil, DeactivateServiceForSubjectRequestObject{
+			ServiceID: serviceID,
+			SubjectID: subjectID,
+		})
+
+		assert.NoError(t, err)
+		assert.IsType(t, DeactivateServiceForSubject202JSONResponse{Reason: expectedErr.Error()}, response)
+	})
 	t.Run("error", func(t *testing.T) {
 		test := newMockContext(t)
 		test.client.EXPECT().DeactivateServiceForSubject(gomock.Any(), serviceID, subjectID).Return(errors.New("foo"))
@@ -199,7 +212,7 @@ func TestWrapper_GetServiceActivation(t *testing.T) {
 		assert.NoError(t, err)
 		require.IsType(t, GetServiceActivation200JSONResponse{}, response)
 		assert.True(t, response.(GetServiceActivation200JSONResponse).Activated)
-		assert.Equal(t, ServiceStatusActive, string(response.(GetServiceActivation200JSONResponse).Status))
+		assert.Equal(t, ServiceStatusActive, *response.(GetServiceActivation200JSONResponse).Status)
 		assert.Nil(t, response.(GetServiceActivation200JSONResponse).Error)
 		assert.Empty(t, response.(GetServiceActivation200JSONResponse).Vp)
 	})
@@ -215,7 +228,7 @@ func TestWrapper_GetServiceActivation(t *testing.T) {
 		assert.NoError(t, err)
 		require.IsType(t, GetServiceActivation200JSONResponse{}, response)
 		assert.True(t, response.(GetServiceActivation200JSONResponse).Activated)
-		assert.Equal(t, ServiceStatusError, string(response.(GetServiceActivation200JSONResponse).Status))
+		assert.Equal(t, ServiceStatusError, *response.(GetServiceActivation200JSONResponse).Status)
 		assert.NotNil(t, response.(GetServiceActivation200JSONResponse).Error)
 		assert.Empty(t, response.(GetServiceActivation200JSONResponse).Vp)
 	})

discovery/api/v1/types.go

@@ -37,7 +37,7 @@ type GetServiceActivation200JSONResponseStatus string
 
 const (
 	// ServiceStatusActive is the status for an active service.
-	ServiceStatusActive = "active"
+	ServiceStatusActive GetServiceActivation200JSONResponseStatus = "active"
 	// ServiceStatusError is the status for an inactive service.
-	ServiceStatusError = "error"
+	ServiceStatusError GetServiceActivation200JSONResponseStatus = "error"
 )