Skip to content

Commit

Permalink
Fixes file count (#1781)
Browse files Browse the repository at this point in the history

Co-authored-by: Robert-Anthony Lee-Faison <[email protected]>
  • Loading branch information
hectorcorrea and leefaisonr authored Apr 24, 2024
1 parent c36b9cd commit e707bd4
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 30 deletions.
11 changes: 1 addition & 10 deletions app/services/s3_query_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -325,16 +325,7 @@ def md5(io:)

def count_objects(bucket_name: self.bucket_name, prefix: self.prefix)
responses = s3_responses(bucket_name:, prefix:)
total_key_count = responses.reduce(0) { |total, resp| total + resp.key_count }
if total_key_count == 0
# if the bucket does not exist
# it will return 0
0
else
# if the bucket does exist
# s3 always sends back the bucket key as the first response, so we should not count it
total_key_count - 1
end
responses.reduce(0) { |total, resp| total + resp.key_count }
end

private
Expand Down
32 changes: 12 additions & 20 deletions spec/services/s3_query_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,6 @@
{
is_truncated: false,
contents: [
{
etag: "\"#{s3_etag1}\"",
key: s3_bucket_key,
last_modified: s3_last_modified1,
size: 0,
storage_class: "STANDARD"
},
{
etag: "\"#{s3_etag1}\"",
key: s3_key1,
Expand All @@ -46,7 +39,8 @@
size: 0,
storage_class: "STANDARD"
}
]
],
key_count: 3
}
end
let(:empty_s3_hash) do
Expand Down Expand Up @@ -383,7 +377,7 @@

describe "#count_objects" do
before do
fake_s3_resp.stub(:key_count).and_return(s3_hash[:contents].count)
fake_s3_resp.stub(:key_count).and_return(s3_hash[:key_count])
fake_s3_resp.stub(:is_truncated).and_return(false)
end

Expand All @@ -393,12 +387,12 @@

context "truncated results" do
before do
fake_s3_resp.stub(:key_count).and_return(s3_hash[:contents].count)
fake_s3_resp.stub(:key_count).and_return(s3_hash[:key_count])
fake_s3_resp.stub(:is_truncated).and_return(true, false)
fake_s3_resp.stub(:next_continuation_token).and_return("abc")
end
it "returns all the objects" do
expect(s3_query_service.count_objects).to eq(7) # do not count the bucket the first time
expect(s3_query_service.count_objects).to eq(6)
end
end

Expand Down Expand Up @@ -746,15 +740,13 @@

it "retrieves the directories if requested" do
files = s3_query_service.client_s3_files(reload: true, bucket_name: "other-bucket", prefix: "new-prefix", ignore_directories: false)
expect(files.count).to eq 8
expect(files.first.filename).to match(/#{s3_bucket_key}/)
expect(files[1].filename).to match(/README/)
expect(files[2].filename).to match(/SCoData_combined_v1_2020-07_datapackage.json/)
expect(files[3].filename).to match(/directory/)
expect(files[4].filename).to match(/#{s3_bucket_key}/)
expect(files[5].filename).to match(/README/)
expect(files[6].filename).to match(/SCoData_combined_v1_2020-07_datapackage.json/)
expect(files[7].filename).to match(/directory/)
expect(files.count).to eq 6
expect(files[0].filename).to match(/README/)
expect(files[1].filename).to match(/SCoData_combined_v1_2020-07_datapackage.json/)
expect(files[2].filename).to match(/directory/)
expect(files[3].filename).to match(/README/)
expect(files[4].filename).to match(/SCoData_combined_v1_2020-07_datapackage.json/)
expect(files[5].filename).to match(/directory/)
expect(fake_aws_client).to have_received(:list_objects_v2).with(bucket: "other-bucket", max_keys: 1000, prefix: "new-prefix")
expect(fake_aws_client).to have_received(:list_objects_v2).with(bucket: "other-bucket", continuation_token: "abc123", max_keys: 1000, prefix: "new-prefix")
end
Expand Down

0 comments on commit e707bd4

Please sign in to comment.