Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions backend/app/api/handlers/v1/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package v1

import (
"encoding/json"
"errors"
"fmt"
"net/http"
"time"

Expand All @@ -15,10 +17,25 @@ import (
"github.com/sysadminsmedia/homebox/backend/internal/core/services/reporting/eventbus"
"github.com/sysadminsmedia/homebox/backend/internal/data/repo"
"github.com/sysadminsmedia/homebox/backend/internal/sys/config"
"github.com/sysadminsmedia/homebox/backend/internal/sys/validate"

"github.com/olahol/melody"
)

// multipartFormError translates a ParseMultipartForm failure into a
// RequestError. A body that tripped the MaxBytesReader cap installed by the
// body-size middleware surfaces as 413 with an explicit message instead of a
// misleading 400 "failed to parse multipart form" (#1538).
func multipartFormError(err error) error {
var maxBytesErr *http.MaxBytesError
if errors.As(err, &maxBytesErr) {
return validate.NewRequestError(
fmt.Errorf("uploaded file exceeds the size limit of %d bytes", maxBytesErr.Limit),
http.StatusRequestEntityTooLarge)
}
return validate.NewRequestError(errors.New("failed to parse multipart form"), http.StatusBadRequest)
}

type Results[T any] struct {
Items []T `json:"items"`
}
Expand Down
2 changes: 1 addition & 1 deletion backend/app/api/handlers/v1/v1_ctrl_entities.go
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ func (ctrl *V1Controller) HandleEntitiesImport() errchain.HandlerFunc {
parseSpan.End()
recordCtrlSpanError(span, err)
log.Err(err).Msg("failed to parse multipart form")
return validate.NewRequestError(err, http.StatusInternalServerError)
return multipartFormError(err)
}

file, _, err := r.FormFile("csv")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (ctrl *V1Controller) HandleEntityAttachmentCreate() errchain.HandlerFunc {
parseSpan.End()
recordCtrlSpanError(span, err)
log.Err(err).Msg("failed to parse multipart form")
return validate.NewRequestError(errors.New("failed to parse multipart form"), http.StatusBadRequest)
return multipartFormError(err)
}

errs := validate.NewFieldErrors()
Expand Down
2 changes: 1 addition & 1 deletion backend/app/api/handlers/v1/v1_ctrl_exports.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ func (ctrl *V1Controller) HandleCollectionImport() errchain.HandlerFunc {
// as the memory-vs-disk threshold so larger archives spool gracefully.
if err := r.ParseMultipartForm(ctrl.maxParseMemory << 20); err != nil {
log.Err(err).Msg("import: parse multipart")
return validate.NewRequestError(err, http.StatusBadRequest)
return multipartFormError(err)
}
// Remove any spooled temp files the multipart parser may have created.
// Registered before file.Close so the close (LIFO) runs first — on
Expand Down
23 changes: 23 additions & 0 deletions backend/app/api/static/docs/docs.go
Original file line number Diff line number Diff line change
Expand Up @@ -4775,6 +4775,19 @@ const docTemplate = `{
"entityTypeId": {
"type": "string"
},
"manufacturer": {
"type": "string",
"maxLength": 255,
"x-nullable": true,
"x-omitempty": true
},
"modelNumber": {
"description": "Identifications — optional at create time; populated e.g. by the\nbarcode product-search import flow (#1578).",
"type": "string",
"maxLength": 255,
"x-nullable": true,
"x-omitempty": true
},
"name": {
"type": "string",
"maxLength": 255,
Expand Down Expand Up @@ -4905,6 +4918,16 @@ const docTemplate = `{
"description": "Warranty",
"type": "boolean"
},
"location": {
"description": "Location is the nearest ancestor whose entity type is a location.\nWhen the direct parent is already a location it equals Parent; when\nthe entity is nested inside other items it is the location those\nitems ultimately live in. Nil for top-level entities.",
"allOf": [
{
"$ref": "#/definitions/repo.EntitySummary"
}
],
"x-nullable": true,
"x-omitempty": true
},
"manufacturer": {
"type": "string"
},
Expand Down
23 changes: 23 additions & 0 deletions backend/app/api/static/docs/openapi-3.json
Original file line number Diff line number Diff line change
Expand Up @@ -5014,6 +5014,19 @@
"entityTypeId": {
"type": "string"
},
"manufacturer": {
"type": "string",
"maxLength": 255,
"x-omitempty": true,
"nullable": true
},
"modelNumber": {
"description": "Identifications — optional at create time; populated e.g. by the\nbarcode product-search import flow (#1578).",
"type": "string",
"maxLength": 255,
"x-omitempty": true,
"nullable": true
},
"name": {
"type": "string",
"maxLength": 255,
Expand Down Expand Up @@ -5144,6 +5157,16 @@
"description": "Warranty",
"type": "boolean"
},
"location": {
"description": "Location is the nearest ancestor whose entity type is a location.\nWhen the direct parent is already a location it equals Parent; when\nthe entity is nested inside other items it is the location those\nitems ultimately live in. Nil for top-level entities.",
"allOf": [
{
"$ref": "#/components/schemas/repo.EntitySummary"
}
],
"x-omitempty": true,
"nullable": true
},
"manufacturer": {
"type": "string"
},
Expand Down
23 changes: 23 additions & 0 deletions backend/app/api/static/docs/openapi-3.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3153,6 +3153,19 @@ components:
maxLength: 1000
entityTypeId:
type: string
manufacturer:
type: string
maxLength: 255
x-omitempty: true
nullable: true
modelNumber:
description: |-
Identifications — optional at create time; populated e.g. by the
barcode product-search import flow (#1578).
type: string
maxLength: 255
x-omitempty: true
nullable: true
name:
type: string
maxLength: 255
Expand Down Expand Up @@ -3242,6 +3255,16 @@ components:
lifetimeWarranty:
description: Warranty
type: boolean
location:
description: |-
Location is the nearest ancestor whose entity type is a location.
When the direct parent is already a location it equals Parent; when
the entity is nested inside other items it is the location those
items ultimately live in. Nil for top-level entities.
allOf:
- $ref: "#/components/schemas/repo.EntitySummary"
x-omitempty: true
nullable: true
manufacturer:
type: string
modelNumber:
Expand Down
23 changes: 23 additions & 0 deletions backend/app/api/static/docs/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -4772,6 +4772,19 @@
"entityTypeId": {
"type": "string"
},
"manufacturer": {
"type": "string",
"maxLength": 255,
"x-nullable": true,
"x-omitempty": true
},
"modelNumber": {
"description": "Identifications — optional at create time; populated e.g. by the\nbarcode product-search import flow (#1578).",
"type": "string",
"maxLength": 255,
"x-nullable": true,
"x-omitempty": true
},
"name": {
"type": "string",
"maxLength": 255,
Expand Down Expand Up @@ -4902,6 +4915,16 @@
"description": "Warranty",
"type": "boolean"
},
"location": {
"description": "Location is the nearest ancestor whose entity type is a location.\nWhen the direct parent is already a location it equals Parent; when\nthe entity is nested inside other items it is the location those\nitems ultimately live in. Nil for top-level entities.",
"allOf": [
{
"$ref": "#/definitions/repo.EntitySummary"
}
],
"x-nullable": true,
"x-omitempty": true
},
"manufacturer": {
"type": "string"
},
Expand Down
23 changes: 23 additions & 0 deletions backend/app/api/static/docs/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1100,6 +1100,19 @@ definitions:
type: string
entityTypeId:
type: string
manufacturer:
maxLength: 255
type: string
x-nullable: true
x-omitempty: true
modelNumber:
description: |-
Identifications — optional at create time; populated e.g. by the
barcode product-search import flow (#1578).
maxLength: 255
type: string
x-nullable: true
x-omitempty: true
name:
maxLength: 255
minLength: 1
Expand Down Expand Up @@ -1191,6 +1204,16 @@ definitions:
lifetimeWarranty:
description: Warranty
type: boolean
location:
allOf:
- $ref: '#/definitions/repo.EntitySummary'
description: |-
Location is the nearest ancestor whose entity type is a location.
When the direct parent is already a location it equals Parent; when
the entity is nested inside other items it is the location those
items ultimately live in. Nil for top-level entities.
x-nullable: true
x-omitempty: true
manufacturer:
type: string
modelNumber:
Expand Down
46 changes: 35 additions & 11 deletions backend/internal/core/services/service_exports.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"os"
"path"
"strings"
"sync"
"time"

"github.com/google/uuid"
Expand Down Expand Up @@ -203,6 +204,14 @@ type ExportService struct {
storage config.Storage
pubSubConn string
dialect string // "sqlite3" or "postgres"

// topics caches the publisher topic per topic name so it is opened once
// and reused for the lifetime of the process. Publishers must never call
// Shutdown on these: the default mem:// driver returns a shared singleton
// per URL, so shutting it down after one send permanently breaks every
// later publish until restart (#1592).
topicsMu sync.Mutex
topics map[string]*pubsub.Topic
}

// Enqueue creates a pending Export row for gid and publishes a job to the
Expand Down Expand Up @@ -538,17 +547,37 @@ func (s *ExportService) copyAttachmentBlobs(ctx context.Context, zw *zip.Writer,
return nil
}

// publishExportJob sends a message on the export topic.
func (s *ExportService) publishExportJob(ctx context.Context, gid, exportID uuid.UUID) error {
conn, err := utils.GenerateSubPubConn(s.pubSubConn, TopicCollectionExport)
// openTopic returns the long-lived publisher topic for name, opening and
// caching it on first use. The topic is reused across publishes and stays
// open for the process lifetime — see the topics field doc for why a
// per-publish Shutdown must not be reintroduced here.
func (s *ExportService) openTopic(ctx context.Context, name string) (*pubsub.Topic, error) {
s.topicsMu.Lock()
defer s.topicsMu.Unlock()
if topic, ok := s.topics[name]; ok {
return topic, nil
}
conn, err := utils.GenerateSubPubConn(s.pubSubConn, name)
if err != nil {
return err
return nil, err
}
topic, err := pubsub.OpenTopic(ctx, conn)
if err != nil {
return nil, err
}
if s.topics == nil {
s.topics = make(map[string]*pubsub.Topic)
}
s.topics[name] = topic
return topic, nil
}
Comment thread
tankerkiller125 marked this conversation as resolved.

// publishExportJob sends a message on the export topic.
func (s *ExportService) publishExportJob(ctx context.Context, gid, exportID uuid.UUID) error {
topic, err := s.openTopic(ctx, TopicCollectionExport)
if err != nil {
return err
}
defer func() { _ = topic.Shutdown(ctx) }()
return topic.Send(ctx, &pubsub.Message{
Body: []byte("collection_export:" + exportID.String()),
Metadata: map[string]string{
Expand All @@ -563,15 +592,10 @@ func (s *ExportService) publishExportJob(ctx context.Context, gid, exportID uuid
// storage at the row's artifact_path, unzips, restores into the group
// identified by gid, then deletes the staged upload.
func (s *ExportService) publishImportJob(ctx context.Context, gid, userID, importID uuid.UUID) error {
conn, err := utils.GenerateSubPubConn(s.pubSubConn, TopicCollectionImport)
if err != nil {
return err
}
topic, err := pubsub.OpenTopic(ctx, conn)
topic, err := s.openTopic(ctx, TopicCollectionImport)
if err != nil {
return err
}
defer func() { _ = topic.Shutdown(ctx) }()
return topic.Send(ctx, &pubsub.Message{
Body: []byte("collection_import:" + gid.String()),
Metadata: map[string]string{
Expand Down
16 changes: 16 additions & 0 deletions backend/internal/core/services/service_exports_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,22 @@ func TestIsGroupReadyForImport_BlocksUserCreatedRows(t *testing.T) {
})
}

// TestPublishJobsTopicReuse guards against #1592: the mem:// pubsub driver
// returns a shared singleton topic per URL, so the old per-publish Shutdown
// permanently broke every publish after the first ("pubsub: Topic has been
// Shutdown") until the process restarted. Publishing twice on each topic
// must succeed.
func TestPublishJobsTopicReuse(t *testing.T) {
ctx := context.Background()

for i := 1; i <= 2; i++ {
require.NoError(t, tSvc.Exports.publishExportJob(ctx, uuid.New(), uuid.New()),
"export publish attempt %d", i)
require.NoError(t, tSvc.Exports.publishImportJob(ctx, uuid.New(), uuid.New(), uuid.New()),
"import publish attempt %d", i)
}
}

// copyBlobUnderTest reuses the export service's bucket plumbing to copy a
// blob from one key to another in the same backing store. Used to "stage"
// the just-produced export under the destination group's import prefix.
Expand Down
Loading
Loading