Skip to content

feat(core): update storage outputs class for multi-bucket support #5476

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

Merged
merged 10 commits into from
Sep 23, 2024
Merged

feat(core): update storage outputs class for multi-bucket support #5476

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
13d6723
updating storage_outputs class for multi-bucket support
Sep 17, 2024
c5c0438
seperated bucket out into its own class instead of using a map
Sep 17, 2024
f22dd48
made bucket_output class a proper output class like the others in amp…
Sep 18, 2024
cc6c55b
Update auth.dart
ekjotmultani Sep 18, 2024
b6f8719
added doc comments and changed name of bucket class
Sep 18, 2024
66ad860
Merge branch 'storagewereoutput-update' of https://github.com/aws-amp…
Sep 18, 2024
3fc0ce9
Delete devtools_options.yaml
ekjotmultani Sep 18, 2024
9114206
added trailing commas to pass ci test
Sep 18, 2024
a17b602
Merge branch 'storage-output-update' of https://github.com/aws-amplif…
Sep 18, 2024
e7906c7
ran dart format on two failing files
Sep 18, 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
16 changes: 16 additions & 0 deletions packages/amplify_core/lib/src/config/amplify_outputs/storage/storage_output_bucket.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0



/// {@template amplify_core.amplify_outputs.amazon_pinpoint_channel}
/// Supported channels for Amazon Pinpoint.
/// {@endtemplate}
class StorageOutputBucket {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
class StorageOutputBucket {
class BucketOutputs {

StorageOutputBucket(this.name, this.bucketName, this.awsRegion);
Copy link
Contributor

Choose a reason for hiding this comment

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

the ctor can be const. I would suggest to use named parameter similat to other Amplify Outputs type

factory StorageOutputBucket.fromJson(Map<String, dynamic> json) => StorageOutputBucket(json['name'].toString(), json['bucket_name'].toString(), json['aws_region'].toString());
String name;
String bucketName;
String awsRegion;
Map<String, dynamic> toJson() => <String, dynamic>{'name':name, 'bucket_name':bucketName, 'aws_region':awsRegion};
}
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of wrting the fromJson and toJson use generated code similar to other Amplify Outputs types.

Copy link
Member Author

Choose a reason for hiding this comment

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

if these functions are not written into the class no code gets generated and an error is thrown because the generator does not know how to serialize and deserialize this object

9 changes: 7 additions & 2 deletions packages/amplify_core/lib/src/config/amplify_outputs/storage/storage_outputs.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

import 'package:amplify_core/amplify_core.dart';
import 'package:amplify_core/src/config/amplify_outputs/storage/storage_output_bucket.dart';

part 'storage_outputs.g.dart';

Expand All @@ -12,7 +13,8 @@ part 'storage_outputs.g.dart';
class StorageOutputs
with AWSEquatable<StorageOutputs>, AWSSerializable, AWSDebuggable {
/// {@macro amplify_core.amplify_outputs.storage_outputs}
const StorageOutputs({required this.awsRegion, required this.bucketName});
const StorageOutputs({required this.awsRegion, required this.bucketName, this.buckets});


factory StorageOutputs.fromJson(Map<String, Object?> json) =>
_$StorageOutputsFromJson(json);
Expand All @@ -23,8 +25,11 @@ class StorageOutputs
/// The Amazon S3 bucket name.
final String bucketName;

/// The list of buckets if there are multiple buckets for the project
final List<StorageOutputBucket>? buckets;

@override
List<Object?> get props => [awsRegion, bucketName];
List<Object?> get props => [awsRegion, bucketName, buckets];

@override
String get runtimeTypeName => 'StorageOutputs';
Expand Down
26 changes: 21 additions & 5 deletions packages/amplify_core/lib/src/config/amplify_outputs/storage/storage_outputs.g.dart
View file
Open in desktop

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

23 changes: 20 additions & 3 deletions packages/amplify_core/test/config/amplify_outputs/test_data.dart
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: I see the test data has been updated, but are the tests written in a way that validate the new functionality too? Do they need to be updated?

Copy link
Member Author

@ekjotmultani ekjotmultani Sep 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This is a good question, since multiple buckets are optional, I think we should validate that it works when only one bucket is supplied too. I'll check this out

Copy link
Member Author

Choose a reason for hiding this comment

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

I've verified that the test does work properly when only a single bucket is supplied and that the test itself actually validates the new function for multi-bucket support

Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,26 @@ const amplifyoutputs = '''{
]
},
"storage": {
"aws_region": "oem dks",
"bucket_name": "dolor et esse"
},
"aws_region": "us-east-2",
"bucket_name": "amplifyTeamDrive-one-stora-testbucketgen2bucket0b8c-9ggcfqfunkjr",
"buckets": [
{
"name": "amplifyTeamDrive-one",
"bucket_name": "amplifyTeamDrive-one-stora-testbucketgen2bucket0b8c-9ggcfqfunkjr",
"aws_region": "us-east-2"
},
{
"name": "amplifyTeamDrive-two",
"bucket_name": "amplifyTeamDrive-two-stora-testbucketgen2bucket0b8c-2",
"aws_region": "us-east-2"
},
{
"name": "amplifyTeamDrive-three",
"bucket_name": "amplifyTeamDrive-three-stora-testbucketgen2bucket0b8c-3",
"aws_region": "us-east-2"
}
]
},
"version": "1"
}
''';
Loading