refactor(gateway, manager, resolver): clean up code and improve error handling (#42)

Author: aaronschweig

internal/gateway/gateway.go

@@ -3,13 +3,14 @@ package gateway
 import (
 	"errors"
 	"fmt"
+	"regexp"
+	"strings"
+
 	"github.com/go-openapi/spec"
 	"github.com/graphql-go/graphql"
 	"github.com/openmfp/crd-gql-gateway/internal/resolver"
 	"github.com/openmfp/golang-commons/logger"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"regexp"
-	"strings"
 )
 
 type Provider interface {
@@ -21,8 +22,7 @@ type Gateway struct {
 	resolver      resolver.Provider
 	graphqlSchema graphql.Schema
 
-	definitions   spec.Definitions
-	subscriptions graphql.Fields
+	definitions spec.Definitions
 
 	// typesCache stores generated GraphQL object types(fields) to prevent redundant repeated generation.
 	typesCache map[string]*graphql.Object
@@ -37,7 +37,6 @@ func New(log *logger.Logger, definitions spec.Definitions, resolver resolver.Pro
 		log:              log,
 		resolver:         resolver,
 		definitions:      definitions,
-		subscriptions:    graphql.Fields{},
 		typesCache:       make(map[string]*graphql.Object),
 		inputTypesCache:  make(map[string]*graphql.InputObject),
 		typeNameRegistry: make(map[string]string),
@@ -248,8 +247,7 @@ func (g *Gateway) generateGraphQLFields(resourceScheme *spec.Schema, typePrefix
 		}
 
 		fields[sanitizedFieldName] = &graphql.Field{
-			Type:    fieldType,
-			Resolve: g.resolver.UnstructuredFieldResolver(fieldName),
+			Type: fieldType,
 		}
 
 		inputFields[sanitizedFieldName] = &graphql.InputObjectFieldConfig{

internal/manager/manager.go

@@ -143,7 +143,6 @@ func (s *Service) OnFileDeleted(filename string) {
 	defer s.mu.Unlock()
 
 	delete(s.handlers, filename)
-
 }
 
 func (s *Service) loadSchemaFromFile(filename string) (*graphql.Schema, error) {
@@ -174,24 +173,35 @@ func (s *Service) createHandler(schema *graphql.Schema) *graphqlHandler {
 }
 
 func (s *Service) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-	workspace, endpoint, err := s.parsePath(r.URL.Path)
+	workspace, err := s.parsePath(r.URL.Path)
 	if err != nil {
+		s.log.Error().Err(err).Str("path", r.URL.Path).Msg("Error parsing path")
 		http.NotFound(w, r)
 		return
 	}
 
-	h, ok := s.getHandler(workspace)
+	s.mu.RLock()
+	h, ok := s.handlers[workspace]
+	s.mu.RUnlock()
+
 	if !ok {
+		s.log.Info().Str("workspace", workspace).Msg("no handler found for workspace")
 		http.NotFound(w, r)
 		return
 	}
 
-	if endpoint == "graphql" && r.Method == http.MethodGet {
+	if r.Method == http.MethodGet {
 		h.handler.ServeHTTP(w, r)
 		return
 	}
 
-	cfg, err := s.getConfigForRuntimeClient(workspace, r.Header.Get("Authorization"))
+	token := r.Header.Get("Authorization")
+	if token == "" {
+		http.Error(w, "Authorization header is required", http.StatusUnauthorized)
+		return
+	}
+
+	cfg, err := s.getConfigForRuntimeClient(workspace, token)
 	if err != nil {
 		writeJSONError(w, http.StatusInternalServerError, "Error getting a runtime client's config")
 		return
@@ -228,28 +238,17 @@ func writeJSONError(w http.ResponseWriter, status int, message string) {
 }
 
 // parsePath extracts filename and endpoint from the requested URL path.
-func (s *Service) parsePath(path string) (workspace, endpoint string, err error) {
+func (s *Service) parsePath(path string) (workspace string, err error) {
 	parts := strings.Split(strings.Trim(path, "/"), "/")
 	if len(parts) != 2 {
-		return "", "", fmt.Errorf("invalid path")
+		return "", fmt.Errorf("invalid path")
 	}
-	return parts[0], parts[1], nil
-}
 
-// getHandler retrieves the graphqlHandler associated with the given filename.
-func (s *Service) getHandler(filename string) (*graphqlHandler, bool) {
-	s.mu.RLock()
-	defer s.mu.RUnlock()
-	h, ok := s.handlers[filename]
-	return h, ok
+	return parts[0], nil
 }
 
 // getConfigForRuntimeClient initializes a runtime client for the given server address.
 func (s *Service) getConfigForRuntimeClient(workspace, token string) (*rest.Config, error) {
-	if token == "" { // if no token, use current-context
-		return s.restCfg, nil
-	}
-
 	requestConfig := rest.CopyConfig(s.restCfg)
 	requestConfig.BearerToken = token
 	u, err := url.Parse(s.restCfg.Host)
@@ -270,8 +269,6 @@ func (s *Service) handleSubscription(w http.ResponseWriter, r *http.Request, sch
 	w.Header().Set("Cache-Control", "no-cache")
 	w.Header().Set("Connection", "keep-alive")
 
-	flusher := http.NewResponseController(w)
-
 	var params struct {
 		Query         string                 `json:"query"`
 		OperationName string                 `json:"operationName"`
@@ -283,6 +280,10 @@ func (s *Service) handleSubscription(w http.ResponseWriter, r *http.Request, sch
 		return
 	}
 
+	flusher := http.NewResponseController(w)
+
+	r.Body.Close()
+
 	subscriptionParams := graphql.Params{
 		Schema:         *schema,
 		RequestString:  params.Query,
@@ -299,8 +300,10 @@ func (s *Service) handleSubscription(w http.ResponseWriter, r *http.Request, sch
 
 		data, err := json.Marshal(res)
 		if err != nil {
+			s.log.Error().Err(err).Msg("Error marshalling subscription response")
 			continue
 		}
+
 		fmt.Fprintf(w, "event: next\ndata: %s\n\n", data)
 		flusher.Flush()
 	}
@@ -309,13 +312,14 @@ func (s *Service) handleSubscription(w http.ResponseWriter, r *http.Request, sch
 }
 
 func readDefinitionFromFile(filePath string) (spec.Definitions, error) {
-	data, err := os.ReadFile(filePath)
+	f, err := os.Open(filePath)
 	if err != nil {
 		return nil, err
 	}
+	defer f.Close()
 
 	var swagger spec.Swagger
-	err = json.Unmarshal(data, &swagger)
+	err = json.NewDecoder(f).Decode(&swagger)
 	if err != nil {
 		return nil, err
 	}

internal/resolver/resolver.go

@@ -5,7 +5,6 @@ import (
 	"errors"
 	"fmt"
 	"regexp"
-	"sort"
 
 	"github.com/graphql-go/graphql"
 	"go.opentelemetry.io/otel"
@@ -47,7 +46,6 @@ type CrudProvider interface {
 
 type FieldResolverProvider interface {
 	CommonResolver() graphql.FieldResolveFn
-	UnstructuredFieldResolver(fieldName string) graphql.FieldResolveFn
 	SanitizeGroupName(string) string
 	GetOriginalGroupName(string) string
 }
@@ -71,38 +69,6 @@ func New(log *logger.Logger) *Service {
 	}
 }
 
-// UnstructuredFieldResolver returns a GraphQL FieldResolveFn to resolve a field from an unstructured object.
-func (r *Service) UnstructuredFieldResolver(fieldName string) graphql.FieldResolveFn {
-	return func(p graphql.ResolveParams) (interface{}, error) {
-		var objMap map[string]interface{}
-
-		switch source := p.Source.(type) {
-		case *unstructured.Unstructured:
-			objMap = source.Object
-		case unstructured.Unstructured:
-			objMap = source.Object
-		case map[string]interface{}:
-			objMap = source
-		default:
-			r.log.Error().
-				Str("type", fmt.Sprintf("%T", p.Source)).
-				Msg("Source is of unexpected type")
-			return nil, errors.New("source is of unexpected type")
-		}
-
-		value, found, err := unstructured.NestedFieldNoCopy(objMap, fieldName)
-		if err != nil {
-			r.log.Error().Err(err).Str("field", fieldName).Msg("Error retrieving field")
-			return nil, err
-		}
-		if !found {
-			return nil, nil
-		}
-
-		return value, nil
-	}
-}
-
 // ListItems returns a GraphQL CommonResolver function that lists Kubernetes resources of the given GroupVersionKind.
 func (r *Service) ListItems(gvk schema.GroupVersionKind) graphql.FieldResolveFn {
 	return func(p graphql.ResolveParams) (interface{}, error) {
@@ -130,20 +96,14 @@ func (r *Service) ListItems(gvk schema.GroupVersionKind) graphql.FieldResolveFn
 
 		// Create an unstructured list to hold the results
 		list := &unstructured.UnstructuredList{}
-		list.SetGroupVersionKind(schema.GroupVersionKind{
-			Group:   gvk.Group,
-			Version: gvk.Version,
-			Kind:    gvk.Kind + "List",
-		})
+		list.SetGroupVersionKind(gvk)
 
 		var opts []client.ListOption
 		// Handle label selector argument
 		if labelSelector, ok := p.Args[labelSelectorArg].(string); ok && labelSelector != "" {
 			selector, err := labels.Parse(labelSelector)
 			if err != nil {
-				log.Error().Err(err).
-					Str(labelSelectorArg, labelSelector).
-					Msg("Unable to parse given label selector")
+				log.Error().Err(err).Str(labelSelectorArg, labelSelector).Msg("Unable to parse given label selector")
 				return nil, err
 			}
 			opts = append(opts, client.MatchingLabelsSelector{Selector: selector})
@@ -155,18 +115,14 @@ func (r *Service) ListItems(gvk schema.GroupVersionKind) graphql.FieldResolveFn
 		}
 
 		if err = runtimeClient.List(ctx, list, opts...); err != nil {
-			log.Error().
-				Err(err).
-				Msg("Unable to list objects")
+			log.Error().Err(err).Msg("Unable to list objects")
 			return nil, err
 		}
 
-		items := list.Items
-
-		// Sort the items by name for consistent ordering
-		sort.Slice(items, func(i, j int) bool {
-			return items[i].GetName() < items[j].GetName()
-		})
+		items := make([]map[string]any, len(list.Items))
+		for i, item := range list.Items {
+			items[i] = item.Object
+		}
 
 		return items, nil
 	}
@@ -198,39 +154,32 @@ func (r *Service) GetItem(gvk schema.GroupVersionKind) graphql.FieldResolveFn {
 		}
 
 		// Retrieve required arguments
-		name, nameOK := p.Args["name"].(string)
-		namespace, nsOK := p.Args["namespace"].(string)
-
-		if !nameOK || name == "" {
-			err := errors.New("missing required argument: name")
-			log.Error().Err(err).Msg("Name argument is required")
+		name, ok := p.Args["name"].(string)
+		if !ok || name == "" {
+			log.Error().Err(errors.New("missing required argument: name")).Msg("Name argument is required")
 			return nil, err
 		}
-		if !nsOK || namespace == "" {
-			err := errors.New("missing required argument: namespace")
-			log.Error().Err(err).Msg("Namespace argument is required")
+
+		namespace, ok := p.Args["namespace"].(string)
+		if !ok || namespace == "" {
+			log.Error().Err(errors.New("missing required argument: namespace")).Msg("Namespace argument is required")
 			return nil, err
 		}
 
 		// Create an unstructured object to hold the result
 		obj := &unstructured.Unstructured{}
 		obj.SetGroupVersionKind(gvk)
 
-		key := client.ObjectKey{
+		// Get the object using the runtime client
+		if err = runtimeClient.Get(ctx, client.ObjectKey{
 			Namespace: namespace,
 			Name:      name,
-		}
-
-		// Get the object using the runtime client
-		if err = runtimeClient.Get(ctx, key, obj); err != nil {
-			log.Error().Err(err).
-				Str("name", name).
-				Str("namespace", namespace).
-				Msg("Unable to get object")
+		}, obj); err != nil {
+			log.Error().Err(err).Str("name", name).Str("namespace", namespace).Msg("Unable to get object")
 			return nil, err
 		}
 
-		return obj, nil
+		return obj.Object, nil
 	}
 }
 
@@ -266,7 +215,7 @@ func (r *Service) CreateItem(gvk schema.GroupVersionKind) graphql.FieldResolveFn
 			return nil, err
 		}
 
-		return obj, nil
+		return obj.Object, nil
 	}
 }
 
@@ -317,7 +266,7 @@ func (r *Service) UpdateItem(gvk schema.GroupVersionKind) graphql.FieldResolveFn
 			return nil, err
 		}
 
-		return existingObj, nil
+		return existingObj.Object, nil
 	}
 }
 
@@ -355,10 +304,6 @@ func (r *Service) DeleteItem(gvk schema.GroupVersionKind) graphql.FieldResolveFn
 
 func (r *Service) CommonResolver() graphql.FieldResolveFn {
 	return func(p graphql.ResolveParams) (interface{}, error) {
-		if p.Source == nil {
-			// At the example level, return a non-nil value (e.g., an empty map)
-			return map[string]interface{}{}, nil
-		}
 		return p.Source, nil
 	}
 }