Skip to content

Commit

Permalink
chore: Added completer, removed conditional import, added closeable, …
Browse files Browse the repository at this point in the history 
…removed credentials provider from base class and only added to defaultconstraintprovider, changed fileStorage to use FileStorage instead of just FileStorageImpl, removed isRunning since it is never false after the first run, added completer so that delay isn't needed for tests, removed the catching of errors, added credentials provider parameter to awssigv4signer,
  • Loading branch information
@khatruong2009
khatruong2009 committed Sep 15, 2023
1 parent 507d086 commit bc635d8
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 95 deletions.
97 changes: 43 additions & 54 deletions packages/logging_cloudwatch/aws_logging_cloudwatch/lib/src/remote_constraint_provider.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ import 'package:amplify_core/amplify_core.dart';

import 'package:aws_common/aws_common.dart';
import 'package:aws_logging_cloudwatch/aws_logging_cloudwatch.dart';
import 'package:aws_logging_cloudwatch/src/file_storage/file_storage_stub.dart'
if (dart.library.io) 'package:aws_logging_cloudwatch/src/file_storage/file_storage_vm.dart'
if (dart.library.html) 'package:aws_logging_cloudwatch/src/file_storage/file_storage_web.dart';
import 'package:aws_logging_cloudwatch/src/file_storage/file_storage.dart';
import 'package:aws_signature_v4/aws_signature_v4.dart';
import 'package:meta/meta.dart';

