Skip to content

Commit 193cc0f

Browse files
authored
feat(uploads): [PPT-2302] add storage_id parameter to index endpoint for listing uploads from specific storages (#423)
1 parent 8a85f9c commit 193cc0f

3 files changed

Lines changed: 148 additions & 5 deletions

File tree

OPENAPI_DOC.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19252,6 +19252,14 @@ paths:
1925219252
items:
1925319253
type: string
1925419254
nullable: true
19255+
- name: storage_id
19256+
in: query
19257+
description: storage id to list uploads from (defaults to authority's default
19258+
storage)
19259+
example: storage-XXX
19260+
schema:
19261+
type: string
19262+
nullable: true
1925519263
- name: q
1925619264
in: query
1925719265
description: returns results based on a [simple query string](https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-simple-query-string-query.html)

spec/controllers/uploads_spec.cr

Lines changed: 126 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -288,13 +288,14 @@ module PlaceOS::Api
288288
sig["url"].as_s.size.should eq(0)
289289
upload = Model::Upload.find!(info["upload_id"].as_s)
290290

291+
part_number = "1"
291292
params = {
292-
"part" => Base64.strict_encode(UUID.random.to_s),
293+
"part" => part_number,
293294
"file_id" => "some_file_md5_hash",
294295
}
295296

296-
pinfo = Uploads::PartInfo.new(params["file_id"], 1, params["part"])
297-
uinfo = Uploads::UpdateInfo.new(params["file_id"], 1, "some-random-resumable-id", [pinfo], [1], false)
297+
pinfo = Uploads::PartInfo.new(md5: params["file_id"], part: 1)
298+
uinfo = Uploads::UpdateInfo.new(resumable_id: "some-random-resumable-id", part_data: [pinfo], part_list: [1])
298299

299300
resp = client.patch(
300301
path: "#{Uploads.base_route}/#{upload.id}?#{HTTP::Params.encode(params)}",
@@ -310,7 +311,9 @@ module PlaceOS::Api
310311
uri = URI.parse(sig["url"].as_s)
311312
uri.host.should eq(sprintf("%s.blob.core.windows.net", "myteststorage"))
312313
qparams = URI::Params.parse(uri.query || "")
313-
qparams["blockid"].should eq(params["part"])
314+
# Azure encodes the part as Base64(part.rjust(6, '0'))
315+
expected_block_id = Base64.strict_encode(part_number.rjust(6, '0'))
316+
qparams["blockid"].should eq(expected_block_id)
314317

315318
params = {
316319
"part" => "finish",
@@ -324,5 +327,124 @@ module PlaceOS::Api
324327
info["type"].should eq("finish")
325328
info["body"].should_not be_nil
326329
end
330+
331+
it "should list uploads from a specific storage when storage_id is provided" do
332+
authority = Model::Authority.find_by_domain("localhost").not_nil!
333+
334+
# Create two storages for the same authority
335+
storage1 = Model::Generator.storage(authority_id: authority.id)
336+
storage1.is_default = true
337+
storage1.save!
338+
339+
storage2 = Model::Generator.storage(authority_id: authority.id)
340+
storage2.is_default = false
341+
storage2.save!
342+
343+
# Create uploads in both storages
344+
Model::Generator.upload(file_name: "file_in_storage1", storage_id: storage1.id).save!
345+
Model::Generator.upload(file_name: "file_in_storage2_a", storage_id: storage2.id).save!
346+
Model::Generator.upload(file_name: "file_in_storage2_b", storage_id: storage2.id).save!
347+
348+
# List uploads from storage2
349+
params = HTTP::Params.encode({
350+
"storage_id" => storage2.id.as(String),
351+
})
352+
353+
result = client.get("#{Uploads.base_route}/?#{params}",
354+
headers: Spec::Authentication.headers)
355+
356+
result.success?.should be_true
357+
result.headers["X-Total-Count"].should eq "2"
358+
uploads = Array(Model::Upload).from_json(result.body)
359+
uploads.size.should eq(2)
360+
uploads.all? { |u| u.storage_id == storage2.id }.should be_true
361+
end
362+
363+
it "should list uploads from default storage when storage_id is not provided" do
364+
authority = Model::Authority.find_by_domain("localhost").not_nil!
365+
366+
# Create two storages
367+
storage1 = Model::Generator.storage(authority_id: authority.id)
368+
storage1.is_default = true
369+
storage1.save!
370+
371+
storage2 = Model::Generator.storage(authority_id: authority.id)
372+
storage2.is_default = false
373+
storage2.save!
374+
375+
# Create uploads in both storages
376+
Model::Generator.upload(file_name: "default_file", storage_id: storage1.id).save!
377+
Model::Generator.upload(file_name: "other_file", storage_id: storage2.id).save!
378+
379+
# List uploads without storage_id (should use default)
380+
result = client.get(Uploads.base_route,
381+
headers: Spec::Authentication.headers)
382+
383+
result.success?.should be_true
384+
result.headers["X-Total-Count"].should eq "1"
385+
uploads = Array(Model::Upload).from_json(result.body)
386+
uploads.size.should eq(1)
387+
uploads.first.storage_id.should eq(storage1.id)
388+
uploads.first.file_name.should eq("default_file")
389+
end
390+
391+
it "should return 404 when storage_id does not exist" do
392+
params = HTTP::Params.encode({
393+
"storage_id" => "storage-nonexistent",
394+
})
395+
396+
result = client.get("#{Uploads.base_route}/?#{params}",
397+
headers: Spec::Authentication.headers)
398+
399+
result.status_code.should eq(404)
400+
JSON.parse(result.body).as_h["error"].as_s.should contain("Storage not found")
401+
end
402+
403+
it "should return 403 when storage_id belongs to different authority" do
404+
authority = Model::Authority.find_by_domain("localhost").not_nil!
405+
406+
# Create storage for a different authority
407+
other_authority = Model::Generator.authority
408+
other_authority.domain = "other.example.com"
409+
other_authority.save!
410+
411+
other_storage = Model::Generator.storage(authority_id: other_authority.id)
412+
other_storage.save!
413+
414+
# Try to list uploads from other authority's storage
415+
params = HTTP::Params.encode({
416+
"storage_id" => other_storage.id.as(String),
417+
})
418+
419+
result = client.get("#{Uploads.base_route}/?#{params}",
420+
headers: Spec::Authentication.headers)
421+
422+
result.status_code.should eq(403)
423+
end
424+
425+
it "should allow access to global storage (authority_id is nil)" do
426+
authority = Model::Authority.find_by_domain("localhost").not_nil!
427+
428+
# Create a global storage (no authority_id)
429+
global_storage = Model::Generator.storage(authority_id: nil)
430+
global_storage.save!
431+
432+
# Create upload in global storage
433+
Model::Generator.upload(file_name: "global_file", storage_id: global_storage.id).save!
434+
435+
# List uploads from global storage
436+
params = HTTP::Params.encode({
437+
"storage_id" => global_storage.id.as(String),
438+
})
439+
440+
result = client.get("#{Uploads.base_route}/?#{params}",
441+
headers: Spec::Authentication.headers)
442+
443+
result.success?.should be_true
444+
result.headers["X-Total-Count"].should eq "1"
445+
uploads = Array(Model::Upload).from_json(result.body)
446+
uploads.size.should eq(1)
447+
uploads.first.file_name.should eq("global_file")
448+
end
327449
end
328450
end

src/placeos-rest-api/controllers/uploads.cr

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ module PlaceOS::Api
6363
offset : Int32 = 0,
6464
@[AC::Param::Info(description: "return uploads with particular tags", example: "staff,email")]
6565
tags : Array(String)? = nil,
66+
@[AC::Param::Info(description: "storage id to list uploads from (defaults to authority's default storage)", example: "storage-XXX")]
67+
storage_id : String? = nil,
6668
) : Array(::PlaceOS::Model::Upload)
6769
# validate each incoming tag
6870
if tags
@@ -75,7 +77,18 @@ module PlaceOS::Api
7577

7678
# 1. Prepare table name and storage
7779
table_name = ::PlaceOS::Model::Upload.table_name
78-
storage = ::PlaceOS::Model::Storage.storage_or_default(authority.id)
80+
storage = if storage_id
81+
# Validate that the storage exists and belongs to this authority
82+
unless found_storage = ::PlaceOS::Model::Storage.find?(storage_id)
83+
raise Error::NotFound.new("Storage not found: #{storage_id}")
84+
end
85+
unless found_storage.authority_id == authority.id || found_storage.authority_id.nil?
86+
raise Error::Forbidden.new("Storage does not belong to this authority")
87+
end
88+
found_storage
89+
else
90+
::PlaceOS::Model::Storage.storage_or_default(authority.id)
91+
end
7992

8093
# 2. Build conditions and bind params
8194
conditions = ["u.storage_id = $1"]

0 commit comments

Comments
 (0)