feat(storage): getProperties API #11378

kvramyasri7 · 2023-05-16T23:35:13Z

Description of changes

This api enables users to retrieve properties of object from storage.

Issue #, if available

#6830
#1550

Description of how you validated changes

Tested with a sample app and added unit test cases

Checklist

PR description included
yarn test passes
Tests are changed or added
Relevant documentation is changed or added (and PR referenced)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

codecov-commenter · 2023-05-16T23:55:12Z

Codecov Report

Merging #11378 (eea6590) into main (58a26dc) will increase coverage by 0.00%.
The diff coverage is 91.66%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@           Coverage Diff           @@
##             main   #11378   +/-   ##
=======================================
  Coverage   82.99%   82.99%           
=======================================
  Files         240      240           
  Lines       20163    20179   +16     
  Branches     4355     4358    +3     
=======================================
+ Hits        16734    16748   +14     
- Misses       3142     3144    +2     
  Partials      287      287

Impacted Files	Coverage Δ
packages/storage/src/providers/AWSS3Provider.ts	`91.41% <85.71%> (-0.48%)`	⬇️
packages/storage/src/Storage.ts	`97.97% <100.00%> (-0.16%)`	⬇️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

jimblanc

Looks good! One comment below about SSE support.

packages/storage/src/providers/AWSS3Provider.ts

jimblanc · 2023-05-17T15:32:21Z

packages/storage/src/providers/AWSS3Provider.ts

