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.

Sign up for GitHub

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(server): asset url management #1303

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat(server): asset url management #1303

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c923939
feat(server): asset url management
yk-eukarya Nov 8, 2024
1b489c0
Merge branch 'main' into feat/asst-url
yk-eukarya Nov 22, 2024
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
5 changes: 5 additions & 0 deletions go.work.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,11 @@ cloud.google.com/go/auth v0.7.2/go.mod h1:VEc4p5NNxycWQTMQEDQF0bd6aTMb6VgYDXEwiJ
cloud.google.com/go/auth v0.8.0/go.mod h1:qGVp/Y3kDRSDZ5gFD/XPUfYQ9xW1iI7q8RIRoCyBbJc=
cloud.google.com/go/auth v0.9.0/go.mod h1:2HsApZBr9zGZhC9QAXsYVYaWk8kNUt37uny+XVKi7wM=
cloud.google.com/go/auth v0.9.3/go.mod h1:7z6VY+7h3KUdRov5F1i8NDP5ZzWKYmEPO842BgCsmTk=
cloud.google.com/go/auth v0.9.4/go.mod h1:SHia8n6//Ya940F1rLimhJCjjx7KE17t0ctFEci3HkA=
cloud.google.com/go/auth v0.9.9/go.mod h1:xxA5AqpDrvS+Gkmo9RqrGGRh6WSNKKOXhY3zNOr38tI=
cloud.google.com/go/auth/oauth2adapt v0.2.2/go.mod h1:wcYjgpZI9+Yu7LyYBg4pqSiaRkfEK3GQcpb7C/uyF1Q=
cloud.google.com/go/auth/oauth2adapt v0.2.3/go.mod h1:tMQXOfZzFuNuUxOypHlQEXgdfX5cuhwU+ffUuXRJE8I=
cloud.google.com/go/auth/oauth2adapt v0.2.4/go.mod h1:jC/jOpwFP6JBxhB3P5Rr0a9HLMC/Pe3eaL4NmdvqPtc=
cloud.google.com/go/automl v1.5.0 h1:Av8bE5rMQxUl4SVfYsEczEJf1ftIKQOUGGzpzEYwodY=
cloud.google.com/go/automl v1.5.0/go.mod h1:34EjfoFGMZ5sgJ9EoLsRtdPSNZLcfflJR39VbVNS2M0=
cloud.google.com/go/automl v1.6.0 h1:U+kHmeKGXgBvTlrecPJhwkItWaIpIscG5DUpQxBQZZg=
Expand Down Expand Up @@ -3378,6 +3381,8 @@ google.golang.org/genproto/googleapis/bytestream v0.0.0-20240903143218-8af14fe29
google.golang.org/genproto/googleapis/bytestream v0.0.0-20240903143218-8af14fe29dc1/go.mod h1:q0eWNnCW04EJlyrmLT+ZHsjuoUiZ36/eAEdCCezZoco=
google.golang.org/genproto/googleapis/bytestream v0.0.0-20241015192408-796eee8c2d53 h1:mVZqGNBNN8C63iGnWgHZSGbT/vG7voylnp4atysmReg=
google.golang.org/genproto/googleapis/bytestream v0.0.0-20241015192408-796eee8c2d53/go.mod h1:T8O3fECQbif8cez15vxAcjbwXxvL2xbnvbQ7ZfiMAMs=
google.golang.org/genproto/googleapis/bytestream v0.0.0-20241021214115-324edc3d5d38 h1:42FIsHvG4GOaVHLDMcy/YMqC4clCbgAPojbcA2hXp5w=
google.golang.org/genproto/googleapis/bytestream v0.0.0-20241021214115-324edc3d5d38/go.mod h1:T8O3fECQbif8cez15vxAcjbwXxvL2xbnvbQ7ZfiMAMs=
google.golang.org/genproto/googleapis/rpc v0.0.0-20230525234030-28d5490b6b19/go.mod h1:66JfowdXAEgad5O9NnYcsNPLCPZJD++2L9X0PCMODrA=
google.golang.org/genproto/googleapis/rpc v0.0.0-20230526203410-71b5a4ffd15e/go.mod h1:66JfowdXAEgad5O9NnYcsNPLCPZJD++2L9X0PCMODrA=
google.golang.org/genproto/googleapis/rpc v0.0.0-20230530153820-e85fd2cbaebc/go.mod h1:66JfowdXAEgad5O9NnYcsNPLCPZJD++2L9X0PCMODrA=
Expand Down
11 changes: 10 additions & 1 deletion server/.env.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ REEARTH_CMS_DB=mongodb://localhost
REEARTH_CMS_HOST=https://localhost:8080
REEARTH_CMS_SERVERHOST=https://localhost:8080
REEARTH_CMS_HOST_WEB=https://localhost:3000
REEARTH_CMS_ASSETBASEURL=https://localhost:8080/assets
REEARTH_CMS_DEV=false
REEARTH_CMS_SIGNUPSECRET=
REEARTH_CMS_ORIGINS=www.example.com,localhost:3000
Expand Down Expand Up @@ -43,6 +42,16 @@ REEARTH_CMS_GCS_PUBLICATIONCACHECONTROL=public,max-age=31536000
REEARTH_CMS_S3_BUCKETNAME=
REEARTH_CMS_S3_PUBLICATIONCACHECONTROL=public,max-age=31536000

