Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
7 changes: 7 additions & 0 deletions Source/common/SNTConfigBundle.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,11 @@
- (void)fullSyncInterval:(void (^)(NSUInteger))block;
- (void)pushNotificationsFullSyncInterval:(void (^)(NSUInteger))block;

///
/// When set, signals the daemon to clear persisted sync state before
/// applying the rest of the bundle. Set only by PostflightConfigBundle
/// when the in-flight sync was Clean or CleanAll.
///
- (void)clearSyncStateBeforeApply:(void (^)(BOOL))block;

@end
9 changes: 9 additions & 0 deletions Source/common/SNTConfigBundle.mm
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ @interface SNTConfigBundle ()
@property NSArray<SNTCELFallbackRule*>* celFallbackRules;
@property NSNumber* fullSyncInterval;
@property NSNumber* pushNotificationsFullSyncInterval;
@property NSNumber* clearSyncStateBeforeApply;
@end

@implementation SNTConfigBundle
Expand Down Expand Up @@ -92,6 +93,7 @@ - (void)encodeWithCoder:(NSCoder*)coder {
ENCODE(coder, celFallbackRules);
ENCODE(coder, fullSyncInterval);
ENCODE(coder, pushNotificationsFullSyncInterval);
ENCODE(coder, clearSyncStateBeforeApply);
}

- (instancetype)initWithCoder:(NSCoder*)decoder {
Expand Down Expand Up @@ -128,6 +130,7 @@ - (instancetype)initWithCoder:(NSCoder*)decoder {
DECODE_ARRAY(decoder, celFallbackRules, SNTCELFallbackRule);
DECODE(decoder, fullSyncInterval, NSNumber);
DECODE(decoder, pushNotificationsFullSyncInterval, NSNumber);
DECODE(decoder, clearSyncStateBeforeApply, NSNumber);
}
return self;
}
Expand Down Expand Up @@ -317,4 +320,10 @@ - (void)pushNotificationsFullSyncInterval:(void (^)(NSUInteger))block {
}
}

- (void)clearSyncStateBeforeApply:(void (^)(BOOL))block {
if (self.clearSyncStateBeforeApply) {
block([self.clearSyncStateBeforeApply boolValue]);
}
}

@end
43 changes: 43 additions & 0 deletions Source/common/SNTConfigBundleTest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ @interface SNTConfigBundle (Testing)
@property NSArray<SNTCELFallbackRule*>* celFallbackRules;
@property NSNumber* fullSyncInterval;
@property NSNumber* pushNotificationsFullSyncInterval;
@property NSNumber* clearSyncStateBeforeApply;
@end

@interface SNTConfigBundleTest : XCTestCase
Expand Down Expand Up @@ -372,4 +373,46 @@ - (void)testGettersWithoutValues {
}];
}

- (void)testClearSyncStateBeforeApplyRoundTrips {
// When set to YES, the round-trip preserves the value and the accessor block fires with YES.
SNTConfigBundle* set = [[SNTConfigBundle alloc] init];
set.clearSyncStateBeforeApply = @(YES);

NSError* error = nil;
NSData* setData = [NSKeyedArchiver archivedDataWithRootObject:set
requiringSecureCoding:YES
error:&error];
XCTAssertNil(error);
SNTConfigBundle* setDecoded = [NSKeyedUnarchiver unarchivedObjectOfClass:[SNTConfigBundle class]
fromData:setData
error:&error];
XCTAssertNil(error);

__block BOOL setFired = NO;
__block BOOL setValue = NO;
[setDecoded clearSyncStateBeforeApply:^(BOOL v) {
setFired = YES;
setValue = v;
}];
XCTAssertTrue(setFired);
XCTAssertEqual(setValue, YES);

// When left unset, the round-trip preserves the unset state and the accessor block does not fire.
SNTConfigBundle* unset = [[SNTConfigBundle alloc] init];
NSData* unsetData = [NSKeyedArchiver archivedDataWithRootObject:unset
requiringSecureCoding:YES
error:&error];
XCTAssertNil(error);
SNTConfigBundle* unsetDecoded = [NSKeyedUnarchiver unarchivedObjectOfClass:[SNTConfigBundle class]
fromData:unsetData
error:&error];
XCTAssertNil(error);

__block BOOL unsetFired = NO;
[unsetDecoded clearSyncStateBeforeApply:^(BOOL v) {
unsetFired = YES;
}];
XCTAssertFalse(unsetFired);
}