+		const s3 = this._createNewS3Client(opt, emitter);
+		logger.debug('getProperties ' + key + ' from ' + final_key);
+
+		const params: HeadObjectCommandInput = {


IMO this API should support SSE to align with the other APIs (HeadObject command supports it it looks like). It would be strange if you can upload/download SSE-enabled objects but can't get the meta-data for them.

Added SSE support!

packages/storage/src/Storage.ts

packages/storage/src/types/AWSS3Provider.ts

Co-authored-by: Jim Blanchard <jim.l.blanchard@gmail.com>

AllanZhengYP

It looks good to me generally and have minor comments in for the test. I think we need to confirm whether SSE parameters can be omitted before merging it.

packages/storage/__tests__/providers/AWSS3Provider-unit-test.ts

AllanZhengYP

🚀🚀🚀

Samaritan1011001 · 2023-05-18T00:50:25Z

packages/storage/src/Storage.ts

+		config?: StorageGetPropertiesConfig<T>
+	): StorageGetPropertiesOutput<T> {
+		const provider = config?.provider || DEFAULT_PROVIDER;
+		const prov = this._pluggables.find(


nit: prov can be made more descriptive? Also there seems to be a util function to get the provider.

AllanZhengYP

🚢🚢🚢

jimblanc · 2023-05-22T12:49:40Z

packages/storage/__tests__/Storage-unit-test.ts

 				await storage.put('key', 'test upload');
 			} catch (err) {
-				expect(err).toEqual('No plugin found in Storage for the provider');
+				expect(err).toEqual(new Error('No plugin found with providerName'));


Nit: It would be a little cleaner (here and elsewhere) to test failure modes like this:

await expect(storage.put('key', 'test upload')).rejects.toThrow(new Error('No plugin found with providerName'));

jimblanc · 2023-05-22T13:03:27Z

packages/storage/src/Storage.ts

-			return null;
+			throw new Error('No plugin found with providerName');


This approach makes sense and is cleaner, but I'm just worried if this might constitute a breaking change on a public API surface for folks who may have implemented their own providers or be using this method directly in some non-standard ward.

yeah that's a valid point. Thanks.

jimblanc · 2023-05-22T13:04:24Z

packages/storage/src/Storage.ts

-		const prov = this._pluggables.find(
-			pluggable => pluggable.getProviderName() === provider
-		);
-		if (prov === undefined) {
-			logger.debug('No plugin found with providerName', provider);
-			return Promise.reject(
-				'No plugin found in Storage for the provider'
-			) as StorageCopyOutput<T>;
-		}


Thank you for cleaning this up!

jimblanc

LGTM, nice work

This reverts commit 3bed12b.

* chore(release): Publish [ci skip] - aws-amplify@5.2.3 - @aws-amplify/datastore-storage-adapter@2.0.33 - @aws-amplify/datastore@4.4.0 - @aws-amplify/predictions@5.2.0 - @aws-amplify/storage@5.3.0 * chore(release): update API docs [ci skip] * fix(notifications): Add babel plugins to devDependencies (#11414) * fix(storage): make getProperties param optional (#11413) * Revert "fix(storage): make getProperties param optional (#11413)" This reverts commit 5d00b1f. * Revert "feat(storage): getProperties API (#11378)" This reverts commit 3bed12b. --------- Co-authored-by: israx <70438514+israx@users.noreply.github.com> Co-authored-by: aws-amplify-bot <aws@amazon.com> Co-authored-by: Chris F <5827964+cshfang@users.noreply.github.com> Co-authored-by: Jim Blanchard <jablanch@amazon.com> Co-authored-by: Jim Blanchard <jim.l.blanchard@gmail.com>

* chore(release): Publish [ci skip] - aws-amplify@5.2.3 - @aws-amplify/datastore-storage-adapter@2.0.33 - @aws-amplify/datastore@4.4.0 - @aws-amplify/predictions@5.2.0 - @aws-amplify/storage@5.3.0 * chore(release): update API docs [ci skip] * fix(notifications): Add babel plugins to devDependencies (#11414) * fix(storage): make getProperties param optional (#11413) * Revert "fix(storage): make getProperties param optional (#11413)" This reverts commit 5d00b1f. * Revert "feat(storage): getProperties API (#11378)" This reverts commit 3bed12b. * fix(core): bundle react-native-url-polyfill to unblock jest test failure (#11422) * fix(core): bundle react-native-url-polyfill to unblock jest test failure * Revert "test(notification): unblock unit test failure of rn url polyfill" This reverts commit dbacebc. * chore: remove unused code * chore: remove unused polyfills/URL/package.json * chore(release): Publish [ci skip] - @aws-amplify/analytics@6.1.1 - @aws-amplify/api-graphql@3.2.1 - @aws-amplify/api-rest@3.1.1 - @aws-amplify/api@5.1.1 - @aws-amplify/auth@5.3.7 - aws-amplify@5.2.4 - @aws-amplify/cache@5.0.33 - @aws-amplify/core@5.3.1 - @aws-amplify/datastore-storage-adapter@2.0.34 - @aws-amplify/datastore@4.4.1 - @aws-amplify/geo@2.0.33 - @aws-amplify/interactions@5.0.33 - @aws-amplify/notifications@1.1.7 - @aws-amplify/predictions@5.2.1 - @aws-amplify/pubsub@5.1.16 - @aws-amplify/pushnotification@5.0.33 - @aws-amplify/storage@5.3.1 * chore(release): update API docs [ci skip] --------- Co-authored-by: israx <70438514+israx@users.noreply.github.com> Co-authored-by: aws-amplify-bot <aws@amazon.com> Co-authored-by: Chris F <5827964+cshfang@users.noreply.github.com> Co-authored-by: Jim Blanchard <jablanch@amazon.com> Co-authored-by: Jim Blanchard <jim.l.blanchard@gmail.com> Co-authored-by: AllanZhengYP <zheallan@amazon.com>

Venkata Ramyasri Kota added 7 commits May 16, 2023 12:18

chore: add getProperties in StorageProvider

7c22a1e

chore: add S3ProviderGetPropertiesOutput type

2d473d1

chore: add types in Storage.ts

ab00587

chore: add plugin level getProperties method

7a6fa39

feat: add AWSS3Provider getProperties Implementation

92aee69

chore: add unit test cases

796c11f

fix: increase size-limit for predictions package

af5098f

Merge branch 'aws-amplify:main' into storage/getProperties_API

2251f9b

kvramyasri7 marked this pull request as ready for review May 17, 2023 15:13

kvramyasri7 requested a review from a team as a code owner May 17, 2023 15:13

jimblanc reviewed May 17, 2023

View reviewed changes

israx reviewed May 17, 2023

View reviewed changes

packages/storage/src/Storage.ts Outdated Show resolved Hide resolved

packages/storage/src/Storage.ts Outdated Show resolved Hide resolved

packages/storage/src/types/AWSS3Provider.ts Outdated Show resolved Hide resolved

Venkata Ramyasri Kota and others added 4 commits May 17, 2023 08:54

Update packages/storage/src/providers/AWSS3Provider.ts

d21c027

Co-authored-by: Jim Blanchard <jim.l.blanchard@gmail.com>

fix: fixed string literal

f7a6160

fix: change metaData to metadata

f764581

fix:change reject to Error

8f4572e

AllanZhengYP reviewed May 17, 2023

View reviewed changes

packages/storage/__tests__/providers/AWSS3Provider-unit-test.ts Show resolved Hide resolved

packages/storage/__tests__/providers/AWSS3Provider-unit-test.ts Show resolved Hide resolved

packages/storage/__tests__/providers/AWSS3Provider-unit-test.ts Show resolved Hide resolved

Venkata Ramyasri Kota added 3 commits May 17, 2023 10:29

chore: add extra line for readability

fdea3cc

chore: changes to support SSE parameters

e6c8a9f

chore: add assertion for spyon

5ad36c4

AllanZhengYP previously approved these changes May 17, 2023

View reviewed changes

Samaritan1011001 reviewed May 18, 2023

View reviewed changes

chore: use getPluggable utility function clean up

2bce874

kvramyasri7 dismissed AllanZhengYP’s stale review via 2bce874 May 18, 2023 16:03

Venkata Ramyasri Kota and others added 5 commits May 18, 2023 09:57

fix: change unit tests for getPluggable function

1a3837d

chore: add unit test for getProperties in storage-unit-tests

cecb214

fix: remove Date.now function

18f9e97

chore: add contentType in unit tests

0ad6dec

Merge branch 'main' into storage/getProperties_API

eea6590

AllanZhengYP previously approved these changes May 18, 2023

View reviewed changes

jimblanc reviewed May 22, 2023

View reviewed changes

Venkata Ramyasri Kota and others added 2 commits May 22, 2023 08:43

Merge branch 'aws-amplify:main' into storage/getProperties_API

68cd516

fix: revert getPluggable utility changes

405e8f8

kvramyasri7 dismissed AllanZhengYP’s stale review via 405e8f8 May 22, 2023 16:44

jimblanc approved these changes May 22, 2023

View reviewed changes

AllanZhengYP approved these changes May 22, 2023

View reviewed changes

Merge branch 'main' into storage/getProperties_API

8449227

kvramyasri7 merged commit 3bed12b into aws-amplify:main May 22, 2023

jimblanc added a commit to jimblanc/amplify-js that referenced this pull request May 26, 2023

Revert "feat(storage): getProperties API (aws-amplify#11378)"

7179636

This reverts commit 3bed12b.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(storage): getProperties API #11378

feat(storage): getProperties API #11378

kvramyasri7 commented May 16, 2023

codecov-commenter commented May 16, 2023 •

edited

Loading

jimblanc left a comment

jimblanc May 17, 2023

kvramyasri7 May 17, 2023

AllanZhengYP left a comment

AllanZhengYP left a comment

Samaritan1011001 May 18, 2023

AllanZhengYP left a comment

jimblanc May 22, 2023

jimblanc May 22, 2023

kvramyasri7 May 22, 2023

jimblanc May 22, 2023

jimblanc left a comment

		return null;
		throw new Error('No plugin found with providerName');

feat(storage): getProperties API #11378

feat(storage): getProperties API #11378

Conversation

kvramyasri7 commented May 16, 2023

Description of changes

Issue #, if available

Description of how you validated changes

Checklist

codecov-commenter commented May 16, 2023 • edited Loading

Codecov Report

jimblanc left a comment

Choose a reason for hiding this comment

jimblanc May 17, 2023

Choose a reason for hiding this comment

kvramyasri7 May 17, 2023

Choose a reason for hiding this comment

AllanZhengYP left a comment

Choose a reason for hiding this comment

AllanZhengYP left a comment

Choose a reason for hiding this comment

Samaritan1011001 May 18, 2023

Choose a reason for hiding this comment

AllanZhengYP left a comment

Choose a reason for hiding this comment

jimblanc May 22, 2023

Choose a reason for hiding this comment

jimblanc May 22, 2023

Choose a reason for hiding this comment

kvramyasri7 May 22, 2023

Choose a reason for hiding this comment

jimblanc May 22, 2023

Choose a reason for hiding this comment

jimblanc left a comment

Choose a reason for hiding this comment

codecov-commenter commented May 16, 2023 •

edited

Loading