#Assets

REEARTH_CMS_ASSETBASEURL=https://localhost:8080

# for GCS and S3 only, you can set the public flag to make the assets public
# and then can be accessed without from bucket url directly
# if this flag is set to false then cms will control the bucket ACL for the public assets
# and the privet assets can be accessed only by the cms api protected by auth
REEARTH_CMS_ASSET_PUBLIC=true
yk-eukarya marked this conversation as resolved.
Show resolved Hide resolved

#Auth
#there are multiple ways to set up auth

Expand Down
1 change: 1 addition & 0 deletions server/e2e/gql_model_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@ func TestCreateModel(t *testing.T) {
HasValue("key", "test-1")

}

func TestUpdateModel(t *testing.T) {
e := StartServer(t, &app.Config{}, true, baseSeederUser)

Expand Down
2 changes: 1 addition & 1 deletion server/e2e/integration_schema_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
)

// GET /projects/{projectIdOrKey}/schemata
func TestIntegrationScemaFilterAPI(t *testing.T) {
func TestIntegrationSchemaFilterAPI(t *testing.T) {
endpoint := "/api/projects/{projectIdOrKey}/schemata"
e := StartServer(t, &app.Config{}, true, baseSeeder)

Expand Down
2 changes: 1 addition & 1 deletion server/internal/adapter/gql/resolver_asset.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

85 changes: 79 additions & 6 deletions server/internal/adapter/integration/asset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ func (s *Server) AssetFilter(ctx context.Context, request AssetFilterRequestObje
}

itemList, err := util.TryMap(assets, func(a *asset.Asset) (integrationapi.Asset, error) {
aurl := uc.Asset.GetURL(a)
aa := integrationapi.NewAsset(a, nil, aurl, true)
aUrl := uc.Asset.GetURL(a)
aa := integrationapi.NewAsset(a, nil, aUrl, true)
return *aa, nil
})
if err != nil {
Expand Down Expand Up @@ -123,8 +123,8 @@ func (s *Server) AssetCreate(ctx context.Context, request AssetCreateRequestObje
return AssetCreate400Response{}, err
}

aurl := uc.Asset.GetURL(a)
aa := integrationapi.NewAsset(a, af, aurl, true)
aUrl := uc.Asset.GetURL(a)
aa := integrationapi.NewAsset(a, af, aUrl, true)
return AssetCreate200JSONResponse(*aa), nil
}

Expand Down Expand Up @@ -161,8 +161,8 @@ func (s *Server) AssetGet(ctx context.Context, request AssetGetRequestObject) (A
return AssetGet400Response{}, err
}

aurl := uc.Asset.GetURL(a)
aa := integrationapi.NewAsset(a, f, aurl, true)
aUrl := uc.Asset.GetURL(a)
aa := integrationapi.NewAsset(a, f, aUrl, true)
return AssetGet200JSONResponse(*aa), nil
}

Expand Down Expand Up @@ -191,3 +191,76 @@ func (s *Server) AssetUploadCreate(ctx context.Context, request AssetUploadCreat
Next: &au.Next,
}, nil
}