Expand All @@ -32,15 +30,13 @@ abstract class RemoteLoggingConstraintProvider {
/// {@endtemplate}
base class BaseRemoteLoggingConstraintProvider
with AWSDebuggable, AWSLoggerMixin
implements RemoteLoggingConstraintProvider {
implements RemoteLoggingConstraintProvider, Closeable {
/// {@macro aws_logging_cloudwatch.base_remote_constraints_provider}
BaseRemoteLoggingConstraintProvider({
required DefaultRemoteConfiguration config,
required AWSCredentialsProvider credentialsProvider,
FileStorageImpl? fileStorage,
FileStorage? fileStorage,
}) : _fileStorage = fileStorage,
_config = config,
_credentialsProvider = credentialsProvider,
_awsHttpClient = AWSHttpClient() {
_init();
}
Expand All @@ -50,31 +46,27 @@ base class BaseRemoteLoggingConstraintProvider
@visibleForTesting
BaseRemoteLoggingConstraintProvider.forTesting({
required DefaultRemoteConfiguration config,
required AWSCredentialsProvider credentialsProvider,
required AWSHttpClient awsHttpClient,
FileStorageImpl? fileStorage,
FileStorage? fileStorage,
}) : _fileStorage = fileStorage,
_config = config,
_credentialsProvider = credentialsProvider,
_awsHttpClient = awsHttpClient {
_init();
}

final FileStorageImpl? _fileStorage;
final FileStorage? _fileStorage;

final DefaultRemoteConfiguration _config;
final AWSCredentialsProvider _credentialsProvider;

LoggingConstraint? _loggingConstraint;

final AWSHttpClient _awsHttpClient;

static const _cacheFileName = 'remoteloggingconstraints.json';

// The timer to refresh the constraint periodically.
Timer? _timer;

/// Whether the periodic fetch is running.
bool _isRunning = false;

/// Retrives the runtime type name used for logging.
@override
String get runtimeTypeName => 'BaseRemoteConstraintsProvider';
Expand All @@ -83,12 +75,17 @@ base class BaseRemoteLoggingConstraintProvider
/// the constraint from the endpoint initially and then
/// starting the refresh timer afterwards.
void _init() {
_refreshConstraintPeriodically();
_readyCompleter.complete(_refreshConstraintPeriodically());
}

final Completer<void> _readyCompleter = Completer();

/// A future that completes when the [BaseRemoteLoggingConstraintProvider]
Future<void> get ready => _readyCompleter.future;

/// Creates a request to fetch the constraint from the endpoint.
@protected
Future<AWSHttpRequest> createRequest() async {
Future<AWSBaseHttpRequest> createRequest() async {
final uri = Uri.parse(_config.endpoint);
return AWSHttpRequest(
method: AWSHttpMethod.get,
Expand All @@ -106,31 +103,25 @@ base class BaseRemoteLoggingConstraintProvider
final operation = _awsHttpClient.send(request);
final response = await operation.response;
final body = await response.decodeBody();
if (response.statusCode == 200) {
final fetchedConstraint = LoggingConstraint.fromJson(
jsonDecode(body) as Map<String, dynamic>,
if (response.statusCode != 200) {
logger.error('Failed to fetch constraints', body);
return;
}
final fetchedConstraint = LoggingConstraint.fromJson(
jsonDecode(body) as Map<String, dynamic>,
);
_loggingConstraint = fetchedConstraint;

if (_fileStorage != null) {
await _fileStorage!.save(
_cacheFileName,
jsonEncode(fetchedConstraint.toJson()),
);
_loggingConstraint = fetchedConstraint;

if (_fileStorage != null) {
await _fileStorage!.save(
'remoteloggingconstraints.json',
jsonEncode(fetchedConstraint.toJson()),
);
}
} else {
await _loadConstraintFromLocalCache();
}
} on Exception catch (exception) {
logger.error(
'Failed to fetch logging constraint from ${_config.endpoint}: $exception',
);
await _loadConstraintFromLocalCache();
} on Error catch (error, stackTrace) {
logger.error(
'Error while fetching logging constraint from ${_config.endpoint}: $error',
stackTrace,
);
}
}

Expand All @@ -139,8 +130,7 @@ base class BaseRemoteLoggingConstraintProvider
LoggingConstraint? get loggingConstraint => _loggingConstraint;

Future<void> _loadConstraintFromLocalCache() async {
final localConstraint =
await _fileStorage!.load('remoteloggingconstraints.json');
final localConstraint = await _fileStorage!.load(_cacheFileName);
if (localConstraint != null) {
_loggingConstraint = LoggingConstraint.fromJson(
jsonDecode(localConstraint) as Map<String, dynamic>,
Expand All @@ -149,19 +139,19 @@ base class BaseRemoteLoggingConstraintProvider
}

/// Refreshes the constraint from the endpoint periodically.
void _refreshConstraintPeriodically() {
if (_isRunning) {
return;
}

_isRunning = true;
_timer?.cancel();

Future<void> _refreshConstraintPeriodically() async {
await _loadConstraintFromLocalCache();
_timer = Timer.periodic(
_config.refreshInterval,
(_) => _fetchAndCacheConstraintFromEndpoint(),
);
Timer.run(_fetchAndCacheConstraintFromEndpoint);
await _fetchAndCacheConstraintFromEndpoint();
}

@override
void close() {
_timer?.cancel();
_timer = null;
}
}

Expand All @@ -176,16 +166,18 @@ final class DefaultRemoteLoggingConstraintProvider
required super.config,
required this.credentialsProvider,
super.fileStorage,
}) : super(credentialsProvider: credentialsProvider);
});

/// The credentials provider to use for signing the request.
final AWSCredentialsProvider credentialsProvider;

/// The signer to use for signing the request.
final AWSSigV4Signer _signer = const AWSSigV4Signer();
late final AWSSigV4Signer _signer = AWSSigV4Signer(
credentialsProvider: credentialsProvider,
);

@override
Future<AWSHttpRequest> createRequest() async {
Future<AWSBaseHttpRequest> createRequest() async {
final baseRequest = await super.createRequest();
final scope = AWSCredentialScope(
region: _config.region,
Expand All @@ -197,10 +189,7 @@ final class DefaultRemoteLoggingConstraintProvider
credentialScope: scope,
);

final newRequest =
AWSHttpRequest(method: signedRequest.method, uri: signedRequest.uri);

return newRequest;
return signedRequest;
}
}

Expand Down
68 changes: 27 additions & 41 deletions packages/logging_cloudwatch/aws_logging_cloudwatch/test/remote_constraint_provider_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@
import 'dart:convert';

import 'package:amplify_core/amplify_core.dart';
import 'package:aws_common/testing.dart';
import 'package:aws_logging_cloudwatch/aws_logging_cloudwatch.dart';
import 'package:aws_logging_cloudwatch/src/file_storage/file_storage.dart'
if (dart.library.io) 'package:aws_logging_cloudwatch/src/file_storage/file_storage_vm.dart'
if (dart.library.html) 'package:aws_logging_cloudwatch/src/file_storage/file_storage_web.dart';
// import 'package:aws_logging_cloudwatch/src/file_storage/file_storage_vm.dart';
import 'package:aws_logging_cloudwatch/src/file_storage/file_storage.dart';
import 'package:mocktail/mocktail.dart';
import 'package:test/test.dart';

Expand All @@ -31,24 +29,15 @@ const sampleJson = '''
}
''';

class MockFileStorage extends Mock implements FileStorageImpl {
MockFileStorage();

class MockFileStorage extends Mock implements FileStorage {
@override
Future<void> save(String fileName, String content) async {}
}

class MockAWSHttpClient extends Mock implements AWSHttpClient {}

class MockAWSHttpOperation extends Mock
implements AWSHttpOperation<AWSBaseHttpResponse> {}

class MockAWSCredentialsProvider extends Mock
implements AWSCredentialsProvider {}

class PathProvider extends Mock implements AppPathProvider {
PathProvider();

class PathProvider implements AppPathProvider {
@override
Future<String> getApplicationSupportPath() async {
return '';
Expand All @@ -68,12 +57,19 @@ final fakeRequest = AWSHttpRequest(
);

void main() {
test('LoggingConstraint', () {
final sampleJsonMap = jsonDecode(sampleJson) as Map<String, dynamic>;
final loggingConstraint = LoggingConstraint.fromJson(sampleJsonMap);
expect(
loggingConstraint.toJson(),
sampleJsonMap,
);
});

group('RemoteLoggingConstraintProvider', () {
late BaseRemoteLoggingConstraintProvider provider;
late FileStorageImpl mockFileStorage;
late FileStorage mockFileStorage;
late MockAWSHttpClient mockAWSHttpClient;
late MockAWSCredentialsProvider mockCredentialsProvider;
late MockAWSHttpOperation mockOperation;

const sampleJson = '''
{
Expand All @@ -96,25 +92,9 @@ void main() {

setUp(() {
mockFileStorage = MockFileStorage();
mockAWSHttpClient = MockAWSHttpClient();
mockCredentialsProvider = MockAWSCredentialsProvider();
mockOperation = MockAWSHttpOperation();

registerFallbackValue(fakeRequest);

// mock the response from the endpoint
when(() => mockOperation.response).thenAnswer((_) async {
return AWSHttpResponse(
statusCode: 200,
body: utf8.encode(sampleJson),
);
});

// mock the call to createRequest
when(() => mockAWSHttpClient.send(any())).thenAnswer((_) {
return mockOperation;
});

when(() => mockFileStorage.load(any()))
.thenAnswer((_) async => Future.value(sampleJson));

Expand All @@ -124,13 +104,19 @@ void main() {
endpoint: 'https://example.com',
region: 'us-west-2',
),
credentialsProvider: mockCredentialsProvider,
fileStorage: mockFileStorage,
awsHttpClient: mockAWSHttpClient,
);
});

test('initializes _loggingConstraint from endpoint', () async {
mockAWSHttpClient = MockAWSHttpClient((request, _) {
return AWSHttpResponse(
statusCode: 200,
body: utf8.encode(sampleJson),
);
});

await Future<void>.delayed(const Duration(seconds: 3));

// Verify that _loggingConstraint exists
Expand All @@ -143,7 +129,7 @@ void main() {
test(
'fetches _loggingConstraint from local storage and returns null if there are no constraints in local storage',
() async {
when(() => mockOperation.response).thenAnswer((_) async {
mockAWSHttpClient = MockAWSHttpClient((request, _) {
return AWSHttpResponse(
statusCode: 400,
body: utf8.encode('NO RESPONSE'),
Expand All @@ -154,14 +140,14 @@ void main() {
when(() => mockFileStorage.load(any()))
.thenAnswer((_) async => Future.value(null));

await Future<void>.delayed(const Duration(seconds: 3));
await provider.ready;

// Verify that _loggingConstraint is set
expect(provider.loggingConstraint, equals(null));
});

test('uses local storage if endpoint fails', () async {
when(() => mockOperation.response).thenAnswer((_) async {
mockAWSHttpClient = MockAWSHttpClient((request, _) {
return AWSHttpResponse(
statusCode: 400,
body: utf8.encode('NO RESPONSE'),
Expand All @@ -171,7 +157,7 @@ void main() {
when(() => mockFileStorage.load(any()))
.thenAnswer((_) async => Future.value(sampleJson));

await Future<void>.delayed(const Duration(seconds: 3));
await provider.ready;

// Verify that _loggingConstraint uses local storage
expect(
Expand Down Expand Up @@ -201,14 +187,14 @@ void main() {
}
''';

when(() => mockOperation.response).thenAnswer((_) async {
mockAWSHttpClient = MockAWSHttpClient((request, _) {
return AWSHttpResponse(
statusCode: 200,
body: utf8.encode(updatedJson),
);
});

await Future<void>.delayed(const Duration(seconds: 3));
await provider.ready;

// Verify that _loggingConstraint got updated
expect(
Expand Down

0 comments on commit bc635d8

Please sign in to comment.