code cleanup

Author: anik120

catalogd/internal/serverutil/serverutil.go

@@ -95,7 +95,6 @@ func logrLoggingHandler(l logr.Logger, handler http.Handler) http.Handler {
 }
 
 func storageServerHandlerWrapped(mgr ctrl.Manager, cfg CatalogServerConfig) http.Handler {
-
 	handler := cfg.LocalStorage.StorageServerHandler()
 	handler = gzhttp.GzipHandler(handler)
 	handler = catalogdmetrics.AddMetricsToHandler(handler)

catalogd/internal/serverutil/serverutil_test.go

@@ -234,7 +234,10 @@ type mockStorageInstance struct {
 
 func (m *mockStorageInstance) StorageServerHandler() http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		w.Write([]byte(m.content))
+		_, err := w.Write([]byte(m.content))
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+		}
 	})
 }
 

catalogd/internal/storage/http_precoditions_check.go

@@ -24,7 +24,7 @@ const (
 
 // checkPreconditions evaluates request preconditions and reports whether a precondition
 // resulted in sending StatusNotModified or StatusPreconditionFailed.
-func checkPreconditions(w http.ResponseWriter, r *http.Request, modtime time.Time) (done bool) {
+func checkPreconditions(w http.ResponseWriter, r *http.Request, modtime time.Time) bool {
 	// This function carefully follows RFC 7232 section 6.
 	ch := checkIfMatch(r)
 	if ch == condNone {

catalogd/internal/storage/index.go

@@ -119,7 +119,7 @@ func (i *index) getSectionSet(schema, packageName, name string) sets.Set[section
 	}
 }
 
-func newIndex(metasChan <-chan *declcfg.Meta) (*index, error) {
+func newIndex(metasChan <-chan *declcfg.Meta) *index {
 	idx := &index{
 		BySchema:  make(map[string][]section),
 		ByPackage: make(map[string][]section),
@@ -142,5 +142,5 @@ func newIndex(metasChan <-chan *declcfg.Meta) (*index, error) {
 			idx.ByName[meta.Name] = append(idx.ByName[meta.Name], s)
 		}
 	}
-	return idx, nil
+	return idx
 }

catalogd/internal/storage/localdir.go

@@ -42,8 +42,8 @@ type LocalDirV1 struct {
 }
 
 var (
-	_                Instance = &LocalDirV1{}
-	ErrInvalidParams          = errors.New("invalid parameters")
+	_                Instance = (*LocalDirV1)(nil)
+	errInvalidParams          = errors.New("invalid parameters")
 )
 
 func (s *LocalDirV1) Store(ctx context.Context, catalog string, fsys fs.FS) error {
@@ -169,10 +169,7 @@ func storeCatalogData(catalogDir string, metas <-chan *declcfg.Meta) error {
 }
 
 func storeIndexData(catalogDir string, metas <-chan *declcfg.Meta) error {
-	idx, err := newIndex(metas)
-	if err != nil {
-		return err
-	}
+	idx := newIndex(metas)
 
 	f, err := os.Create(catalogIndexFilePath(catalogDir))
 	if err != nil {
@@ -209,7 +206,7 @@ func (s *LocalDirV1) handleV1All(w http.ResponseWriter, r *http.Request) {
 		httpError(w, err)
 		return
 	}
-	serveJsonLines(w, r, catalogStat.ModTime(), catalogFile)
+	serveJSONLines(w, r, catalogStat.ModTime(), catalogFile)
 }
 
 func (s *LocalDirV1) handleV1Query(w http.ResponseWriter, r *http.Request) {
@@ -236,7 +233,7 @@ func (s *LocalDirV1) handleV1Query(w http.ResponseWriter, r *http.Request) {
 
 	if schema == "" && pkg == "" && name == "" {
 		// If no parameters are provided, return the entire catalog (this is the same as /api/v1/all)
-		serveJsonLines(w, r, catalogStat.ModTime(), catalogFile)
+		serveJSONLines(w, r, catalogStat.ModTime(), catalogFile)
 		return
 	}
 	idx, err := s.getIndex(catalog)
@@ -249,7 +246,7 @@ func (s *LocalDirV1) handleV1Query(w http.ResponseWriter, r *http.Request) {
 		httpError(w, fs.ErrNotExist)
 		return
 	}
-	serveJsonLinesQuery(w, indexReader)
+	serveJSONLinesQuery(w, indexReader)
 }
 
 func (s *LocalDirV1) catalogData(catalog string) (*os.File, os.FileInfo, error) {
@@ -271,20 +268,20 @@ func httpError(w http.ResponseWriter, err error) {
 		code = http.StatusNotFound
 	case errors.Is(err, fs.ErrPermission):
 		code = http.StatusForbidden
-	case errors.Is(err, ErrInvalidParams):
+	case errors.Is(err, errInvalidParams):
 		code = http.StatusBadRequest
 	default:
 		code = http.StatusInternalServerError
 	}
 	http.Error(w, fmt.Sprintf("%d %s", code, http.StatusText(code)), code)
 }
 
-func serveJsonLines(w http.ResponseWriter, r *http.Request, modTime time.Time, rs io.ReadSeeker) {
+func serveJSONLines(w http.ResponseWriter, r *http.Request, modTime time.Time, rs io.ReadSeeker) {
 	w.Header().Add("Content-Type", "application/jsonl")
 	http.ServeContent(w, r, "", modTime, rs)
 }
 
-func serveJsonLinesQuery(w http.ResponseWriter, rs io.Reader) {
+func serveJSONLinesQuery(w http.ResponseWriter, rs io.Reader) {
 	w.Header().Add("Content-Type", "application/jsonl")
 	_, err := io.Copy(w, rs)
 	if err != nil {

catalogd/internal/storage/localdir_test.go

@@ -310,24 +310,24 @@ func TestQueryEndpoint(t *testing.T) {
 			expectedStatusCode: http.StatusOK,
 			expectedContent:    `{"image":"quaydock.io/namespace/bundle:0.0.3","name":"bundle.v0.0.1","package":"webhook_operator_test","properties":[{"type":"olm.bundle.object","value":{"data":"dW5pbXBvcnRhbnQK"}},{"type":"some.other","value":{"data":"arbitrary-info"}}],"relatedImages":[{"image":"testimage:latest","name":"test"}],"schema":"olm.bundle"}`,
 		},
-		// {
-		// 	name:               "valid query for package schema for a package that does not exist",
-		// 	queryParams:        "?schema=olm.package&name=not-present",
-		// 	expectedStatusCode: http.StatusOK,
-		// 	expectedContent:    "",
-		// },
+		{
+			name:               "valid query for package schema for a package that does not exist",
+			queryParams:        "?schema=olm.package&name=not-present",
+			expectedStatusCode: http.StatusOK,
+			expectedContent:    "",
+		},
 		{
 			name:               "valid query with package and name",
 			queryParams:        "?package=webhook_operator_test&name=bundle.v0.0.1",
 			expectedStatusCode: http.StatusOK,
 			expectedContent:    `{"image":"quaydock.io/namespace/bundle:0.0.3","name":"bundle.v0.0.1","package":"webhook_operator_test","properties":[{"type":"olm.bundle.object","value":{"data":"dW5pbXBvcnRhbnQK"}},{"type":"some.other","value":{"data":"arbitrary-info"}}],"relatedImages":[{"image":"testimage:latest","name":"test"}],"schema":"olm.bundle"}`,
 		},
-		// {
-		// 	name:               "invalid query with non-existent schema",
-		// 	queryParams:        "?schema=non_existent_schema",
-		// 	expectedStatusCode: http.StatusNotFound,
-		// 	expectedContent:    "400 Bad Request",
-		// },
+		{
+			name:               "query with non-existent schema",
+			queryParams:        "?schema=non_existent_schema",
+			expectedStatusCode: http.StatusOK,
+			expectedContent:    "",
+		},
 		{
 			name:               "cached response with If-Modified-Since",
 			queryParams:        "?schema=olm.package",
@@ -346,6 +346,7 @@ func TestQueryEndpoint(t *testing.T) {
 				// for the actual request
 				resp, err := http.DefaultClient.Do(req)
 				require.NoError(t, err)
+				resp.Body.Close()
 				req.Header.Set("If-Modified-Since", resp.Header.Get("Last-Modified"))
 			}
 			resp, err := http.DefaultClient.Do(req)
@@ -409,7 +410,7 @@ func TestServerLoadHandling(t *testing.T) {
 				}
 				for _, resp := range responses {
 					require.Equal(t, http.StatusOK, resp.StatusCode)
-					require.Equal(t, resp.Header.Get("Content-Type"), "application/jsonl")
+					require.Equal(t, "application/jsonl", resp.Header.Get("Content-Type"))
 					resp.Body.Close()
 				}
 			},