func (s *Server) AssetContentGet(ctx context.Context, request AssetContentGetRequestObject) (AssetContentGetResponseObject, error) {
uc := adapter.Usecases(ctx)
op := adapter.Operator(ctx)

a, err := uc.Asset.FindByUUID(ctx, request.Uuid1+request.Uuid2, op)
if err != nil {
if errors.Is(err, rerror.ErrNotFound) {
return AssetContentGet404Response{}, err
}
return AssetContentGet400Response{}, err
}
if a.FileName() != request.Filename {
return AssetContentGet404Response{}, rerror.ErrNotFound
}

rc, err := uc.Asset.DownloadByID(ctx, a.ID(), op)
if err != nil {
if errors.Is(err, rerror.ErrNotFound) {
return AssetContentGet404Response{}, err
}
return AssetContentGet400Response{}, err
}

return AssetContentGet200ApplicationoctetStreamResponse{
Body: rc,
ContentLength: int64(a.Size()),
}, nil
}
Comment on lines +195 to +222
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add Content-Type header and implement streaming for large files.

Consider the following improvements:

  1. Set the Content-Type header based on the asset's content type to prevent content sniffing attacks
  2. Implement streaming for large files to prevent memory issues
 return AssetContentGet200ApplicationoctetStreamResponse{
     Body:          rc,
     ContentLength: int64(a.Size()),
+    ContentType:   a.Type(),
 }, nil

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


func (s *Server) AssetPublish(ctx context.Context, request AssetPublishRequestObject) (AssetPublishResponseObject, error) {
uc := adapter.Usecases(ctx)
op := adapter.Operator(ctx)

a, err := uc.Asset.Publish(ctx, request.AssetId, op)
if err != nil {
if errors.Is(err, rerror.ErrNotFound) {
return AssetPublish404Response{}, err
}
return AssetPublish404Response{}, err
}
Comment on lines +229 to +234
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix inconsistent error handling in AssetPublish.

The error handling returns 404 for all error cases, which is incorrect. Non-404 errors should return 400.

 if err != nil {
     if errors.Is(err, rerror.ErrNotFound) {
         return AssetPublish404Response{}, err
     }
-    return AssetPublish404Response{}, err
+    return AssetPublish400Response{}, err
 }
📝 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
if err != nil {
if errors.Is(err, rerror.ErrNotFound) {
return AssetPublish404Response{}, err
}
return AssetPublish404Response{}, err
}
if err != nil {
if errors.Is(err, rerror.ErrNotFound) {
return AssetPublish404Response{}, err
}
return AssetPublish400Response{}, err
}


f, err := uc.Asset.FindFileByID(ctx, request.AssetId, op)
if err != nil && !errors.Is(err, rerror.ErrNotFound) {
return AssetPublish404Response{}, err
}

aUrl := uc.Asset.GetURL(a)
aa := integrationapi.NewAsset(a, f, aUrl, true)
return AssetPublish200JSONResponse(*aa), nil
}

func (s *Server) AssetUnpublish(ctx context.Context, request AssetUnpublishRequestObject) (AssetUnpublishResponseObject, error) {
uc := adapter.Usecases(ctx)
op := adapter.Operator(ctx)

a, err := uc.Asset.Unpublish(ctx, request.AssetId, op)
if err != nil {
if errors.Is(err, rerror.ErrNotFound) {
return AssetUnpublish404Response{}, err
}
return AssetUnpublish404Response{}, err
}
Comment on lines +251 to +256
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix inconsistent error handling in AssetUnpublish.

The error handling returns 404 for all error cases, which is incorrect. Non-404 errors should return 400.

 if err != nil {
     if errors.Is(err, rerror.ErrNotFound) {
         return AssetUnpublish404Response{}, err
     }
-    return AssetUnpublish404Response{}, err
+    return AssetUnpublish400Response{}, err
 }
📝 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
if err != nil {
if errors.Is(err, rerror.ErrNotFound) {
return AssetUnpublish404Response{}, err
}
return AssetUnpublish404Response{}, err
}
if err != nil {
if errors.Is(err, rerror.ErrNotFound) {
return AssetUnpublish404Response{}, err
}
return AssetUnpublish400Response{}, err
}


f, err := uc.Asset.FindFileByID(ctx, request.AssetId, op)
if err != nil && !errors.Is(err, rerror.ErrNotFound) {
return AssetUnpublish404Response{}, err
}

aUrl := uc.Asset.GetURL(a)
aa := integrationapi.NewAsset(a, f, aUrl, true)
return AssetUnpublish200JSONResponse(*aa), nil
}
Loading
Loading