saoudrizwan / Disk

Easily persist structs, images, and data on iOS
MIT License
3.1k stars 171 forks source link

Saving a new Codable into a folder #8

Closed sahandnayebaziz closed 7 years ago

sahandnayebaziz commented 7 years ago

Hello @saoudrizwan! Thank you for sharing Disk, I am excited to try it out. I'm running into a small issue I've tried to debug but I'm not having any luck. Maybe you can help.

I shortened the code to the smallest reproducible case. With this code...

struct Message: Codable {
    var body: String
}

static func saveMessage() {
    do {
        let messages = [Message]()
        try Disk.save(messages, to: .documents, as: "Messages/")

        let first = Message(body: "Hello world.")
        try Disk.save(first, to: .documents, as: "Messages/first.json")
    } catch let error as NSError {
        print(error)
    }
}

I get this error...

Error Domain=NSCocoaErrorDomain Code=516 "The file “Messages” couldn’t be saved 
in the folder “Documents” because a file with the same name already exists." 
UserInfo={NSFilePath=/var/mobile/Containers/Data/Application/F45ABDA8-33A7-4003-9552-66722A855C55/Documents/Messages, 
NSUnderlyingError=0x1c024d500 {Error Domain=NSPOSIXErrorDomain Code=17 "File exists"}}

Things I've tried:

My end goal is to have a folder with individual JSON files, one for each Codable. From the readme, it seems that the one possible set of steps is:

  1. Create a folder from an empty array of type [Codable]
  2. Save an object of that same Codable type with a path that is [folderName]/[fileName].json

Am I perhaps using Disk incorrectly? Thank you for your help!

saoudrizwan commented 7 years ago
try Disk.save(messages, to: .documents, as: "Messages/")

This will result in a file called 'Message' being created in the documents directory, and then messages will be encoded into binary data and written to this file. Arrays of messages are saved as one file, whereas arrays of images are saved as numerated files to a folder like how you're thinking. I understand this can be confusing but I thought it was the best way to keep things simple in Disk.

Arrays are converted to JSON data that looks kind of like this:

{
     [
        {
            "name": "Caffè Macs",
            "coordinates": {
                "lat": 37.330576,
                "lng": -122.029739
            },
            "meals": ["breakfast", "lunch", "dinner"]
        },
                {
            "name": "McDonalds",
            "coordinates": {
                "lat": 81.320472,
                "lng": -218.01119
            },
            "meals": ["breakfast", "lunch", "dinner"]
        },
                {
            "name": "Olive Garden",
            "coordinates": {
                "lat": 12.335568,
                "lng": -117.025439
            },
            "meals": ["breakfast", "lunch", "dinner"]
        }
    ]
}

Also, that error 516 occurs when you try saving a file with the same name as a folder that exists in that directory. So you can't have a file name 'Messages' and a folder named 'Messages' in the documents directory at the same time. When you try saving to 'Messages/first.json', then it's trying to create the subfolder 'Messages' for you automatically and therefore you get that error since 'Messages' already exists as a file holding messages

Let me know if you need more clarification!

sahandnayebaziz commented 7 years ago

I do! Why have different behavior for Codables and Data?

Wouldn’t it be less work for both Disk and the developer if you could save one Codable to one JSON file in a folder and be done with it?

The way it’s currently set up, if I have an array of 100 large JSON objects, and I make a change to one, I either have to use the Disk.append method or re-save all 100 large objects even though only 1/100th of that data has changed.

Or, for example, one more level nested, what if somebody wants to make a folder named Meta/ and save three files in that folder named posts.json, messages.json, and contacts.json, each file a JSON array of [Codable]? Wouldn’t that not be possible with Disk because of the way Disk is handling Codables?

sahandnayebaziz commented 7 years ago

I think you’ve set up a wonderful library with nice mechanics, and I ask these questions because I want to see it succeed and I think these are basic questions that will come up often as other developers try to use Disk for their needs that will not be uncommon. If I tried to do it, there are doubtlessly others who have and more that will too.

sahandnayebaziz commented 7 years ago

Also, maybe this could use an error message to tell the developer that a folder wasn't created?

try Disk.save(messages, to: .documents, as: "Messages/")

This will result in a file called 'Message' being created in the documents directory, and then messages will be encoded into binary data and written to this file. Arrays of messages are saved as one file, whereas arrays of images are saved as numerated files to a folder like how you're thinking.