@end
14 changes: 14 additions & 0 deletions Source/common/SNTConfigurator.h
Original file line number Diff line number Diff line change
Expand Up @@ -1152,6 +1152,20 @@ extern NSString* _Nonnull const kEnableMenuItemUserOverride;
///
- (void)clearSyncState;

///
/// Buffer a series of sync-state mutations into a single atomic commit.
///
/// Inside the block, calls to `updateSyncStateForKey:value:` and
/// `clearSyncState` write only to an in-memory working dictionary —
/// they do not fire KVO and do not write to disk. When the block
/// returns, the working dictionary is assigned to `syncState` (single
/// KVO fire) and saved to disk (single `writeToFile:atomically:YES`).
///
/// Must be called outside any other batch on this configurator.
/// Asserts in debug; logs and skips in release.
///
- (void)performSyncStateBatch:(nonnull void(NS_NOESCAPE ^)(void))block;
Comment thread
sharvilshah marked this conversation as resolved.
Outdated

///
/// Validate the configuration profile.
///
Expand Down
46 changes: 42 additions & 4 deletions Source/common/SNTConfigurator.mm
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ @interface SNTConfigurator ()
@property(atomic) NSMutableDictionary* configState;
@property(atomic) NSDictionary* state;

/// Working dictionary for an in-progress `performSyncStateBatch:` transaction.
/// Non-nil only while inside the batch's block. Always accessed on the main
/// thread, matching the convention enforced by `updateSyncStateForKey:value:`.
@property(nonatomic) NSMutableDictionary* batchedSyncState;

@property(readonly, nonatomic) NSString* syncStateFilePath;
@property(readonly, nonatomic) NSString* stateFilePath;

