Inject the previews preference instead of reading shared defaults

CI caught a real problem this introduced. `NotificationServiceTests`
already asserted the full-preview title and body, and both it and the new
redaction tests read `NotificationPrivacySettings` from
`UserDefaults.standard` in the same process. Whichever ran second
depended on the other's cleanup, so it passed locally and failed in CI:

  XCTAssertEqual failed: ("🔒 new dm") is not equal to ("🔒 DM from Alice")

Fixed at the source rather than by ordering or serialization.
`NotificationService` now takes a `hidePreviewsProvider`, defaulting to
the real preference, so each test states which behavior it asserts. The
pre-existing test asks for previews shown and gains a redacted
counterpart; the redaction tests no longer touch the shared store.

`NotificationPrivacySettings` also gained store-injecting accessors so
the default-value and round-trip assertions can use an isolated suite
rather than mutating preferences other tests read.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
jack
2026-07-26 18:16:45 +02:00
co-authored by Claude Opus 5
parent dc7475a0ba
commit 89c5a5d999
4 changed files with 128 additions and 76 deletions
@@ -22,13 +22,23 @@ enum NotificationPrivacySettings {
private static let hidePreviewsKey = "notifications.hideMessagePreviews"
static var hideMessagePreviews: Bool {
get { UserDefaults.standard.object(forKey: hidePreviewsKey) as? Bool ?? true }
set { UserDefaults.standard.set(newValue, forKey: hidePreviewsKey) }
get { hideMessagePreviews(in: .standard) }
set { setHideMessagePreviews(newValue, in: .standard) }
}
/// Store-injecting forms, so tests can assert the default and both settings
/// without touching the shared preferences other tests read.
static func hideMessagePreviews(in defaults: UserDefaults) -> Bool {
defaults.object(forKey: hidePreviewsKey) as? Bool ?? true
}
static func setHideMessagePreviews(_ hide: Bool, in defaults: UserDefaults) {
defaults.set(hide, forKey: hidePreviewsKey)
}
/// Panic-wipe hook. Removing the key restores the hidden default, so a
/// wiped device cannot come back quieter than a fresh install.
static func reset() {
UserDefaults.standard.removeObject(forKey: hidePreviewsKey)
/// wiped device cannot come back louder than a fresh install.
static func reset(in defaults: UserDefaults = .standard) {
defaults.removeObject(forKey: hidePreviewsKey)
}
}
+11 -2
View File
@@ -114,8 +114,14 @@ final class NotificationService {
}
/// Whether delivered alerts must withhold sender, content, and geohash.
///
/// Injected rather than read from the preference directly so tests state
/// which behavior they are asserting instead of inheriting whatever the
/// shared preference happens to hold when they run.
private let hidePreviewsProvider: () -> Bool
private var hidePreviews: Bool {
NotificationPrivacySettings.hideMessagePreviews
hidePreviewsProvider()
}
private let isRunningTestsProvider: () -> Bool
@@ -129,6 +135,7 @@ final class NotificationService {
}
private init() {
self.hidePreviewsProvider = { NotificationPrivacySettings.hideMessagePreviews }
self.isRunningTestsProvider = {
let env = ProcessInfo.processInfo.environment
return NSClassFromString("XCTestCase") != nil ||
@@ -153,12 +160,14 @@ final class NotificationService {
isRunningTestsProvider: @escaping () -> Bool,
authorizer: NotificationAuthorizing,
requestDeliverer: NotificationRequestDelivering,
categoryRegistrar: NotificationCategoryRegistering = NoopNotificationCategoryRegistrar()
categoryRegistrar: NotificationCategoryRegistering = NoopNotificationCategoryRegistrar(),
hidePreviewsProvider: @escaping () -> Bool = { NotificationPrivacySettings.hideMessagePreviews }
) {
self.isRunningTestsProvider = isRunningTestsProvider
self.authorizer = authorizer
self.requestDeliverer = requestDeliverer
self.categoryRegistrar = categoryRegistrar
self.hidePreviewsProvider = hidePreviewsProvider
}
func requestAuthorization() {
@@ -9,9 +9,10 @@ import UserNotifications
/// a table narrated conversations to anyone looking at it. These cover the
/// redaction that is now the default.
///
/// Serialized because the preference lives in `UserDefaults.standard`; parallel
/// cases would race each other's saves.
@Suite(.serialized)
/// The preference is injected rather than written to shared preferences. These
/// run in the same process as `NotificationServiceTests`, and mutating the
/// global store made whichever ran second depend on the other's cleanup which
/// passed locally and failed in CI.
struct NotificationRedactionTests {
private final class RecordingDeliverer: NotificationRequestDelivering {
var requests: [UNNotificationRequest] = []
@@ -30,92 +31,100 @@ struct NotificationRedactionTests {
}
}
/// Runs `body` with the preference forced to `hidePreviews`, restoring
/// whatever the environment had afterwards.
private func withHidePreviews(
_ hidePreviews: Bool,
_ body: (NotificationService, RecordingDeliverer) throws -> Void
) rethrows {
let original = NotificationPrivacySettings.hideMessagePreviews
defer { NotificationPrivacySettings.hideMessagePreviews = original }
NotificationPrivacySettings.hideMessagePreviews = hidePreviews
private func makeService(
hidePreviews: Bool
) -> (NotificationService, RecordingDeliverer) {
let deliverer = RecordingDeliverer()
let service = NotificationService(
isRunningTestsProvider: { false },
authorizer: StubAuthorizer(),
requestDeliverer: deliverer
requestDeliverer: deliverer,
hidePreviewsProvider: { hidePreviews }
)
try body(service, deliverer)
return (service, deliverer)
}
private func isolatedDefaults() -> UserDefaults {
UserDefaults(suiteName: "bitchat.tests.notifications.\(UUID().uuidString)")!
}
@Test func previewsAreHiddenByDefault() {
// A fresh install must start quiet rather than opt-in quiet.
UserDefaults.standard.removeObject(forKey: "notifications.hideMessagePreviews")
#expect(NotificationPrivacySettings.hideMessagePreviews)
#expect(NotificationPrivacySettings.hideMessagePreviews(in: isolatedDefaults()))
}
@Test func redactedDirectMessageWithholdsSenderAndBody() {
withHidePreviews(true) { service, deliverer in
service.sendPrivateMessageNotification(
from: "alice",
message: "meet at the north gate",
peerID: PeerID(str: "00112233445566ff")
)
@Test func theSettingRoundTrips() {
let defaults = isolatedDefaults()
let content = try! #require(deliverer.requests.first).content
#expect(!content.title.contains("alice"))
#expect(!content.body.contains("north gate"))
#expect(!content.title.isEmpty)
// Still routable: userInfo is never rendered on the lock screen.
#expect(content.userInfo["peerID"] as? String == "00112233445566ff")
}
NotificationPrivacySettings.setHideMessagePreviews(false, in: defaults)
#expect(!NotificationPrivacySettings.hideMessagePreviews(in: defaults))
// Panic wipe restores the safe default rather than the last choice.
NotificationPrivacySettings.reset(in: defaults)
#expect(NotificationPrivacySettings.hideMessagePreviews(in: defaults))
}
@Test func redactedMentionWithholdsSenderAndBody() {
withHidePreviews(true) { service, deliverer in
service.sendMentionNotification(from: "bob", message: "regroup now")
@Test func redactedDirectMessageWithholdsSenderAndBody() throws {
let (service, deliverer) = makeService(hidePreviews: true)
let content = try! #require(deliverer.requests.first).content
#expect(!content.title.contains("bob"))
#expect(!content.body.contains("regroup"))
}
service.sendPrivateMessageNotification(
from: "alice",
message: "meet at the north gate",
peerID: PeerID(str: "00112233445566ff")
)
let content = try #require(deliverer.requests.first).content
#expect(!content.title.contains("alice"))
#expect(!content.body.contains("north gate"))
#expect(!content.title.isEmpty)
// Still routable: userInfo is never rendered on the lock screen.
#expect(content.userInfo["peerID"] as? String == "00112233445566ff")
}
@Test func redactedGeohashActivityWithholdsTheGeohash() {
withHidePreviews(true) { service, deliverer in
service.sendGeohashActivityNotification(
geohash: "u4pruyd",
bodyPreview: "someone said something"
)
@Test func redactedMentionWithholdsSenderAndBody() throws {
let (service, deliverer) = makeService(hidePreviews: true)
let content = try! #require(deliverer.requests.first).content
#expect(!content.title.contains("u4pruyd"))
#expect(!content.body.contains("someone said"))
// The deep link still carries it: tapping must land in the channel.
#expect(content.userInfo["deeplink"] as? String == "bitchat://geohash/u4pruyd")
}
service.sendMentionNotification(from: "bob", message: "regroup now")
let content = try #require(deliverer.requests.first).content
#expect(!content.title.contains("bob"))
#expect(!content.body.contains("regroup"))
}
@Test func redactedGeohashActivityWithholdsTheGeohash() throws {
let (service, deliverer) = makeService(hidePreviews: true)
service.sendGeohashActivityNotification(
geohash: "u4pruyd",
bodyPreview: "someone said something"
)
let content = try #require(deliverer.requests.first).content
#expect(!content.title.contains("u4pruyd"))
#expect(!content.body.contains("someone said"))
// The deep link still carries it: tapping must land in the channel.
#expect(content.userInfo["deeplink"] as? String == "bitchat://geohash/u4pruyd")
}
@Test func previewsShownWhenTheSettingIsOff() {
withHidePreviews(false) { service, deliverer in
service.sendPrivateMessageNotification(
from: "alice",
message: "meet at the north gate",
peerID: PeerID(str: "00112233445566ff")
)
service.sendGeohashActivityNotification(
geohash: "u4pruyd",
bodyPreview: "someone said something"
)
let (service, deliverer) = makeService(hidePreviews: false)
#expect(deliverer.requests.count == 2)
let dm = deliverer.requests[0].content
#expect(dm.title.contains("alice"))
#expect(dm.body == "meet at the north gate")
let geo = deliverer.requests[1].content
#expect(geo.title.contains("u4pruyd"))
#expect(geo.body == "someone said something")
}
service.sendPrivateMessageNotification(
from: "alice",
message: "meet at the north gate",
peerID: PeerID(str: "00112233445566ff")
)
service.sendGeohashActivityNotification(
geohash: "u4pruyd",
bodyPreview: "someone said something"
)
#expect(deliverer.requests.count == 2)
let dm = deliverer.requests[0].content
#expect(dm.title.contains("alice"))
#expect(dm.body == "meet at the north gate")
let geo = deliverer.requests[1].content
#expect(geo.title.contains("u4pruyd"))
#expect(geo.body == "someone said something")
}
}
@@ -56,12 +56,15 @@ final class NotificationServiceTests: XCTestCase {
XCTAssertNil(request?.trigger)
}
/// Previews shown: the opt-in behavior. Stated explicitly rather than
/// inherited from the shared preference, which now defaults to hidden.
func test_sendPrivateMessageNotification_populatesPeerMetadata() {
let deliverer = RecordingNotificationRequestDeliverer()
let service = NotificationService(
isRunningTestsProvider: { false },
authorizer: RecordingNotificationAuthorizer(),
requestDeliverer: deliverer
requestDeliverer: deliverer,
hidePreviewsProvider: { false }
)
let peerID = PeerID(str: "deadbeefdeadbeef")
@@ -74,6 +77,27 @@ final class NotificationServiceTests: XCTestCase {
XCTAssertEqual(request?.content.userInfo["senderName"] as? String, "Alice")
}
/// Previews hidden: the default. The routing payload has to survive
/// redaction, or tapping the alert would not open the conversation.
func test_sendPrivateMessageNotification_withPreviewsHidden_keepsRoutingButDropsContent() {
let deliverer = RecordingNotificationRequestDeliverer()
let service = NotificationService(
isRunningTestsProvider: { false },
authorizer: RecordingNotificationAuthorizer(),
requestDeliverer: deliverer,
hidePreviewsProvider: { true }
)
let peerID = PeerID(str: "deadbeefdeadbeef")
service.sendPrivateMessageNotification(from: "Alice", message: "hi", peerID: peerID)
let request = deliverer.requests.singleValue
XCTAssertFalse(request?.content.title.contains("Alice") ?? true)
XCTAssertFalse(request?.content.body.contains("hi") ?? true)
XCTAssertFalse(request?.content.title.isEmpty ?? true)
XCTAssertEqual(request?.content.userInfo["peerID"] as? String, peerID.id)
}
func test_wrapperNotifications_setExpectedIdentifiersAndDeepLinks() {
let deliverer = RecordingNotificationRequestDeliverer()
let service = NotificationService(