Since the run of .save() here has a slash at the end of the as: parameter, maybe that's enough to know that the developer is intending to create a folder. If this is tried with, say, a single Codable, maybe Disk can return an error saying "A folder can only be created with an empty value" or "A single file can not be created with the name of a folder", something besides a silent success. It was unclear to me why I was getting a File exists error after successfully calling .save() with an as: that ended with a slash. I was working with the assumption that Disk had succeeded only because Disk was able to create a folder, or otherwise it would have failed and thrown an error.

sahandnayebaziz commented 7 years ago

I just implemented a retrieveAll to load all files of Codable type T from a folder and return an array [T]:

static func retrieveAll<T: Decodable>(_ path: String, from directory: Directory, as type: T.Type) throws -> [T] {
    do {
        let url = try getExistingFileURL(for: path, in: directory)
        let fileUrls = try FileManager.default.contentsOfDirectory(at: url, includingPropertiesForKeys: nil, options: [])
        var objects = [T]()

        let decoder = JSONDecoder()

        for i in 0..<fileUrls.count {
            let fileUrl = fileUrls[i]
            let data = try Data(contentsOf: fileUrl)
            let decoded = try decoder.decode(type, from: data)
            objects.append(decoded)
        }

        return objects
    } catch {
        throw error
    }
}

I also had to change the save method, but now this works great for this use case as I have multiple .json files, one for each Codable, and they can be individually deleted/renamed/shared/etc without having to load and save all the Codables that need to go in that folder.

sahandnayebaziz commented 7 years ago

If the above is a direction you'd like to take this repo, let me know and I'm happy to work together on a PR. If not, no worries, thank you for letting me say my part :)

saoudrizwan commented 7 years ago

Hey @sahandnayebaziz, thanks for your interest in Disk! The reason that Disk handles Codable and Data types differently is because binary data isn't easy to manipulate without decoding it to a usable type. Whereas with Codable, all Disk needs to know is the type T and it will automatically decode the binary JSON data to that type using JSONDecoder. While this can be confusing, I have personally never had to store several pieces of binary Data to a single file, but rather as multiple files to a folder. Your retrieveAll method would fail if it encountered a file that couldn't be decoded to T. So you can't just retrieve Codable structs as Codable.self, you need to specify the exact struct type for JSONDecoder to work, i.e. Message.self.

If you need to retrieve all the JSON files in a folder, then you have to retrieve them as [Data], like so:

struct Contact: Codable {
    let firstName: String
    let lastName: String
}

struct Message: Codable {
    let title: String
    let body: String
}

let contacts = [Contact(firstName: "Saoud", lastName: "Rizwan"), Contact(firstName: "Sahand", lastName: "Nayebaziz")]

let messages = [Message(title: "One", body: "..."), Message(title: "Two", body: "...")]

do {
    try Disk.save(contacts, to: .documents, as: "Phone/contacts.json")
    try Disk.save(messages, to: .documents, as: "Phone/messages.json")
    let allData = try Disk.retrieve("Phone/", from: .documents, as: [Data].self)
} catch {
    // ...
}

I updated Disk (v0.2.4) to throw an error if you try saving struct(s) with a "/" at the end, so hopefully that clarifies things for anyone that encounters this same misunderstanding.

I think the way Disk behaves right now can me a bit hard to understand at first if you're not familiar with how Codable arrays are encoded, but once you realize that arrays are saved to one JSON file, then everything becomes much easier to use practically speaking. IMO, you don't need to worry about reading and writing to one single JSON file with lots of data, because at the end of the day it's just JSON text data, so it would never exceed more than a few kilobytes. The iPhone's hard disk write/read speeds are extremely fast, so in a real world scenario this isn't an issue to be worried about. If somehow you have megabytes of JSON data to persist, then just make sure you use Disk on a different thread than the main thread.

Thanks again for your input.

sahandnayebaziz commented 7 years ago

Thank you for your response.

While this can be confusing, I have personally never had to store several pieces of binary Data to a single file, but rather as multiple files to a folder.

^ I agree 100%. This is the exact behavior that other developers may want to use Disk for, but with JSON. As in...

I have personally never had to store several JSON objects to a single file as an array, but rather as multiple files to a folder. I would rather save each JSON object as its own file because they don't necessarily make sense put together in an array.

I will continue with this modified version of disk, so that I can have a folder saved with this structure:

projects/
  myFirstProject.json
  weekendProject.json
  sharedProject.json

This way, I don't need to take these objects in and out of arrays, they can just sit ready to go as individual files.

If you ever take this into a different direction to support individual JSON files not only in arrays, hit me up and I'll come back and contribute to a PR.

Cheers