Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(asset): add asset management functionality and validation #60

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
.env
.env.*
.idea
6 changes: 4 additions & 2 deletions account/accountdomain/workspace/initializer.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@ type InitParams struct {

func Init(p InitParams) (*user.User, *Workspace, error) {
if p.UserID == nil {
p.UserID = user.NewID().Ref()
newID := user.NewID()
p.UserID = newID.Ref()
}
if p.WorkspaceID == nil {
p.WorkspaceID = NewID().Ref()
newWorkspaceID := NewID()
p.WorkspaceID = newWorkspaceID.Ref()
}
if p.Lang == nil {
p.Lang = &language.Tag{}
Expand Down
6 changes: 3 additions & 3 deletions account/accountdomain/workspace/member.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func (m *Members) Users() map[UserID]Member {
func (m *Members) UserIDs() []UserID {
users := lo.Keys(m.users)
sort.SliceStable(users, func(a, b int) bool {
return users[a].Compare(users[b]) > 0
return users[a].Compare(&users[b]) > 0
})
return users
}
Expand All @@ -94,7 +94,7 @@ func (m *Members) Integrations() map[IntegrationID]Member {
func (m *Members) IntegrationIDs() []IntegrationID {
integrations := lo.Keys(m.integrations)
sort.SliceStable(integrations, func(a, b int) bool {
return integrations[a].Compare(integrations[b]) > 0
return integrations[a].Compare(&integrations[b]) > 0
})
return integrations
}
Expand Down Expand Up @@ -255,7 +255,7 @@ func (m *Members) UsersByRole(role Role) []UserID {
}

sort.SliceStable(users, func(a, b int) bool {
return users[a].Compare(users[b]) > 0
return users[a].Compare(&users[b]) > 0
})

return users
Expand Down
7 changes: 6 additions & 1 deletion account/accountinfrastructure/accountmemory/workspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,12 @@ func (r *Workspace) FindByIDs(_ context.Context, ids workspace.IDList) (workspac
res := r.data.FindAll(func(key workspace.ID, value *workspace.Workspace) bool {
return ids.Has(key)
})
slices.SortFunc(res, func(a, b *workspace.Workspace) int { return a.ID().Compare(b.ID()) })
slices.SortFunc(res, func(a, b *workspace.Workspace) int {
idA := a.ID()
idB := b.ID()
return idA.Compare(&idB)
})

return res, nil
}

Expand Down
36 changes: 36 additions & 0 deletions asset/decompress/zip.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package decompress

import (
"archive/zip"
"context"
"github.com/reearth/reearthx/asset"
)

type ZipDecompressor struct {
assetService *asset.Service
}

func NewZipDecompressor(assetService *asset.Service) *ZipDecompressor {
return &ZipDecompressor{
assetService: assetService,
}
}

func (d *ZipDecompressor) DecompressAsync(ctx context.Context, assetID asset.ID) error {
// Get the zip file from asset service
zipFile, err := d.assetService.GetFile(ctx, assetID)
if err != nil {
return err
}
defer zipFile.Close()

// Create a temporary file to store the zip content
// Implementation of async zip extraction
return nil
}
Comment on lines +19 to +30
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Implementation incomplete and missing error handling.

The DecompressAsync method:

  1. Returns nil without actually processing the ZIP file
  2. Missing ZIP reader initialization
  3. Should handle context cancellation

Consider implementing the missing functionality:

func (d *ZipDecompressor) DecompressAsync(ctx context.Context, assetID asset.ID) error {
    zipFile, err := d.assetService.GetFile(ctx, assetID)
    if err != nil {
        return fmt.Errorf("failed to get zip file: %w", err)
    }
    defer zipFile.Close()

+   // Create zip reader
+   zipReader, err := zip.NewReader(zipFile, -1)
+   if err != nil {
+       return fmt.Errorf("failed to create zip reader: %w", err)
+   }
+
+   // Process asynchronously
+   go func() {
+       if err := d.processZipFile(ctx, zipReader); err != nil {
+           // Consider implementing error callback or logging
+           log.Printf("Error processing zip file: %v", err)
+       }
+   }()

    return nil
}

Committable suggestion skipped: line range outside the PR's diff.


func (d *ZipDecompressor) processZipFile(ctx context.Context, zipReader *zip.Reader) error {
// Process each file in the zip
// Create new assets for each file
return nil
}
Comment on lines +32 to +36
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Implement the processZipFile method or remove if unused.

The processZipFile method is currently a stub and marked as unused by static analysis.

Either implement the method with proper ZIP file processing logic or remove it if not needed. Consider:

func (d *ZipDecompressor) processZipFile(ctx context.Context, zipReader *zip.Reader) error {
    for _, file := range zipReader.File {
        select {
        case <-ctx.Done():
            return ctx.Err()
        default:
            if err := d.processZipEntry(ctx, file); err != nil {
                return fmt.Errorf("failed to process %s: %w", file.Name, err)
            }
        }
    }
    return nil
}
🧰 Tools
🪛 golangci-lint (1.62.2)

32-32: func (*ZipDecompressor).processZipFile is unused

(unused)

35 changes: 35 additions & 0 deletions asset/pubsub/pubsub.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package pubsub

import (
"context"
"github.com/reearth/reearthx/asset"
)

type AssetEvent struct {
Type string `json:"type"`
AssetID asset.ID `json:"asset_id"`
}

type Publisher interface {
Publish(ctx context.Context, topic string, msg interface{}) error
}

type AssetPubSub struct {
publisher Publisher
topic string
}

func NewAssetPubSub(publisher Publisher, topic string) *AssetPubSub {
return &AssetPubSub{
publisher: publisher,
topic: topic,
}
}

func (p *AssetPubSub) PublishAssetEvent(ctx context.Context, eventType string, assetID asset.ID) error {
event := AssetEvent{
Type: eventType,
AssetID: assetID,
}
return p.publisher.Publish(ctx, p.topic, event)
}
Comment on lines +29 to +35
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add validation for eventType parameter.

The PublishAssetEvent method accepts any string as eventType without validation. Consider adding validation to ensure only valid event types are published.

func (p *AssetPubSub) PublishAssetEvent(ctx context.Context, eventType string, assetID asset.ID) error {
+   if eventType == "" {
+       return errors.New("event type cannot be empty")
+   }
    event := AssetEvent{
        Type:    eventType,
        AssetID: assetID,
    }
    return p.publisher.Publish(ctx, p.topic, event)
}

15 changes: 15 additions & 0 deletions asset/repository.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package asset

import (
"context"
"io"
)

type Repository interface {
Fetch(ctx context.Context, id ID) (*Asset, error)
FetchFile(ctx context.Context, id ID) (io.ReadCloser, error)
Save(ctx context.Context, asset *Asset) error
Remove(ctx context.Context, id ID) error
Upload(ctx context.Context, id ID, file io.Reader) error
GetUploadURL(ctx context.Context, id ID) (string, error)
}
76 changes: 76 additions & 0 deletions asset/service.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package asset

import (
"context"
"io"
"time"
)

type Service struct {
repo Repository
}

func NewService(repo Repository) *Service {
return &Service{repo: repo}
}

func (s *Service) Create(ctx context.Context, input CreateAssetInput) (*Asset, error) {
asset := &Asset{
ID: ID(generateID()),
Name: input.Name,
Size: input.Size,
ContentType: input.ContentType,
CreatedAt: time.Now(),
UpdatedAt: time.Now(),
}

if err := s.repo.Save(ctx, asset); err != nil {
return nil, err
}

return asset, nil
}
Comment on lines +17 to +32
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance error handling and validation in Create method.

The Create method needs additional validation and error wrapping:

  1. Input validation is missing
  2. ID generation error handling is not implemented
  3. Error messages could be more descriptive
func (s *Service) Create(ctx context.Context, input CreateAssetInput) (*Asset, error) {
+   if err := input.Validate(); err != nil {
+       return nil, fmt.Errorf("invalid input: %w", err)
+   }
+
+   id, err := generateID()
+   if err != nil {
+       return nil, fmt.Errorf("failed to generate ID: %w", err)
+   }

    asset := &Asset{
-       ID:          ID(generateID()),
+       ID:          ID(id),
        Name:        input.Name,
        Size:        input.Size,
        ContentType: input.ContentType,
        CreatedAt:   time.Now(),
        UpdatedAt:   time.Now(),
    }

    if err := s.repo.Save(ctx, asset); err != nil {
-       return nil, err
+       return nil, fmt.Errorf("failed to save asset: %w", err)
    }

    return asset, nil
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (s *Service) Create(ctx context.Context, input CreateAssetInput) (*Asset, error) {
asset := &Asset{
ID: ID(generateID()),
Name: input.Name,
Size: input.Size,
ContentType: input.ContentType,
CreatedAt: time.Now(),
UpdatedAt: time.Now(),
}
if err := s.repo.Save(ctx, asset); err != nil {
return nil, err
}
return asset, nil
}
func (s *Service) Create(ctx context.Context, input CreateAssetInput) (*Asset, error) {
if err := input.Validate(); err != nil {
return nil, fmt.Errorf("invalid input: %w", err)
}
id, err := generateID()
if err != nil {
return nil, fmt.Errorf("failed to generate ID: %w", err)
}
asset := &Asset{
ID: ID(id),
Name: input.Name,
Size: input.Size,
ContentType: input.ContentType,
CreatedAt: time.Now(),
UpdatedAt: time.Now(),
}
if err := s.repo.Save(ctx, asset); err != nil {
return nil, fmt.Errorf("failed to save asset: %w", err)
}
return asset, nil
}


func (s *Service) Update(ctx context.Context, id ID, input UpdateAssetInput) (*Asset, error) {
asset, err := s.repo.Fetch(ctx, id)
if err != nil {
return nil, err
}

if input.Name != nil {
asset.Name = *input.Name
}
if input.URL != nil {
asset.URL = *input.URL
}
if input.ContentType != nil {
asset.ContentType = *input.ContentType
}
asset.UpdatedAt = time.Now()

if err := s.repo.Save(ctx, asset); err != nil {
return nil, err
}

return asset, nil
}

func (s *Service) Delete(ctx context.Context, id ID) error {
return s.repo.Remove(ctx, id)
}

func (s *Service) Get(ctx context.Context, id ID) (*Asset, error) {
return s.repo.Fetch(ctx, id)
}

func (s *Service) GetFile(ctx context.Context, id ID) (io.ReadCloser, error) {
return s.repo.FetchFile(ctx, id)
}

func (s *Service) Upload(ctx context.Context, id ID, file io.Reader) error {
return s.repo.Upload(ctx, id, file)
}
Comment on lines +70 to +72
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add file validation in Upload method.

The Upload method should validate the file before uploading:

  1. Check file size limits
  2. Validate content type if applicable
  3. Consider adding progress tracking for large files
func (s *Service) Upload(ctx context.Context, id ID, file io.Reader) error {
+   // Wrap reader with size limitation
+   limitedReader := io.LimitReader(file, MaxFileSize)
+
+   // Optionally wrap with progress tracking
+   progressReader := &ProgressReader{Reader: limitedReader}
+
-   return s.repo.Upload(ctx, id, file)
+   return s.repo.Upload(ctx, id, progressReader)
}

Committable suggestion skipped: line range outside the PR's diff.


func (s *Service) GetUploadURL(ctx context.Context, id ID) (string, error) {
return s.repo.GetUploadURL(ctx, id)
}
29 changes: 29 additions & 0 deletions asset/types.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package asset

import (
"time"
)

type ID string

type Asset struct {
ID ID
Name string
Size int64
URL string
ContentType string
CreatedAt time.Time
UpdatedAt time.Time
}

type CreateAssetInput struct {
Name string
Size int64
ContentType string
}

type UpdateAssetInput struct {
Name *string
URL *string
ContentType *string
}
9 changes: 9 additions & 0 deletions asset/utils.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package asset

import (
"github.com/google/uuid"
)

func generateID() string {
return uuid.New().String()
}
Loading
Loading