[chore/bugfix] Domain block tidying up, Implement first pass of 207 Multi-Status (#1886)

* [chore/refactor] update domain block processing

* expose domain block import errors a lil better

* move/remove unused query keys

internal/api/client/admin/admin.go

@@ -42,8 +42,6 @@ const (
 	EmailPath              = BasePath + "/email"
 	EmailTestPath          = EmailPath + "/test"
 
-	ExportQueryKey        = "export"
-	ImportQueryKey        = "import"
 	IDKey                 = "id"
 	FilterQueryKey        = "filter"
 	MaxShortcodeDomainKey = "max_shortcode_domain"

internal/api/client/admin/domainblockcreate.go

@@ -21,7 +21,6 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
-	"strconv"
 
 	"github.com/gin-gonic/gin"
 	apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model"
@@ -140,48 +139,78 @@ func (m *Module) DomainBlocksPOSTHandler(c *gin.Context) {
 		return
 	}
 
-	imp := false
-	importString := c.Query(ImportQueryKey)
-	if importString != "" {
-		i, err := strconv.ParseBool(importString)
-		if err != nil {
-			err := fmt.Errorf("error parsing %s: %s", ImportQueryKey, err)
-			apiutil.ErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGetV1)
-			return
-		}
-		imp = i
+	importing, errWithCode := apiutil.ParseDomainBlockImport(c.Query(apiutil.DomainBlockImportKey), false)
+	if errWithCode != nil {
+		apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
+		return
 	}
 
-	form := &apimodel.DomainBlockCreateRequest{}
+	form := new(apimodel.DomainBlockCreateRequest)
 	if err := c.ShouldBind(form); err != nil {
 		apiutil.ErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGetV1)
 		return
 	}
 
-	if err := validateCreateDomainBlock(form, imp); err != nil {
-		err := fmt.Errorf("error validating form: %s", err)
+	if err := validateCreateDomainBlock(form, importing); err != nil {
+		err := fmt.Errorf("error validating form: %w", err)
 		apiutil.ErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGetV1)
 		return
 	}
 
-	if imp {
-		// we're importing multiple blocks
-		domainBlocks, errWithCode := m.processor.Admin().DomainBlocksImport(c.Request.Context(), authed.Account, form.Domains)
+	if !importing {
+		// Single domain block creation.
+		domainBlock, errWithCode := m.processor.Admin().DomainBlockCreate(
+			c.Request.Context(),
+			authed.Account,
+			form.Domain,
+			form.Obfuscate,
+			form.PublicComment,
+			form.PrivateComment,
+			"", // No sub ID for single block creation.
+		)
 		if errWithCode != nil {
 			apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
 			return
 		}
-		c.JSON(http.StatusOK, domainBlocks)
+
+		c.JSON(http.StatusOK, domainBlock)
 		return
 	}
 
-	// we're just creating one block
-	domainBlock, errWithCode := m.processor.Admin().DomainBlockCreate(c.Request.Context(), authed.Account, form.Domain, form.Obfuscate, form.PublicComment, form.PrivateComment, "")
+	// We're importing multiple domain blocks,
+	// so we're looking at a multi-status response.
+	multiStatus, errWithCode := m.processor.Admin().DomainBlocksImport(
+		c.Request.Context(),
+		authed.Account,
+		form.Domains, // Pass the file through.
+	)
 	if errWithCode != nil {
 		apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
 		return
 	}
-	c.JSON(http.StatusOK, domainBlock)
+
+	// TODO: Return 207 and multiStatus data nicely
+	//       when supported by the admin panel.
+
+	if multiStatus.Metadata.Failure != 0 {
+		failures := make(map[string]any, multiStatus.Metadata.Failure)
+		for _, entry := range multiStatus.Data {
+			// nolint:forcetypeassert
+			failures[entry.Resource.(string)] = entry.Message
+		}
+
+		err := fmt.Errorf("one or more errors importing domain blocks: %+v", failures)
+		apiutil.ErrorHandler(c, gtserror.NewErrorUnprocessableEntity(err, err.Error()), m.processor.InstanceGetV1)
+		return
+	}
+
+	// Success, return slice of domain blocks.
+	domainBlocks := make([]any, 0, multiStatus.Metadata.Success)
+	for _, entry := range multiStatus.Data {
+		domainBlocks = append(domainBlocks, entry.Resource)
+	}
+
+	c.JSON(http.StatusOK, domainBlocks)
 }
 
 func validateCreateDomainBlock(form *apimodel.DomainBlockCreateRequest, imp bool) error {

internal/api/client/admin/domainblockget.go

@@ -18,10 +18,8 @@
 package admin
 
 import (
-	"errors"
 	"fmt"
 	"net/http"
-	"strconv"
 
 	"github.com/gin-gonic/gin"
 	apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util"
@@ -87,26 +85,19 @@ func (m *Module) DomainBlockGETHandler(c *gin.Context) {
 		return
 	}
 
-	domainBlockID := c.Param(IDKey)
-	if domainBlockID == "" {
-		err := errors.New("no domain block id specified")
-		apiutil.ErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGetV1)
+	domainBlockID, errWithCode := apiutil.ParseID(c.Param(apiutil.IDKey))
+	if errWithCode != nil {
+		apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
 		return
 	}
 
-	export := false
-	exportString := c.Query(ExportQueryKey)
-	if exportString != "" {
-		i, err := strconv.ParseBool(exportString)
-		if err != nil {
-			err := fmt.Errorf("error parsing %s: %s", ExportQueryKey, err)
-			apiutil.ErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGetV1)
-			return
-		}
-		export = i
+	export, errWithCode := apiutil.ParseDomainBlockExport(c.Query(apiutil.DomainBlockExportKey), false)
+	if errWithCode != nil {
+		apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
+		return
 	}
 
-	domainBlock, errWithCode := m.processor.Admin().DomainBlockGet(c.Request.Context(), authed.Account, domainBlockID, export)
+	domainBlock, errWithCode := m.processor.Admin().DomainBlockGet(c.Request.Context(), domainBlockID, export)
 	if errWithCode != nil {
 		apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
 		return

internal/api/client/admin/domainblocksget.go

@@ -20,7 +20,6 @@ package admin
 import (
 	"fmt"
 	"net/http"
-	"strconv"
 
 	"github.com/gin-gonic/gin"
 	apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util"
@@ -92,16 +91,10 @@ func (m *Module) DomainBlocksGETHandler(c *gin.Context) {
 		return
 	}
 
-	export := false
-	exportString := c.Query(ExportQueryKey)
-	if exportString != "" {
-		i, err := strconv.ParseBool(exportString)
-		if err != nil {
-			err := fmt.Errorf("error parsing %s: %s", ExportQueryKey, err)
-			apiutil.ErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGetV1)
-			return
-		}
-		export = i
+	export, errWithCode := apiutil.ParseDomainBlockExport(c.Query(apiutil.DomainBlockExportKey), false)
+	if errWithCode != nil {
+		apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
+		return
 	}
 
 	domainBlocks, errWithCode := m.processor.Admin().DomainBlocksGet(c.Request.Context(), authed.Account, export)

internal/api/model/multistatus.go

@@ -0,0 +1,90 @@
+// GoToSocial
+// Copyright (C) GoToSocial Authors admin@gotosocial.org
+// SPDX-License-Identifier: AGPL-3.0-or-later
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package model
+
+// MultiStatus models a multistatus HTTP response body.
+// This model should be transmitted along with http code
+// 207 MULTI-STATUS to indicate a mixture of responses.
+// See https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/207
+//
+// swagger:model multiStatus
+type MultiStatus struct {
+	Data     []MultiStatusEntry  `json:"data"`
+	Metadata MultiStatusMetadata `json:"metadata"`
+}
+
+// MultiStatusEntry models one entry in multistatus data.
+// It can model either a success or a failure. The type
+// and value of `Resource` is left to the discretion of
+// the caller, but at minimum it should be expected to be
+// JSON-serializable.
+//
+// swagger:model multiStatusEntry
+type MultiStatusEntry struct {
+	// The resource/result for this entry.
+	// Value may be any type, check the docs
+	// per endpoint to see which to expect.
+	Resource any `json:"resource"`
+	// Message/error message for this entry.
+	Message string `json:"message"`
+	// HTTP status code of this entry.
+	Status int `json:"status"`
+}
+
+// MultiStatusMetadata models an at-a-glance summary of
+// the data contained in the MultiStatus.
+//
+// swagger:model multiStatusMetadata
+type MultiStatusMetadata struct {
+	// Success count + failure count.
+	Total int `json:"total"`
+	// Count of successful results (2xx).
+	Success int `json:"success"`
+	// Count of unsuccessful results (!2xx).
+	Failure int `json:"failure"`
+}
+
+// NewMultiStatus returns a new MultiStatus API model with
+// the provided entries, which will be iterated through to
+// look for 2xx and non 2xx status codes, in order to count
+// successes and failures.
+func NewMultiStatus(entries []MultiStatusEntry) *MultiStatus {
+	var (
+		successCount int
+		failureCount int
+		total        = len(entries)
+	)
+
+	for _, e := range entries {
+		// Outside 2xx range = failure.
+		if e.Status > 299 || e.Status < 200 {
+			failureCount++
+		} else {
+			successCount++
+		}
+	}
+
+	return &MultiStatus{
+		Data: entries,
+		Metadata: MultiStatusMetadata{
+			Total:   total,
+			Success: successCount,
+			Failure: failureCount,
+		},
+	}
+}

internal/api/util/parsequery.go

@@ -27,6 +27,7 @@ import (
 const (
 	/* Common keys */
 
+	IDKey    = "id"
 	LimitKey = "limit"
 	LocalKey = "local"
 	MaxIDKey = "max_id"
@@ -41,6 +42,11 @@ const (
 	SearchQueryKey             = "q"
 	SearchResolveKey           = "resolve"
 	SearchTypeKey              = "type"
+
+	/* Domain block keys */
+
+	DomainBlockExportKey = "export"
+	DomainBlockImportKey = "import"
 )
 
 // parseError returns gtserror.WithCode set to 400 Bad Request, to indicate
@@ -50,6 +56,8 @@ func parseError(key string, value, defaultValue any, err error) gtserror.WithCod
 	return gtserror.NewErrorBadRequest(err, err.Error())
 }
 
+// requiredError returns gtserror.WithCode set to 400 Bad Request, to indicate
+// to the caller a required key value was not provided, or was empty.
 func requiredError(key string) gtserror.WithCode {
 	err := fmt.Errorf("required key %s was not set or had empty value", key)
 	return gtserror.NewErrorBadRequest(err, err.Error())
@@ -60,111 +68,51 @@ func requiredError(key string) gtserror.WithCode {
 */
 
 func ParseLimit(value string, defaultValue int, max, min int) (int, gtserror.WithCode) {
-	key := LimitKey
-
-	if value == "" {
-		return defaultValue, nil
-	}
-
-	i, err := strconv.Atoi(value)
-	if err != nil {
-		return defaultValue, parseError(key, value, defaultValue, err)
-	}
-
-	if i > max {
-		i = max
-	} else if i < min {
-		i = min
-	}
-
-	return i, nil
+	return parseInt(value, defaultValue, max, min, LimitKey)
 }
 
 func ParseLocal(value string, defaultValue bool) (bool, gtserror.WithCode) {
-	key := LimitKey
-
-	if value == "" {
-		return defaultValue, nil
-	}
-
-	i, err := strconv.ParseBool(value)
-	if err != nil {
-		return defaultValue, parseError(key, value, defaultValue, err)
-	}
-
-	return i, nil
+	return parseBool(value, defaultValue, LocalKey)
 }
 
 func ParseSearchExcludeUnreviewed(value string, defaultValue bool) (bool, gtserror.WithCode) {
-	key := SearchExcludeUnreviewedKey
-
-	if value == "" {
-		return defaultValue, nil
-	}
-
-	i, err := strconv.ParseBool(value)
-	if err != nil {
-		return defaultValue, parseError(key, value, defaultValue, err)
-	}
-
-	return i, nil
+	return parseBool(value, defaultValue, SearchExcludeUnreviewedKey)
 }
 
 func ParseSearchFollowing(value string, defaultValue bool) (bool, gtserror.WithCode) {
-	key := SearchFollowingKey
-
-	if value == "" {
-		return defaultValue, nil
-	}
-
-	i, err := strconv.ParseBool(value)
-	if err != nil {
-		return defaultValue, parseError(key, value, defaultValue, err)
-	}
-
-	return i, nil
+	return parseBool(value, defaultValue, SearchFollowingKey)
 }
 
 func ParseSearchOffset(value string, defaultValue int, max, min int) (int, gtserror.WithCode) {
-	key := SearchOffsetKey
-
-	if value == "" {
-		return defaultValue, nil
-	}
-
-	i, err := strconv.Atoi(value)
-	if err != nil {
-		return defaultValue, parseError(key, value, defaultValue, err)
-	}
-
-	if i > max {
-		i = max
-	} else if i < min {
-		i = min
-	}
-
-	return i, nil
+	return parseInt(value, defaultValue, max, min, SearchOffsetKey)
 }
 
 func ParseSearchResolve(value string, defaultValue bool) (bool, gtserror.WithCode) {
-	key := SearchResolveKey
+	return parseBool(value, defaultValue, SearchResolveKey)
+}
 
-	if value == "" {
-		return defaultValue, nil
-	}
+func ParseDomainBlockExport(value string, defaultValue bool) (bool, gtserror.WithCode) {
+	return parseBool(value, defaultValue, DomainBlockExportKey)
+}
 
-	i, err := strconv.ParseBool(value)
-	if err != nil {
-		return defaultValue, parseError(key, value, defaultValue, err)
-	}
-
-	return i, nil
+func ParseDomainBlockImport(value string, defaultValue bool) (bool, gtserror.WithCode) {
+	return parseBool(value, defaultValue, DomainBlockImportKey)
 }
 
 /*
 	Parse functions for *REQUIRED* parameters.
 */
 
+func ParseID(value string) (string, gtserror.WithCode) {
+	key := IDKey
+
+	if value == "" {
+		return "", requiredError(key)
+	}
+
+	return value, nil
+}
+
 func ParseSearchLookup(value string) (string, gtserror.WithCode) {
 	key := SearchLookupKey
 
@@ -184,3 +132,39 @@ func ParseSearchQuery(value string) (string, gtserror.WithCode) {
 
 	return value, nil
 }
+
+/*
+	Internal functions
+*/
+
+func parseBool(value string, defaultValue bool, key string) (bool, gtserror.WithCode) {
+	if value == "" {
+		return defaultValue, nil
+	}
+
+	i, err := strconv.ParseBool(value)
+	if err != nil {
+		return defaultValue, parseError(key, value, defaultValue, err)
+	}
+
+	return i, nil
+}
+
+func parseInt(value string, defaultValue int, max int, min int, key string) (int, gtserror.WithCode) {
+	if value == "" {
+		return defaultValue, nil
+	}
+
+	i, err := strconv.Atoi(value)
+	if err != nil {
+		return defaultValue, parseError(key, value, defaultValue, err)
+	}
+
+	if i > max {
+		i = max
+	} else if i < min {
+		i = min
+	}
+
+	return i, nil
+}