Skip to content

Commit b79a3e0

Browse files
AchoArnoldCopilot
AchoArnold
and
Copilot
committed
refactor: address PR review feedback - rename AttachmentStorage to AttachmentRepository
- Rename interface AttachmentStorage -> AttachmentRepository - Rename GCSAttachmentStorage -> GoogleCloudStorageAttachmentRepository - Rename MemoryAttachmentStorage -> MemoryAttachmentRepository - Rename all files: attachment_storage.go -> attachment_repository.go, etc. - Replace ErrAttachmentNotFound with existing ErrCodeNotFound pattern - Map GCS storage.ErrObjectNotExist to ErrCodeNotFound in Download - Use StartWithLogger + ctxLogger in all repository methods - Use StartWithLogger in uploadAttachments service method - Add contentType parameter to Upload interface, set on GCS writer - Mark Attachments field as optional in swagger docs - Fix Swagger @router to include /v1 prefix - Run go mod tidy to fix indirect marker Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 3201494 commit b79a3e0

11 files changed

Lines changed: 121 additions & 121 deletions

‎api/go.mod‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ go 1.25.0
44

55
require (
66
cloud.google.com/go/cloudtasks v1.14.0
7+
cloud.google.com/go/storage v1.62.0
78
firebase.google.com/go v3.13.0+incompatible
89
github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric v0.55.0
910
github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/trace v1.31.0
@@ -50,6 +51,7 @@ require (
5051
go.opentelemetry.io/otel/sdk v1.43.0
5152
go.opentelemetry.io/otel/sdk/metric v1.43.0
5253
go.opentelemetry.io/otel/trace v1.43.0
54+
golang.org/x/sync v0.20.0
5355
google.golang.org/api v0.274.0
5456
google.golang.org/protobuf v1.36.11
5557
gorm.io/driver/postgres v1.6.0
@@ -80,7 +82,6 @@ require (
8082
cloud.google.com/go/iam v1.7.0 // indirect
8183
cloud.google.com/go/longrunning v0.9.0 // indirect
8284
cloud.google.com/go/monitoring v1.25.0 // indirect
83-
cloud.google.com/go/storage v1.62.0 // indirect
8485
cloud.google.com/go/trace v1.12.0 // indirect
8586
dario.cat/mergo v1.0.2 // indirect
8687
filippo.io/edwards25519 v1.2.0 // indirect
@@ -190,7 +191,6 @@ require (
190191
golang.org/x/mod v0.34.0 // indirect
191192
golang.org/x/net v0.52.0 // indirect
192193
golang.org/x/oauth2 v0.36.0 // indirect
193-
golang.org/x/sync v0.20.0 // indirect
194194
golang.org/x/sys v0.42.0 // indirect
195195
golang.org/x/text v0.35.0 // indirect
196196
golang.org/x/time v0.15.0 // indirect

‎api/go.sum‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@ cloud.google.com/go/longrunning v0.9.0 h1:0EzbDEGsAvOZNbqXopgniY0w0a1phvu5IdUFq8
2222
cloud.google.com/go/longrunning v0.9.0/go.mod h1:pkTz846W7bF4o2SzdWJ40Hu0Re+UoNT6Q5t+igIcb8E=
2323
cloud.google.com/go/monitoring v1.25.0 h1:HnsTIOxTN6BCSkt1P/Im23r1m7MHTTpmSYCzPkW7NK4=
2424
cloud.google.com/go/monitoring v1.25.0/go.mod h1:wlj6rX+JGyusw/8+2duW4cJ6kmDHGmde3zMTJuG3Jpc=
25-
cloud.google.com/go/storage v1.61.3 h1:VS//ZfBuPGDvakfD9xyPW1RGF1Vy3BWUoVZXgW1KMOg=
26-
cloud.google.com/go/storage v1.61.3/go.mod h1:JtqK8BBB7TWv0HVGHubtUdzYYrakOQIsMLffZ2Z/HWk=
2725
cloud.google.com/go/storage v1.62.0 h1:w2pQJhpUqVerMON45vatE2FpCYsNTf7OHjkn6ux5mMU=
2826
cloud.google.com/go/storage v1.62.0/go.mod h1:T5hz3qzcpnxZ5LdKc7y8Tw7lh4v9zeeVyrD/cLJAzZU=
2927
cloud.google.com/go/trace v1.12.0 h1:XvWHYfr9q88cX4pZyou6qCcSagnuASyUq2ej1dB6NzQ=
@@ -377,9 +375,8 @@ go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.43.0 h1:88Y4s2C8oTui1LGM6bT
377375
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.43.0/go.mod h1:Vl1/iaggsuRlrHf/hfPJPvVag77kKyvrLeD10kpMl+A=
378376
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.43.0 h1:3iZJKlCZufyRzPzlQhUIWVmfltrXuGyfjREgGP3UUjc=
379377
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.43.0/go.mod h1:/G+nUPfhq2e+qiXMGxMwumDrP5jtzU+mWN7/sjT2rak=
380-
go.opentelemetry.io/otel/exporters/stdout/stdoutmetric v1.40.0 h1:ZrPRak/kS4xI3AVXy8F7pipuDXmDsrO8Lg+yQjBLjw0=
381-
go.opentelemetry.io/otel/exporters/stdout/stdoutmetric v1.40.0/go.mod h1:3y6kQCWztq6hyW8Z9YxQDDm0Je9AJoFar2G0yDcmhRk=
382378
go.opentelemetry.io/otel/exporters/stdout/stdoutmetric v1.42.0 h1:lSZHgNHfbmQTPfuTmWVkEu8J8qXaQwuV30pjCcAUvP8=
379+
go.opentelemetry.io/otel/exporters/stdout/stdoutmetric v1.42.0/go.mod h1:so9ounLcuoRDu033MW/E0AD4hhUjVqswrMF5FoZlBcw=
383380
go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.43.0 h1:mS47AX77OtFfKG4vtp+84kuGSFZHTyxtXIN269vChY0=
384381
go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.43.0/go.mod h1:PJnsC41lAGncJlPUniSwM81gc80GkgWJWr3cu2nKEtU=
385382
go.opentelemetry.io/otel/log v0.19.0 h1:KUZs/GOsw79TBBMfDWsXS+KZ4g2Ckzksd1ymzsIEbo4=

‎api/pkg/di/container.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ type Container struct {
8888
app *fiber.App
8989
eventDispatcher *services.EventDispatcher
9090
logger telemetry.Logger
91-
attachmentStorage repositories.AttachmentStorage
91+
attachmentRepository repositories.AttachmentRepository
9292
}
9393

9494
// NewLiteContainer creates a Container without any routes or listeners
@@ -1433,39 +1433,39 @@ func (container *Container) MessageService() (service *services.MessageService)
14331433
container.MessageRepository(),
14341434
container.EventDispatcher(),
14351435
container.PhoneService(),
1436-
container.AttachmentStorage(),
1436+
container.AttachmentRepository(),
14371437
container.APIBaseURL(),
14381438
)
14391439
}
14401440

1441-
// AttachmentStorage creates a cached AttachmentStorage based on configuration
1442-
func (container *Container) AttachmentStorage() repositories.AttachmentStorage {
1443-
if container.attachmentStorage != nil {
1444-
return container.attachmentStorage
1441+
// AttachmentRepository creates a cached AttachmentRepository based on configuration
1442+
func (container *Container) AttachmentRepository() repositories.AttachmentRepository {
1443+
if container.attachmentRepository != nil {
1444+
return container.attachmentRepository
14451445
}
14461446

14471447
bucket := os.Getenv("GCS_BUCKET_NAME")
14481448
if bucket != "" {
1449-
container.logger.Debug("creating GCSAttachmentStorage")
1449+
container.logger.Debug("creating GoogleCloudStorageAttachmentRepository")
14501450
client, err := storage.NewClient(context.Background())
14511451
if err != nil {
14521452
container.logger.Fatal(stacktrace.Propagate(err, "cannot create GCS client"))
14531453
}
1454-
container.attachmentStorage = repositories.NewGCSAttachmentStorage(
1454+
container.attachmentRepository = repositories.NewGoogleCloudStorageAttachmentRepository(
14551455
container.Logger(),
14561456
container.Tracer(),
14571457
client,
14581458
bucket,
14591459
)
14601460
} else {
1461-
container.logger.Debug("creating MemoryAttachmentStorage (GCS_BUCKET_NAME not set)")
1462-
container.attachmentStorage = repositories.NewMemoryAttachmentStorage(
1461+
container.logger.Debug("creating MemoryAttachmentRepository (GCS_BUCKET_NAME not set)")
1462+
container.attachmentRepository = repositories.NewMemoryAttachmentRepository(
14631463
container.Logger(),
14641464
container.Tracer(),
14651465
)
14661466
}
14671467

1468-
return container.attachmentStorage
1468+
return container.attachmentRepository
14691469
}
14701470

14711471
// APIBaseURL returns the API base URL derived from EVENTS_QUEUE_ENDPOINT
@@ -1480,7 +1480,7 @@ func (container *Container) AttachmentHandler() (handler *handlers.AttachmentHan
14801480
return handlers.NewAttachmentHandler(
14811481
container.Logger(),
14821482
container.Tracer(),
1483-
container.AttachmentStorage(),
1483+
container.AttachmentRepository(),
14841484
)
14851485
}
14861486

‎api/pkg/handlers/attachment_handler.go‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package handlers
22

33
import (
4-
"errors"
54
"fmt"
65
"path/filepath"
76

@@ -16,14 +15,14 @@ type AttachmentHandler struct {
1615
handler
1716
logger telemetry.Logger
1817
tracer telemetry.Tracer
19-
storage repositories.AttachmentStorage
18+
storage repositories.AttachmentRepository
2019
}
2120

2221
// NewAttachmentHandler creates a new AttachmentHandler
2322
func NewAttachmentHandler(
2423
logger telemetry.Logger,
2524
tracer telemetry.Tracer,
26-
storage repositories.AttachmentStorage,
25+
storage repositories.AttachmentRepository,
2726
) (h *AttachmentHandler) {
2827
return &AttachmentHandler{
2928
logger: logger.WithService(fmt.Sprintf("%T", h)),
@@ -49,7 +48,7 @@ func (h *AttachmentHandler) RegisterRoutes(router fiber.Router) {
4948
// @Success 200 {file} binary
5049
// @Failure 404 {object} responses.NotFoundResponse
5150
// @Failure 500 {object} responses.InternalServerError
52-
// @Router /attachments/{userID}/{messageID}/{attachmentIndex}/{filename} [get]
51+
// @Router /v1/attachments/{userID}/{messageID}/{attachmentIndex}/{filename} [get]
5352
func (h *AttachmentHandler) GetAttachment(c *fiber.Ctx) error {
5453
ctx, span := h.tracer.StartFromFiberCtx(c)
5554
defer span.End()
@@ -69,7 +68,7 @@ func (h *AttachmentHandler) GetAttachment(c *fiber.Ctx) error {
6968
if err != nil {
7069
msg := fmt.Sprintf("cannot download attachment from path [%s]", path)
7170
ctxLogger.Warn(stacktrace.Propagate(err, msg))
72-
if errors.Is(err, repositories.ErrAttachmentNotFound) {
71+
if stacktrace.GetCode(err) == repositories.ErrCodeNotFound {
7372
return h.responseNotFound(c, "attachment not found")
7473
}
7574
return h.responseInternalServerError(c)

api/pkg/repositories/attachment_storage.go renamed to api/pkg/repositories/attachment_repository.go

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ import (
77
"strings"
88
)
99

10-
// AttachmentStorage is the interface for storing and retrieving message attachments
11-
type AttachmentStorage interface {
12-
// Upload stores attachment data at the given path
13-
Upload(ctx context.Context, path string, data []byte) error
10+
// AttachmentRepository is the interface for storing and retrieving message attachments
11+
type AttachmentRepository interface {
12+
// Upload stores attachment data at the given path with the specified content type
13+
Upload(ctx context.Context, path string, data []byte, contentType string) error
1414
// Download retrieves attachment data from the given path
1515
Download(ctx context.Context, path string) ([]byte, error)
1616
// Delete removes an attachment at the given path
@@ -77,9 +77,6 @@ func ContentTypeFromExtension(ext string) string {
7777
return "application/octet-stream"
7878
}
7979

80-
// ErrAttachmentNotFound is returned when an attachment is not found in storage
81-
var ErrAttachmentNotFound = fmt.Errorf("attachment not found")
82-
8380
// SanitizeFilename removes path separators and traversal sequences from a filename.
8481
// Returns "attachment-{index}" if the sanitized name is empty.
8582
func SanitizeFilename(name string, index int) string {

api/pkg/repositories/attachment_storage_test.go renamed to api/pkg/repositories/attachment_repository_test.go

File renamed without changes.

api/pkg/repositories/gcs_attachment_storage.go renamed to api/pkg/repositories/gcs_attachment_repository.go

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package repositories
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"io"
78

@@ -10,35 +11,37 @@ import (
1011
"github.com/palantir/stacktrace"
1112
)
1213

13-
// GCSAttachmentStorage stores attachments in Google Cloud Storage
14-
type GCSAttachmentStorage struct {
14+
// GoogleCloudStorageAttachmentRepository stores attachments in Google Cloud Storage
15+
type GoogleCloudStorageAttachmentRepository struct {
1516
logger telemetry.Logger
1617
tracer telemetry.Tracer
1718
client *storage.Client
1819
bucket string
1920
}
2021

21-
// NewGCSAttachmentStorage creates a new GCSAttachmentStorage
22-
func NewGCSAttachmentStorage(
22+
// NewGoogleCloudStorageAttachmentRepository creates a new GoogleCloudStorageAttachmentRepository
23+
func NewGoogleCloudStorageAttachmentRepository(
2324
logger telemetry.Logger,
2425
tracer telemetry.Tracer,
2526
client *storage.Client,
2627
bucket string,
27-
) *GCSAttachmentStorage {
28-
return &GCSAttachmentStorage{
29-
logger: logger.WithService(fmt.Sprintf("%T", &GCSAttachmentStorage{})),
28+
) *GoogleCloudStorageAttachmentRepository {
29+
return &GoogleCloudStorageAttachmentRepository{
30+
logger: logger.WithService(fmt.Sprintf("%T", &GoogleCloudStorageAttachmentRepository{})),
3031
tracer: tracer,
3132
client: client,
3233
bucket: bucket,
3334
}
3435
}
3536

3637
// Upload stores attachment data at the given path in GCS
37-
func (s *GCSAttachmentStorage) Upload(ctx context.Context, path string, data []byte) error {
38-
ctx, span := s.tracer.Start(ctx)
38+
func (s *GoogleCloudStorageAttachmentRepository) Upload(ctx context.Context, path string, data []byte, contentType string) error {
39+
ctx, span, ctxLogger := s.tracer.StartWithLogger(ctx, s.logger)
3940
defer span.End()
4041

4142
writer := s.client.Bucket(s.bucket).Object(path).NewWriter(ctx)
43+
writer.ContentType = contentType
44+
4245
if _, err := writer.Write(data); err != nil {
4346
return s.tracer.WrapErrorSpan(span, stacktrace.Propagate(err, fmt.Sprintf("cannot write attachment to GCS path [%s]", path)))
4447
}
@@ -47,18 +50,22 @@ func (s *GCSAttachmentStorage) Upload(ctx context.Context, path string, data []b
4750
return s.tracer.WrapErrorSpan(span, stacktrace.Propagate(err, fmt.Sprintf("cannot close GCS writer for path [%s]", path)))
4851
}
4952

50-
s.logger.Info(fmt.Sprintf("uploaded attachment to GCS path [%s/%s] with size [%d]", s.bucket, path, len(data)))
53+
ctxLogger.Info(fmt.Sprintf("uploaded attachment to GCS path [%s/%s] with size [%d]", s.bucket, path, len(data)))
5154
return nil
5255
}
5356

5457
// Download retrieves attachment data from the given path in GCS
55-
func (s *GCSAttachmentStorage) Download(ctx context.Context, path string) ([]byte, error) {
56-
ctx, span := s.tracer.Start(ctx)
58+
func (s *GoogleCloudStorageAttachmentRepository) Download(ctx context.Context, path string) ([]byte, error) {
59+
ctx, span, ctxLogger := s.tracer.StartWithLogger(ctx, s.logger)
5760
defer span.End()
5861

5962
reader, err := s.client.Bucket(s.bucket).Object(path).NewReader(ctx)
6063
if err != nil {
61-
return nil, s.tracer.WrapErrorSpan(span, stacktrace.Propagate(err, fmt.Sprintf("cannot open GCS reader for path [%s]", path)))
64+
msg := fmt.Sprintf("cannot open GCS reader for path [%s]", path)
65+
if errors.Is(err, storage.ErrObjectNotExist) {
66+
return nil, s.tracer.WrapErrorSpan(span, stacktrace.PropagateWithCode(err, ErrCodeNotFound, msg))
67+
}
68+
return nil, s.tracer.WrapErrorSpan(span, stacktrace.Propagate(err, msg))
6269
}
6370
defer reader.Close()
6471

@@ -67,18 +74,19 @@ func (s *GCSAttachmentStorage) Download(ctx context.Context, path string) ([]byt
6774
return nil, s.tracer.WrapErrorSpan(span, stacktrace.Propagate(err, fmt.Sprintf("cannot read attachment from GCS path [%s]", path)))
6875
}
6976

77+
ctxLogger.Info(fmt.Sprintf("downloaded attachment from GCS path [%s/%s] with size [%d]", s.bucket, path, len(data)))
7078
return data, nil
7179
}
7280

7381
// Delete removes an attachment at the given path in GCS
74-
func (s *GCSAttachmentStorage) Delete(ctx context.Context, path string) error {
75-
ctx, span := s.tracer.Start(ctx)
82+
func (s *GoogleCloudStorageAttachmentRepository) Delete(ctx context.Context, path string) error {
83+
ctx, span, ctxLogger := s.tracer.StartWithLogger(ctx, s.logger)
7684
defer span.End()
7785

7886
if err := s.client.Bucket(s.bucket).Object(path).Delete(ctx); err != nil {
7987
return s.tracer.WrapErrorSpan(span, stacktrace.Propagate(err, fmt.Sprintf("cannot delete GCS object at path [%s]", path)))
8088
}
8189

82-
s.logger.Info(fmt.Sprintf("deleted attachment from GCS path [%s/%s]", s.bucket, path))
90+
ctxLogger.Info(fmt.Sprintf("deleted attachment from GCS path [%s/%s]", s.bucket, path))
8391
return nil
8492
}

‎api/pkg/repositories/memory_attachment_repository.go‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
package repositories
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"sync"
7+
8+
"github.com/NdoleStudio/httpsms/pkg/telemetry"
9+
"github.com/palantir/stacktrace"
10+
)
11+
12+
// MemoryAttachmentRepository stores attachments in memory
13+
type MemoryAttachmentRepository struct {
14+
logger telemetry.Logger
15+
tracer telemetry.Tracer
16+
data sync.Map
17+
}
18+
19+
// NewMemoryAttachmentRepository creates a new MemoryAttachmentRepository
20+
func NewMemoryAttachmentRepository(
21+
logger telemetry.Logger,
22+
tracer telemetry.Tracer,
23+
) *MemoryAttachmentRepository {
24+
return &MemoryAttachmentRepository{
25+
logger: logger.WithService(fmt.Sprintf("%T", &MemoryAttachmentRepository{})),
26+
tracer: tracer,
27+
}
28+
}
29+
30+
// Upload stores attachment data at the given path
31+
func (s *MemoryAttachmentRepository) Upload(ctx context.Context, path string, data []byte, _ string) error {
32+
_, span, ctxLogger := s.tracer.StartWithLogger(ctx, s.logger)
33+
defer span.End()
34+
35+
s.data.Store(path, data)
36+
ctxLogger.Info(fmt.Sprintf("stored attachment at path [%s] with size [%d]", path, len(data)))
37+
return nil
38+
}
39+
40+
// Download retrieves attachment data from the given path
41+
func (s *MemoryAttachmentRepository) Download(ctx context.Context, path string) ([]byte, error) {
42+
_, span, _ := s.tracer.StartWithLogger(ctx, s.logger)
43+
defer span.End()
44+
45+
value, ok := s.data.Load(path)
46+
if !ok {
47+
return nil, s.tracer.WrapErrorSpan(span, stacktrace.NewErrorWithCode(ErrCodeNotFound, fmt.Sprintf("attachment not found at path [%s]", path)))
48+
}
49+
return value.([]byte), nil
50+
}
51+
52+
// Delete removes an attachment at the given path
53+
func (s *MemoryAttachmentRepository) Delete(ctx context.Context, path string) error {
54+
_, span, ctxLogger := s.tracer.StartWithLogger(ctx, s.logger)
55+
defer span.End()
56+
57+
s.data.Delete(path)
58+
ctxLogger.Info(fmt.Sprintf("deleted attachment at path [%s]", path))
59+
return nil
60+
}

0 commit comments

Comments
 (0)