Lost attachment after restore?

[tappdesign]

Hi Federico,

This is just pretty curious, not sure if you can reproduce it. You can check attached video https://user-images.githubusercontent.com/59613279/103489643-417ead00-4e16-11eb-95ba-cb3eb7f26548.mp4

Just backup all notes, then import it. At a first glance, looks ok, but after restart attachment is lost.

problem could be in BackupHelper.java --> reading old attachments from DB Just look at remarks written in CAPS

/**
     * Imports single note from its file
     */
    public static Note importNote(File file, CryptoHelper ch) {
        Note note = getImportNote(file, ch);
        if (note.getCategory() != null) {
            DbHelper.getInstance().updateCategory(note.getCategory());
        }
        //note.setAttachmentsListOld(DbHelper.getInstance().getNoteAttachments(note)); //  -->  COMMENTED OUT
        DbHelper.getInstance().updateNote(note, false);
        return note;
    }

    /**
     * Retrieves single note from its file
     */
    public static Note getImportNote(File file, CryptoHelper ch) {
        try {
            Note note = new Note();
            String jsonString = FileUtils.readFileToString(file);
            if (!TextUtils.isEmpty(jsonString)) {

                String plaintext = ch.decryptText(jsonString, ch.getCipher());
                if (!plaintext.isEmpty()) {
                    note.buildFromJson(plaintext);
                    //note.setAttachmentsListOld(DbHelper.getInstance().getNoteAttachments(note)); //  -->  COMMENTED OUT
                    note.setAttachmentsListOld(new ArrayList<>(note.getAttachmentsList()));   //  NEW ADDED --> JUST COPY THE ATTACHEMNT LIST, TO MAKE IT COMPATIBLE WITH :   deletedAttachments.remove(attachment);  in DBHelper.java
                }
            }
            return note;
        } catch (IOException e) {
            LogDelegate.e("Error parsing note json");
            return new Note();
        }
    }

the problem could be caused in DBHelper.java in updateNote() function ...(if AttachmentsListOld() are read from DB)

 // Updating attachments
    List<Attachment> deletedAttachments = note.getAttachmentsListOld();
    for (Attachment attachment : note.getAttachmentsList()) {
      updateAttachment(note.get_id() != null ? note.get_id() : values.getAsLong(KEY_CREATION),
          attachment, db);
      deletedAttachments.remove(attachment);  // REMOVING ATTACHMENT WILL FAIL, --> DELETED ATTACHMENTS ARE NOT DECREMENTED
    }
    // Remove from database deleted attachments
    for (Attachment attachmentDeleted : deletedAttachments) {       // DELETEDATTACHMENTS IS NOT EMPTY, IT WILL  BE DELETED FROM DB ACCIDENTALLY
      db.delete(TABLE_ATTACHMENTS, KEY_ATTACHMENT_ID + " = ?",
          new String[]{String.valueOf(attachmentDeleted.getId())});
    }

this test is fair complex, not sure if you can confirm it, and my solution is maybe not the correct one, but at least it could solve this issue on my phone.

[federicoiosue]

Some serious changes on that class are already taken in action on #796. I'll try to replicate your issue and adding some changes as the ones you reported and update this issue

[federicoiosue]

I've just pushed commit ed00f89ec02fa0000a468e94c1fdb524afb82add that should avoid some errors while making clearer with logs if something went wrong. Also a false positive backup processing has been managed (so no more "ok everything worked" with missing data).

Would you mind trying if that issue is still there or if you have more info by looking at error logs?

[tappdesign]

Hi Federico, yes it is there ;) I created pull request just to show the problematic line. After that patch it is working correctly. You can reject it of course, it is just proof of concept.

[tappdesign]

This happen when imported note already exist in current Database. With my proposal, all attachments assigned to the existing note stays untouched - after import will be (current + backuped) attachments assigned to note. (it is the safe way, data will be not lost). But if we want to discard additional attachments (attached later to the note after backup), my patch is wrong.

It has to be patched in DbHelper.java it has to be removed by attachmentID, not by the object itself, as it is in current solution: deletedAttachments.remove(attachment);