jamhall / s3rver

A fake S3 server written in NodeJs
MIT License
574 stars 149 forks source link

listObjectsV2 does not consider "Prefix" parameter #60

Closed Junkern closed 6 years ago

Junkern commented 7 years ago

When calling this.s3Client.listObjectsV2({Bucket: 'baseBucket', Prefix: 'pending'}) the following is returned:

GET /baseBucket?list-type=2&prefix=pending 200 1763 - 7.539 ms
{ IsTruncated: false,
  Contents: 
   [ { Key: 'harFiles/2',
       LastModified: 2017-03-07T14:17:01.073Z,
       ETag: '"9d6b3ad272abe35c3c6b95a948b000db"',
       Size: 14,
       StorageClass: 'Standard',
       Owner: [Object] },
     { Key: 'pending/1',
       LastModified: 2017-03-07T14:17:01.353Z,
       ETag: '"10ed3a4e7f510696c325d6249c82d69e"',
       Size: 14,
       StorageClass: 'Standard',
       Owner: [Object] },
     { Key: 'recipes/1',
       LastModified: 2017-03-07T14:17:01.073Z,
       ETag: '"d4793fe3394939cf279c23e9045f7afc"',
       Size: 36,
       StorageClass: 'Standard',
       Owner: [Object] },
     { Key: 'recipes/2',
       LastModified: 2017-03-07T14:17:01.333Z,
       ETag: '"d897f47178a9e3a89bf4e8abe0497eed"',
       Size: 39,
       StorageClass: 'Standard',
       Owner: [Object] },
     { Key: 'recipes/5',
       LastModified: 2017-03-07T14:17:01.303Z,
       ETag: '"f3e713eee1619c0eeb29b42a6a4aab8d"',
       Size: 29,
       StorageClass: 'Standard',
       Owner: [Object] } ],
  Prefix: '',
  MaxKeys: 1000,
  CommonPrefixes: [] }

Although I indicated a Prefix pending, keys which start with recipes are included.

n1ru4l commented 6 years ago

I created a test for this scenario in #92, seems to work.. If you have anything to add please do so ☺️ Otherwise I am going to close this issue

leontastic commented 6 years ago

The test added in #92 is just a copy of the previous test, it doesn't test anything new. A test for this scenario should fail if there are other files in the response that don't match the prefix.

n1ru4l commented 6 years ago

It actually tests the method @Junkern mentioned. Since it is not failing this is actually not an issue (anymore)?

Edit: Also I do not like the fact that different test can affect each other.

Junkern commented 6 years ago

I don't have my original setup at hand at the moment, so I can't verify whether it works or not. But if the tests are running it seems to be fine?

Regarding the fact that tests affect each other:

I would use beforeEach to spin up the s3rver and close it in afterEach. That will lead to longer running tests, but then each it is running isolated. However, for that the cleanup should also clean up the file system (so we don't end up) creating the files n-times. And for that #58 is probably helpful.

leontastic commented 6 years ago

Seems like the tests in #92 pass even after adding checks for files not matching the prefix. Will merge it and close this.