fix attachments not loading in dev environment

[joshua-journey-apps]

Problem

Attachments files fail to load on IOS builds. This can be seen by following these steps in the Demo app:

1. Build the app in the simulator
2. Create a list
3. Create a todo
4. Add an image to the todo
5. Quit the application using the app launcher
6. Sign in again
7. Go to the todo you created above
8. Observe the stuck spinner

Refer to this discussion for more details:
https://discord.com/channels/1138230179878154300/1387703779662631004

[simolus3]

I'm having some issues with my simulator and can't reliably reproduce this. But my understanding is that:

1. In verifyAttachments, we realize that the path doesn't exist and set localUri to nil and state to .archived.
2. Then in processWatchedAttachments, we realize that the attachment is still live and change it back to synced (line 262).
3. Then again in verifyAttachments, we now queue a download because we've encountered a synced attachment without a local URL.

To me, it sounds like step 2 is the issue. I think the newState check that we have in line 268 also needs to be happening when the item has already been synced (setting the state to queuedDownload if we have no local uri and to synced if we do). Does that also fix this issue? To me that sounds like a more straightforward fix, but I might be misunderstanding the exact cause. Also cc @stevensJourney for thoughts

[stevensJourney]

I might either be repeating or misunderstanding Simon's recommendation. Here are my thoughts.

I haven't tested this with this specific issue yet, but I'd propose a third solution where the verifyAttachments method checks if the file at the local_uri exists and updates the state directly to queuedDownload without going through the loop of clearing the local_uri.

    private func verifyAttachments(context: AttachmentContext) async throws {
        let attachments = try await context.getAttachments()
        var updates: [Attachment] = []

        for attachment in attachments {
            guard let localUri = attachment.localUri else {
                continue
            }

            let exists = try await localStorage.fileExists(filePath: localUri)
            if exists {
                // The file exists, this is correct
                continue
            }

            if attachment.state == AttachmentState.queuedUpload {
                // The file must have been removed from the local storage before upload was completed
                updates.append(attachment.with(
                    state: .archived,
                    localUri: .some(nil) // Clears the value
                ))
            } else if attachment.state == AttachmentState.synced {
                // The file was downloaded, but removed - trigger redownload
                updates.append(attachment.with(
                    state: .queuedDownload
                ))
            }
        }

        try await context.saveAttachments(attachments: updates)
    }

[joshua-journey-apps]

Tested both approaches:

Added the following processWatchedAttachments

    if existingQueueItem.state == AttachmentState.synced && existingQueueItem.localUri == nil  {
        attachmentUpdates.append(
            existingQueueItem.with(state: AttachmentState.queuedDownload)
        )
    }

Works like a charm.

Also tested the verifyAttachments, and the problem comes in with the early continue which causes attachment without local uri's to be skipped before queueing them. So in my head we have to add the following to the localUri else statement:

guard let localUri = attachment.localUri else {
   if attachment.state == AttachmentState.synced {
         updates.append(attachment.with(state: .queuedDownload))
   }
   continue
 }

or even

private func verifyAttachments(context: AttachmentContext) async throws {
   let attachments = try await context.getAttachments()
   var updates: [Attachment] = []

   for attachment in attachments {

       let exists: Bool
       if let localUri = attachment.localUri {
           exists = try await localStorage.fileExists(filePath: localUri)
       } else {
           exists = false
       }
       
       if exists {
           // The file exists, this is correct
           continue
       }

       if attachment.state == AttachmentState.queuedUpload {
           // The file must have been removed from the local storage before upload was completed
           updates.append(attachment.with(
               state: .archived,
               localUri: .some(nil) // Clears the value
           ))
       } else if attachment.state == AttachmentState.synced {
           // The file was downloaded, but removed - trigger redownload
           updates.append(attachment.with(
               state: .queuedDownload
           ))
       }
   }

   try await context.saveAttachments(attachments: updates)
}

Not too sure which approach you guys would recommend?

[simolus3]

Also tested the verifyAttachments, and the problem comes in with the early continue which causes attachment without local uri's to be skipped before queueing them

Is this from a clean state? I'm not sure how assets manage to be in .synced while also having localUrl: nil - that sounds like a broken invariant worth preventing. With the approach that Steven suggested, shouldn't the problematic intermediate state of .archived that later goes back to .synced be fixed?

[joshua-journey-apps]

Cleared sqlite and tested it again and you are right Simon, there were some lingering malformed attachments throwing it off. After testing it with Steven's fix the attachment always goes through the correct flow:

1. Attachment is synced but local uri pointed to path from previous build
2. Previous build path does not exist, so add .queueDownload update
3. handleAsync queues attachment download

[simolus3]

Looks good to me - thank you Joshua!

[stevensJourney]

Just a note. A changelog entry for this fix would still be required.