Expand Down Expand Up @@ -1960,6 +1965,10 @@ - (NSDictionary*)extraMetricLabels {
///
- (void)updateSyncStateForKey:(NSString*)key value:(id)value {
void (^block)(void) = ^{
if (self.batchedSyncState) {
self.batchedSyncState[key] = value;
return;
}
NSMutableDictionary* syncState = self.syncState.mutableCopy;
syncState[key] = value;
self.syncState = syncState;
Expand All @@ -1974,6 +1983,26 @@ - (void)updateSyncStateForKey:(NSString*)key value:(id)value {
}
}

- (void)performSyncStateBatch:(void(NS_NOESCAPE ^)(void))block {
void (^run)(void) = ^{
if (self.batchedSyncState != nil) {
NSAssert(NO, @"Sync-state batches do not nest");
LOGE(@"Sync-state batches do not nest; skipping nested batch");
return;
}
self.batchedSyncState = self.syncState.mutableCopy;
block();
self.syncState = self.batchedSyncState;
self.batchedSyncState = nil;
[self saveSyncStateToDisk];
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
if ([NSThread isMainThread]) {
run();
} else {
dispatch_sync(dispatch_get_main_queue(), run);
}
}

///
/// Read the saved syncState.
///
Expand Down Expand Up @@ -2060,10 +2089,19 @@ - (void)saveSyncStateToDisk {
}

- (void)clearSyncState {
self.syncState = [NSMutableDictionary dictionary];
// TODO: Start a timer to flush the state to disk. On startup, Santa should
// check for the presence of the state file and, if no SyncBaseURL is
// configured, start the timer to clear sync state and flush to disk.
void (^block)(void) = ^{
if (self.batchedSyncState) {
[self.batchedSyncState removeAllObjects];
return;
}
self.syncState = [NSMutableDictionary dictionary];
[[NSFileManager defaultManager] removeItemAtPath:self.syncStateFilePath error:NULL];
};
if ([NSThread isMainThread]) {
block();
} else {
dispatch_sync(dispatch_get_main_queue(), block);
}
}

- (NSArray*)entitlementsPrefixFilter {
Expand Down
109 changes: 109 additions & 0 deletions Source/common/SNTConfiguratorTest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -285,4 +285,113 @@ - (void)testAllowDelegatedSignalsOverride {
XCTAssertFalse(sut.allowDelegatedSignals);
}

#pragma mark - performSyncStateBatch: and clearSyncState tests

- (SNTConfigurator*)configuratorWithEmptySyncStateAtPath:(NSString*)plistPath {
return [[SNTConfigurator alloc] initWithSyncStateFile:plistPath
stateFile:@"/does/not/need/to/exist"
oldStateFile:@"/does/not/need/to/exist"
syncStateAccessAuthorizer:^{
return YES;
}
stateAccessAuthorizer:^BOOL {
return NO;
}];
}

- (void)observeValueForKeyPath:(NSString*)keyPath
ofObject:(id)object
change:(NSDictionary*)change
context:(void*)context {
if (context != NULL) {
NSUInteger* count = (NSUInteger*)context;
(*count)++;
}
}

- (void)testPerformSyncStateBatchCommitsAsSingleKVOFire {
NSString* plistPath = [NSString stringWithFormat:@"%@/batch-kvo.plist", self.testDir];
SNTConfigurator* cfg = [self configuratorWithEmptySyncStateAtPath:plistPath];

__block NSUInteger kvoCount = 0;
[cfg addObserver:self
forKeyPath:@"syncState"
options:NSKeyValueObservingOptionNew
context:&kvoCount];

[cfg performSyncStateBatch:^{
[cfg setSyncServerClientMode:SNTClientModeLockdown];
[cfg setEnableBundles:YES];
[cfg setEnableTransitiveRules:YES];
}];

[cfg removeObserver:self forKeyPath:@"syncState" context:&kvoCount];

XCTAssertEqual(kvoCount, (NSUInteger)1,
@"Expected exactly one KVO fire on syncState across the batch; got %lu",
(unsigned long)kvoCount);
XCTAssertEqual(cfg.clientMode, SNTClientModeLockdown);
XCTAssertTrue(cfg.enableBundles);
XCTAssertTrue(cfg.enableTransitiveRules);

XCTAssertTrue([self.fileMgr removeItemAtPath:plistPath error:nil]);
}

- (void)testPerformSyncStateBatchWithClearAndWritesPersistsOnlyPostClearWrites {
NSString* plistPath = [NSString stringWithFormat:@"%@/batch-clear.plist", self.testDir];
SNTConfigurator* cfg = [self configuratorWithEmptySyncStateAtPath:plistPath];

// Pre-populate stale state via the one-shot path.
[cfg setSyncServerClientMode:SNTClientModeLockdown];
[cfg setEnableBundles:YES];
XCTAssertEqual(cfg.clientMode, SNTClientModeLockdown);
XCTAssertTrue(cfg.enableBundles);

// Inside the batch: clear, then write only one new key.
[cfg performSyncStateBatch:^{
[cfg clearSyncState];
[cfg setSyncServerClientMode:SNTClientModeMonitor];
}];

// ClientMode set to the new value; EnableBundles was cleared and not rewritten.
XCTAssertEqual(cfg.clientMode, SNTClientModeMonitor);
XCTAssertFalse(cfg.enableBundles, @"EnableBundles should be cleared (default NO) after batch");

// On disk: matches in-memory state, no stale keys.
NSDictionary* onDisk = [NSDictionary dictionaryWithContentsOfFile:plistPath];
XCTAssertEqualObjects(onDisk[@"ClientMode"], @(SNTClientModeMonitor));
XCTAssertNil(onDisk[@"EnableBundles"], @"Stale EnableBundles key should be absent from disk");

XCTAssertTrue([self.fileMgr removeItemAtPath:plistPath error:nil]);
}

- (void)testClearSyncStateRemovesDiskFileWhenOutsideBatch {
NSString* plistPath = [NSString stringWithFormat:@"%@/clear-remove.plist", self.testDir];
SNTConfigurator* cfg = [self configuratorWithEmptySyncStateAtPath:plistPath];

// Force a disk write so the file exists.
[cfg setSyncServerClientMode:SNTClientModeLockdown];
XCTAssertTrue([self.fileMgr fileExistsAtPath:plistPath]);

[cfg clearSyncState];

XCTAssertFalse([self.fileMgr fileExistsAtPath:plistPath],
@"clearSyncState must remove sync-state.plist from disk");
XCTAssertEqual(cfg.syncState.count, (NSUInteger)0);
}

- (void)testClearSyncStateIsIdempotentOnMissingFile {
NSString* plistPath = [NSString stringWithFormat:@"%@/clear-missing.plist", self.testDir];
SNTConfigurator* cfg = [self configuratorWithEmptySyncStateAtPath:plistPath];

XCTAssertFalse([self.fileMgr fileExistsAtPath:plistPath]);

// Should be a no-op, no exception, no error.
XCTAssertNoThrow([cfg clearSyncState]);
XCTAssertFalse([self.fileMgr fileExistsAtPath:plistPath]);

// A second call is also a no-op.
XCTAssertNoThrow([cfg clearSyncState]);
}

@end
Loading
Loading