Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 2 additions & 3 deletions modules/ensemble/lib/framework/action.dart
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,8 @@ class NavigateViewGroupAction extends EnsembleAction {
} else if (viewIndex != null) {
final pageGroup = context.findAncestorWidgetOfExactType<PageGroup>();
final menuLen = pageGroup?.menu.menuItems.length ?? 0;
final resolvedIndex = menuLen > 0
? safeViewGroupPayloadIndex(viewIndex, menuLen)
: viewIndex;
final resolvedIndex =
resolveNavigateViewGroupTabIndex(viewIndex, menuLen);
if (payload != null) {
// TODO: this is wrong. Can't mutate the scope like this
scopeManager.dataContext.addDataContext(payload);
Expand Down
24 changes: 19 additions & 5 deletions modules/ensemble/lib/framework/data_context.dart
Original file line number Diff line number Diff line change
Expand Up @@ -746,6 +746,20 @@ class RemoteConfigInvokable with Invokable {
List<String> ensembleStorageClearDispatchKeys(Iterable<String> allKeys) =>
allKeys.where((key) => !key.startsWith('enc_')).toList();

/// Runs [dispatch] for each [keys] entry only after [clearFuture] completes.
@visibleForTesting
Future<void> runEnsembleStorageClearDispatches(
Future<void> clearFuture,
Iterable<String> keys,
void Function(String key) dispatch,
) {
return clearFuture.whenComplete(() {
for (final key in keys) {
dispatch(key);
}
});
}

/// Singleton handling user storage
class EnsembleStorage with Invokable {
static final EnsembleStorage _instance = EnsembleStorage._internal();
Expand Down Expand Up @@ -803,11 +817,11 @@ class EnsembleStorage with Invokable {
// clearPublicStorage is async; binding listeners re-evaluate by reading
// GetStorage. Dispatch only after removal completes so eval() does not
// observe stale persisted values (see PR discussion on #2227).
unawaited(StorageManager().clearPublicStorage().whenComplete(() {
for (final key in keys) {
ScreenController().dispatchStorageChanges(context, key, null);
}
}));
unawaited(runEnsembleStorageClearDispatches(
StorageManager().clearPublicStorage(),
keys,
(key) => ScreenController().dispatchStorageChanges(context, key, null),
));
}

@override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,16 @@ class CdnDefinitionProvider extends DefinitionProvider {
}
}

/// After a stale CDN fetch updates [_artifactCache], either apply changes
/// immediately (artifact refresh on + app initialized) or defer to resume.
Future<void> _applyStaleRefreshOutcome() async {
if (isArtifactRefreshEnabled() && Ensemble().getConfig() != null) {
await _handlePendingUpdate();
} else {
_hasPendingUpdate = true;
}
}

/// Handles pending CDN updates when app resumes.
/// CRITICAL: Must update appBundle and translations BEFORE firing refresh event
/// to ensure screens rebuild with the new resources (fixes race condition).
Expand Down Expand Up @@ -513,16 +523,7 @@ class CdnDefinitionProvider extends DefinitionProvider {
// Save to persistent cache
await _saveCachedState(jsonString);

// If artifact refresh is enabled and app is already initialized,
// immediately update appBundle and fire refresh event.
// This handles the cold start scenario where background refresh
// completes after initial render.
if (isArtifactRefreshEnabled() && Ensemble().getConfig() != null) {
await _handlePendingUpdate();
} else {
// Mark for later refresh on next resume
_hasPendingUpdate = true;
}
await _applyStaleRefreshOutcome();
} catch (e) {
if (kDebugMode) {
debugPrint('⚠️ CDN Provider: Refresh failed: $e');
Expand Down Expand Up @@ -595,7 +596,8 @@ class CdnDefinitionProvider extends DefinitionProvider {
return true;
}

final shouldFetch = _isIncomingNewer(remoteLastUpdate, _lastUpdatedAt);
final shouldFetch =
isCdnManifestIncomingNewer(remoteLastUpdate, _lastUpdatedAt);

_lastUpdatedAt = remoteLastUpdate;

Expand Down Expand Up @@ -854,9 +856,6 @@ class CdnDefinitionProvider extends DefinitionProvider {
// Helpers
// --------------------------------------------------------

static bool _isIncomingNewer(int? incoming, int? current) =>
incoming != null && (current == null || incoming > current);

static Map<String, dynamic>? _asMap(dynamic value) {
if (value is Map<String, dynamic>) return value;
if (value is Map) return Map<String, dynamic>.from(value);
Expand Down Expand Up @@ -925,6 +924,10 @@ class CdnDefinitionProvider extends DefinitionProvider {
@visibleForTesting
Future<void> handlePendingUpdateForTesting() => _handlePendingUpdate();

@visibleForTesting
Future<void> applyStaleRefreshOutcomeForTesting() =>
_applyStaleRefreshOutcome();

@visibleForTesting
bool get hasPendingUpdateForTesting => _hasPendingUpdate;

Expand Down Expand Up @@ -961,3 +964,8 @@ class CdnDefinitionProvider extends DefinitionProvider {
}
}
}

/// Whether CDN [lastUpdateTime.json] indicates a manifest fetch is needed.
@visibleForTesting
bool isCdnManifestIncomingNewer(int? incoming, int? current) =>
incoming != null && (current == null || incoming > current);
7 changes: 7 additions & 0 deletions modules/ensemble/lib/framework/view/page_group.dart
Original file line number Diff line number Diff line change
Expand Up @@ -729,6 +729,13 @@ int safeViewGroupPayloadIndex(int index, int payloadLength) {
return index;
}

/// Resolves [viewIndex] for [NavigateViewGroupAction] before [PageController.jumpToPage].
/// When the ancestor [PageGroup] has menu items, clamp to the tab range; otherwise pass
/// through (e.g. nested view groups without a menu).
@visibleForTesting
int resolveNavigateViewGroupTabIndex(int viewIndex, int menuLen) =>
menuLen > 0 ? safeViewGroupPayloadIndex(viewIndex, menuLen) : viewIndex;

class ViewGroupNotifier extends ChangeNotifier {
int _viewIndex = 0;
Map<String, dynamic>? _payload;
Expand Down
24 changes: 24 additions & 0 deletions modules/ensemble/test/cdn_manifest_freshness_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import 'package:ensemble/framework/definition_providers/cdn_provider.dart';
import 'package:flutter_test/flutter_test.dart';

void main() {
group('isCdnManifestIncomingNewer', () {
test('fetches when remote timestamp is newer than cached', () {
expect(isCdnManifestIncomingNewer(200, 100), isTrue);
});

test('skips fetch when remote is same or older', () {
expect(isCdnManifestIncomingNewer(100, 100), isFalse);
expect(isCdnManifestIncomingNewer(50, 100), isFalse);
});

test('fetches when there is no cached timestamp', () {
expect(isCdnManifestIncomingNewer(1, null), isTrue);
});

test('does not fetch when remote timestamp is missing', () {
expect(isCdnManifestIncomingNewer(null, 100), isFalse);
expect(isCdnManifestIncomingNewer(null, null), isFalse);
});
});
}
49 changes: 49 additions & 0 deletions modules/ensemble/test/cdn_provider_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,55 @@ void main() {
});
});

group('CDN stale refresh outcome', () {
test('applies updates immediately when artifact refresh is enabled',
() async {
final provider = CdnDefinitionProvider('test-app');
final config = EnsembleConfig(definitionProvider: provider);
Ensemble().setEnsembleConfig(config);

await provider.applyRuntimeManifestForTesting(
_manifestWithArtifactRefresh(_manifestWithResourceVersion('v1')),
);
await config.updateAppBundle();

provider.rebuildManifestCacheForTesting(
_manifestWithArtifactRefresh(_manifestWithResourceVersion('v2')),
);

await provider.applyStaleRefreshOutcomeForTesting();

expect(provider.hasPendingUpdateForTesting, isFalse);
expect(
config.getResources()?[ResourceArtifactEntry.Scripts.name]['version'],
'v2',
);
});

test('defers updates when artifact refresh is disabled', () async {
final provider = CdnDefinitionProvider('test-app');
final config = EnsembleConfig(definitionProvider: provider);
Ensemble().setEnsembleConfig(config);

await provider.applyRuntimeManifestForTesting(
_manifestWithResourceVersion('v1'),
);
await config.updateAppBundle();

provider.rebuildManifestCacheForTesting(
_manifestWithResourceVersion('v2'),
);

await provider.applyStaleRefreshOutcomeForTesting();

expect(provider.hasPendingUpdateForTesting, isTrue);
expect(
config.getResources()?[ResourceArtifactEntry.Scripts.name]['version'],
'v1',
);
});
});

group('CDN pending update ordering', () {
test('handlePendingUpdate syncs app bundle from CDN cache and fires refresh',
() async {
Expand Down
40 changes: 40 additions & 0 deletions modules/ensemble/test/ensemble_storage_clear_dispatch_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import 'package:ensemble/framework/data_context.dart';
import 'package:flutter_test/flutter_test.dart';

void main() {
group('runEnsembleStorageClearDispatches', () {
test('dispatches only after clear future completes', () async {
final dispatched = <String>[];
var storageCleared = false;

final clearFuture = Future<void>(() async {
await Future<void>.delayed(const Duration(milliseconds: 10));
storageCleared = true;
});

final done = runEnsembleStorageClearDispatches(
clearFuture,
['session', 'theme'],
dispatched.add,
);

expect(dispatched, isEmpty);
await done;
expect(storageCleared, isTrue);
expect(dispatched, ['session', 'theme']);
});

test('still dispatches when clear future completes with error', () async {
final dispatched = <String>[];

final done = runEnsembleStorageClearDispatches(
Future<void>.error(StateError('clear failed')),
['a'],
dispatched.add,
);

await expectLater(done, throwsStateError);
expect(dispatched, ['a']);
});
});
}
12 changes: 12 additions & 0 deletions modules/ensemble/test/safe_view_group_payload_index_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,18 @@ import 'package:ensemble/framework/view/page_group.dart';
import 'package:flutter_test/flutter_test.dart';

void main() {
group('resolveNavigateViewGroupTabIndex', () {
test('clamps when menu has items', () {
expect(resolveNavigateViewGroupTabIndex(99, 3), 2);
expect(resolveNavigateViewGroupTabIndex(-1, 2), 0);
});

test('passes through when menu length is zero', () {
expect(resolveNavigateViewGroupTabIndex(5, 0), 5);
expect(resolveNavigateViewGroupTabIndex(-1, 0), -1);
});
});

group('safeViewGroupPayloadIndex', () {
test('clamps to valid range for positive lengths', () {
expect(safeViewGroupPayloadIndex(-1, 3), 0);
Expand Down
Loading