unsignedapps / Vexil

Vexil (named for Vexillology) is a Swift package for managing feature flags (also called feature toggles) in a flexible, multi-provider way.
https://vexil.unsignedapps.com
MIT License
115 stars 12 forks source link

Make Flag Sendable where its Value is Sendable #107

Closed huwr closed 1 year ago

huwr commented 1 year ago

📒 Description

Previously, Flag was not Sendable. We can make it as such fairly easily if it’s carrying a Sendable Value (like a Bool or Int).

🔍 Detailed Design

Mainly, we've added conditional conformance to Sendable on Flag where its associated Value is also Sendable:

extension Flag: Sendable where Value: Sendable { }

That was easy. CodingKeyStrategy and FlagInfo then needed to be marked as Sendable too. Since they're value types that was no problem.

The only concern is Decorator class. Since it’s a class, we must manage the thread safety ourselves. We can use an NSLock for that, which works, but it’s perhaps a little manual and prone to developer-error — and perhaps even prone to deadlock.

That also requires making the Decorator class final.

(Another option to that is to make it an actor, however that introduces a bit histrionics and drama of full async/await adoption — there may be other options too.)

📓 Documentation Plan

I don't believe this requires an update to any documentation.

🗳 Test Plan

Not done yet. Perhaps I should add tests for Decorator to check for thread safety.

🧯 Source Impact

There should be no impact on any existing code.

✅ Checklist

KeithBauerANZ commented 1 year ago

106 not only conflicts with this, it makes it much harder to make this change, since Flag has mutable properties.

huwr commented 1 year ago

106 not only conflicts with this, it makes it much harder to make this change, since Flag has mutable properties.

That's okay. This was a short experiment this morning I didn't want to lose. It'll be good for Flag to be Sendable later on.

sonarcloud[bot] commented 1 year ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

huwr commented 1 year ago

I closed this PR because it was just a bit of a brain-wave, and I didn't realise you were working on #106. You're right it makes these changes harder, though. I'm not sure if using an NSLock in there would be the right approach anyway, and perhaps isn't at all any more.