FileDataStorage doesn't store file with correct name

[matthiasgebhardt]

Following both methods don't consider the given key-Parameter for creating the file-name:
public func store(data: Data, forKey key: String) -> Bool
and
public func data(forKey key: String) -> Data?

Instead it looks like an incomplete implementation, where each method defines the file-path by setting a static file-name: "key.md5"

let fileUrl = baseUrl.appendingPathComponent("key.md5")

Discussion
As the current implementation suggests, it should be used a MD5-hashed key for the file-name. The Question is: do we want to implement a md5-hash-function directly or do we want to use it from bytes. The later solution unfortunately would make the RemoteSettings pod dependent of the bytes pod.

Whats your thoughts?

[brototyp]

Oh, that's hilarious! Great catch there! I just know that there is an md5 implementation in bytes. Not sure if we want to reuse this. An alternative could be to change the key from String to UUID.

[matthiasgebhardt]

Not sure if this is helpful, but in my understanding, the UUID is constantly changing. Since we want to know if we have persisted content for a specific key, the UUID wouldn't be sufficient, right?

[brototyp]

Depends how you initialize the UUID I guess. UUID() will always be different. UUID(uuidString: "<a string>") will always be the same.

[matthiasgebhardt]

Nope, unfortunately UUID(uuidString: "<a string>") initializes a UUID-instance from a string that has a UUID-format, otherwise it will return nil. Another solution could be "string".hashValue, since it would be consistent for each String, but it somehow feels wrong.

IMHO, the best solution would be md5. What would be your @brototyp approach for that?

[brototyp]

Are you sure. I just threw this in a playground and it always returned the exact same UUID.

let id = UUID(uuidString: "CEF67EFA-2536-433E-A1F4-4D3D810A16B7")

[matthiasgebhardt]

Yeah right, as I said, only if you pass a UUID-formatted string. But the given key will very unlikely be an UUID-formatted string.

[brototyp]

My thought was to forcing this. If we change the API of the storage for example, it's not our job anymore 😉

protocol DataStorage {
    func store(data: Data, forKey key: UUID) -> Bool
}

The RemoteSetting will then have the task of defining the UUID. Possibly by requiring it as a parameter when initializing a new instance.