diff --git a/app/src/main/java/com/pasich/mynotes/extendedEditor/utils/EditorDocument.java b/app/src/main/java/com/pasich/mynotes/extendedEditor/utils/EditorDocument.java new file mode 100644 index 00000000..d1dc0cf2 --- /dev/null +++ b/app/src/main/java/com/pasich/mynotes/extendedEditor/utils/EditorDocument.java @@ -0,0 +1,134 @@ +package com.pasich.mynotes.extendedEditor.utils; + +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; +import com.google.gson.JsonArray; +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; +import com.pasich.mynotes.extendedEditor.attach.EditorAttachmentBlocks; + +/** + * Whether an Editor.js document holds anything the user actually put there. + * + *

Emptiness used to be decided by comparing the serialized string against {@code "[]"}, which an + * untouched editor never produces: opening a note writes a document with one empty paragraph in it, + * so a note nobody typed into read as meaningful, was saved, and counted in the statistics. The + * answer has to come from the parsed blocks, because what the editor serializes for an empty + * document (block ids, a {@code time} field, a trailing paragraph) keeps changing. + * + *

Deliberately free of {@code android.*}, so the rule is testable under ordinary JVM tests. + */ +public final class EditorDocument { + + private EditorDocument() {} + + /** + * Reports whether the document carries text or an attachment. + * + *

A block counts as content unless it is a text block whose text is blank: a paragraph or + * header with nothing in it, or a list with no non-blank items. Anything else — a delimiter, a + * table, a tool this build does not know — counts, because only the empty paragraph is written + * without the user asking for it; everything else got there by a deliberate insertion. + * + * @param valueJson raw Editor.js block array, may be {@code null} + * @return {@code false} only when the document is provably empty + */ + public static boolean hasContent(@Nullable String valueJson) { + if (valueJson == null || valueJson.trim().isEmpty()) { + return false; + } + JsonArray blocks; + try { + blocks = JsonParser.parseString(valueJson).getAsJsonArray(); + } catch (RuntimeException unreadable) { + // Unreadable but not empty: keep the note rather than discard data we cannot read. + return true; + } + if (!EditorAttachmentBlocks.fileUrls(valueJson).isEmpty()) { + return true; + } + for (JsonElement element : blocks) { + if (!element.isJsonObject() || carriesContent(element.getAsJsonObject())) { + return true; + } + } + return false; + } + + /** The negation of "a text block with blank text". */ + private static boolean carriesContent(@NonNull JsonObject block) { + JsonElement dataElement = block.get("data"); + if (dataElement != null && dataElement.isJsonObject()) { + JsonObject data = dataElement.getAsJsonObject(); + JsonElement text = data.get("text"); + if (isString(text) && !isBlank(text.getAsString())) { + return true; + } + JsonElement items = data.get("items"); + if (items != null && items.isJsonArray() && anyItemFilled(items.getAsJsonArray())) { + return true; + } + } + // Nothing readable as text: only a text tool is allowed to be empty. Any other tool + // is in the document because someone inserted it. + return !isTextTool(block); + } + + /** Walks list items, including the nested ones a checklist or sub-list can hold. */ + private static boolean anyItemFilled(@NonNull JsonArray items) { + for (JsonElement element : items) { + if (element.isJsonPrimitive()) { + // The pre-2.x list tool stored plain strings instead of item objects. + if (!isBlank(element.getAsString())) return true; + continue; + } + if (!element.isJsonObject()) continue; + JsonObject item = element.getAsJsonObject(); + JsonElement content = item.get("content"); + if (isString(content) && !isBlank(content.getAsString())) { + return true; + } + JsonElement nested = item.get("items"); + if (nested != null && nested.isJsonArray() && anyItemFilled(nested.getAsJsonArray())) { + return true; + } + } + return false; + } + + /** Tools whose whole content is the text this class already read and found blank. */ + private static boolean isTextTool(@NonNull JsonObject block) { + JsonElement type = block.get("type"); + if (!isString(type)) return false; + switch (type.getAsString()) { + case "paragraph": + case "header": + case "Headers": + case "list": + case "checklist": + return true; + default: + return false; + } + } + + /** Blank once the markup an empty editor line leaves behind is taken out. */ + private static boolean isBlank(@Nullable String html) { + if (html == null) return true; + String text = + html.replaceAll("(?i)", " ") + .replaceAll("<[^>]+>", "") + .replace(" ", " ") + .replace('\u00a0', ' ') + .replace("\ufeff", "") + .replace("\u200b", ""); + return text.trim().isEmpty(); + } + + private static boolean isString(@Nullable JsonElement element) { + return element != null + && element.isJsonPrimitive() + && element.getAsJsonPrimitive().isString(); + } +} diff --git a/app/src/main/java/com/pasich/mynotes/ui/presenter/NotePresenter.java b/app/src/main/java/com/pasich/mynotes/ui/presenter/NotePresenter.java index 4b60635f..471774c5 100644 --- a/app/src/main/java/com/pasich/mynotes/ui/presenter/NotePresenter.java +++ b/app/src/main/java/com/pasich/mynotes/ui/presenter/NotePresenter.java @@ -7,6 +7,7 @@ import com.pasich.mynotes.data.DataManager; import com.pasich.mynotes.data.model.Note; import com.pasich.mynotes.extendedEditor.models.ParsedNote; +import com.pasich.mynotes.extendedEditor.utils.EditorDocument; import com.pasich.mynotes.extendedEditor.utils.EditorJsonUtils; import com.pasich.mynotes.ui.contract.NoteContract; import com.pasich.mynotes.utils.constants.AutoSave; @@ -320,12 +321,6 @@ public void closeActivity() { pendingClose = true; - // Extended editor "empty JSON" check is WRONG — replace with targetNote - if (extendedEditor && !hasMeaningfulContent(targetNote)) { - if (!isViewDead()) getView().closeNoteActivity(); - return; - } - saveNote( targetNote, new NoteContract.AutoSaveCallback() { @@ -391,14 +386,18 @@ private boolean hasMeaningfulContent(Note note) { if (!note.getTitle().trim().isEmpty()) return true; if (!note.getValue().trim().isEmpty()) return true; - // Attachments (extended editor too) - if (note.getAttachments() != null && !note.getAttachments().trim().isEmpty()) { + // Attachments (extended editor too). Every extended-editor change writes the parsed + // attachment list, so a note without attachments carries the string "[]" here — ask the + // note whether the list holds anything instead of whether the field was written. + if (note.isAttachments()) { return true; } - // Extended editor JSON - String json = note.getValueJson(); - return json != null && !json.trim().isEmpty() && !json.equals("[]"); + // Extended editor document: emptiness is decided by the parsed blocks. An untouched + // editor still serializes a document — an empty paragraph is enough — so comparing the + // string against "[]" made a note nobody typed into read as meaningful, saved it and + // counted it in the statistics. + return EditorDocument.hasContent(note.getValueJson()); } @Override diff --git a/app/src/test/java/com/pasich/mynotes/extendedEditor/utils/EditorDocumentTest.java b/app/src/test/java/com/pasich/mynotes/extendedEditor/utils/EditorDocumentTest.java new file mode 100644 index 00000000..f82eabd0 --- /dev/null +++ b/app/src/test/java/com/pasich/mynotes/extendedEditor/utils/EditorDocumentTest.java @@ -0,0 +1,128 @@ +package com.pasich.mynotes.extendedEditor.utils; + +import static com.google.common.truth.Truth.assertThat; + +import org.junit.Test; + +/** + * What counts as an empty rich note. + * + *

Emptiness used to be "the serialized document is not the string {@code []}", so a note the + * user only opened — the editor writes one empty paragraph into it — was saved and counted in the + * statistics. The shapes below are the ones the editor actually produces for a document nobody + * typed into, plus the ones that must survive. + */ +public class EditorDocumentTest { + + @Test + public void noDocumentAtAllIsEmpty() { + assertThat(EditorDocument.hasContent(null)).isFalse(); + assertThat(EditorDocument.hasContent("")).isFalse(); + assertThat(EditorDocument.hasContent(" ")).isFalse(); + assertThat(EditorDocument.hasContent("[]")).isFalse(); + } + + @Test + public void singleEmptyParagraphIsEmpty() { + // The document an untouched extended editor serializes. + assertThat( + EditorDocument.hasContent( + "[{\"id\":\"a1\",\"type\":\"paragraph\",\"data\":{\"text\":\"\"}}]")) + .isFalse(); + } + + @Test + public void paragraphOfWhitespaceAndEmptyMarkupIsEmpty() { + assertThat( + EditorDocument.hasContent( + "[{\"type\":\"paragraph\",\"data\":{\"text\":\" \"}}," + + "{\"type\":\"paragraph\",\"data\":{\"text\":\"
\"}}," + + "{\"type\":\"paragraph\",\"data\":{\"text\":\" \"}}," + + "{\"type\":\"paragraph\",\"data\":{\"text\":\"\"}}]")) + .isFalse(); + } + + @Test + public void emptyHeaderAndEmptyListAreEmpty() { + assertThat( + EditorDocument.hasContent( + "[{\"type\":\"header\",\"data\":{\"text\":\"\",\"level\":2}}," + + "{\"type\":\"list\",\"data\":{\"style\":\"unordered\",\"items\":[]}}," + + "{\"type\":\"list\",\"data\":{\"style\":\"checklist\",\"items\":[{\"content\":\"\",\"meta\":{\"checked\":false}}]}}]")) + .isFalse(); + } + + @Test + public void typedTextIsContent() { + assertThat( + EditorDocument.hasContent( + "[{\"type\":\"paragraph\",\"data\":{\"text\":\"hello\"}}]")) + .isTrue(); + } + + @Test + public void textHiddenBehindMarkupIsContent() { + assertThat( + EditorDocument.hasContent( + "[{\"type\":\"paragraph\",\"data\":{\"text\":\"bold\"}}]")) + .isTrue(); + } + + @Test + public void filledListItemIsContent() { + assertThat( + EditorDocument.hasContent( + "[{\"type\":\"list\",\"data\":{\"style\":\"unordered\",\"items\":[{\"content\":\"\"},{\"content\":\"milk\"}]}}]")) + .isTrue(); + } + + @Test + public void nestedListItemIsContent() { + assertThat( + EditorDocument.hasContent( + "[{\"type\":\"list\",\"data\":{\"style\":\"unordered\",\"items\":[{\"content\":\"\",\"items\":[{\"content\":\"deep\"}]}]}}]")) + .isTrue(); + } + + @Test + public void attachmentAloneIsContent() { + // A note that is only a file has no text at all and must still be kept. + assertThat( + EditorDocument.hasContent( + "[{\"type\":\"paragraph\",\"data\":{\"text\":\"\"}}," + + "{\"type\":\"attaches\",\"data\":{\"file\":{\"url\":\"editorjs://attachments/note_5/one.pdf\",\"name\":\"one.pdf\"}}}]")) + .isTrue(); + } + + @Test + public void imageAndGalleryBlocksAreContent() { + assertThat( + EditorDocument.hasContent( + "[{\"type\":\"image\",\"data\":{\"file\":{\"url\":\"editorjs://attachments/note_5/a.png\"}}}]")) + .isTrue(); + assertThat( + EditorDocument.hasContent( + "[{\"type\":\"gallery\",\"data\":{\"files\":[{\"url\":\"editorjs://attachments/note_5/b.png\"}]}}]")) + .isTrue(); + } + + @Test + public void aToolWithNoTextOfItsOwnIsContent() { + // Nothing but an empty paragraph is written without the user asking for it, so a + // delimiter, a table, or a tool this build cannot read is kept. + assertThat(EditorDocument.hasContent("[{\"type\":\"delimiter\",\"data\":{}}]")).isTrue(); + assertThat( + EditorDocument.hasContent( + "[{\"type\":\"table\",\"data\":{\"content\":[[\"a\",\"b\"]]}}]")) + .isTrue(); + assertThat(EditorDocument.hasContent("[{\"type\":\"someFutureTool\",\"data\":{}}]")) + .isTrue(); + } + + @Test + public void unreadableDocumentIsKept() { + // Discarding a note we simply failed to parse would lose the user's data. + assertThat(EditorDocument.hasContent("{not json")).isTrue(); + assertThat(EditorDocument.hasContent("{\"blocks\":[]}")).isTrue(); + } +} diff --git a/app/src/test/java/com/pasich/mynotes/presenter/NotePresenterEmptyExtendedNoteTest.java b/app/src/test/java/com/pasich/mynotes/presenter/NotePresenterEmptyExtendedNoteTest.java new file mode 100644 index 00000000..161f7805 --- /dev/null +++ b/app/src/test/java/com/pasich/mynotes/presenter/NotePresenterEmptyExtendedNoteTest.java @@ -0,0 +1,132 @@ +package com.pasich.mynotes.presenter; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.pasich.mynotes.base.BasePresenterTest; +import com.pasich.mynotes.data.DataManager; +import com.pasich.mynotes.data.model.Note; +import com.pasich.mynotes.ui.contract.NoteContract; +import com.pasich.mynotes.ui.presenter.NotePresenter; +import io.reactivex.Completable; +import io.reactivex.disposables.CompositeDisposable; +import java.util.Date; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; + +/** + * A new note left untouched in the extended editor must be discarded, exactly as one left untouched + * in the simple editor is. + * + *

It used to survive: the editor writes an empty paragraph into an opened note, and emptiness + * was decided by comparing that serialized document against the string {@code []}. The note was + * then saved on close and counted in the statistics. + */ +public class NotePresenterEmptyExtendedNoteTest extends BasePresenterTest { + + private static final String UNTOUCHED_DOCUMENT = + "[{\"id\":\"a1\",\"type\":\"paragraph\",\"data\":{\"text\":\"\"}}]"; + + @Mock DataManager mockDataManager; + @Mock NoteContract.view mockView; + NotePresenter presenter; + + @Before + public void setUp() { + initMocks(this); + when(mockDataManager.updateNote(any())).thenReturn(Completable.complete()); + when(mockDataManager.deleteNote(any(Note.class))).thenReturn(Completable.complete()); + presenter = + new NotePresenter( + testSchedulerProvider(), new CompositeDisposable(), mockDataManager); + presenter.attachView(mockView); + presenter.setExtendedEditor(true); + presenter.setNewNoteKey(true); + presenter.setIdKey(1L); + } + + /** The note the editor hands back when it was opened and nothing was typed. */ + private Note untouchedNote() { + Note note = new Note().create("", "", new Date().getTime(), ""); + note.setValueJson(UNTOUCHED_DOCUMENT); + // extendedNoteChange() always writes the parsed attachment list; with no files that is + // the empty list, which used to read as "this note has attachments". + note.setAttachments("[]"); + return note; + } + + @Test + public void untouchedNewNoteIsDeletedOnClose() { + Note note = untouchedNote(); + presenter.setNote(note); + + presenter.closeActivity(); + + verify(mockDataManager, times(1)).deleteNote(note); + verify(mockDataManager, never()).updateNote(any(Note.class)); + verify(mockView).closeNoteActivity(); + } + + @Test + public void untouchedNewNoteIsNotAutoSaved() { + presenter.setNote(untouchedNote()); + + presenter.onNoteChanged(); + + verify(mockDataManager, never()).updateNote(any(Note.class)); + } + + @Test + public void newNoteWithTypedTextIsSavedOnClose() { + Note note = new Note().create("", "", new Date().getTime(), ""); + note.setValueJson("[{\"id\":\"a1\",\"type\":\"paragraph\",\"data\":{\"text\":\"hi\"}}]"); + note.setValue("hi"); + note.setAttachments("[]"); + presenter.setNote(note); + + presenter.closeActivity(); + + verify(mockDataManager, times(1)).updateNote(note); + verify(mockDataManager, never()).deleteNote(any(Note.class)); + } + + /** + * Deliberate, and the same answer the simple editor has always given: emptying a note is not a + * way to erase it, so the stored text stays and the note is not deleted either. Pinned because + * the two editors disagreed here until the emptiness rule was shared. + */ + @Test + public void existingNoteEmptiedByTheUserKeepsWhatWasStored() { + presenter.setNewNoteKey(false); + Note note = new Note().create("", "old text", new Date().getTime(), ""); + note.setValueJson("[{\"type\":\"paragraph\",\"data\":{\"text\":\"old text\"}}]"); + presenter.setNote(note); + + // The editor keeps emitting a document after the user clears it: one empty paragraph. + presenter.extendedNoteChange( + "", "[{\"id\":\"a1\",\"type\":\"paragraph\",\"data\":{\"text\":\"\"}}]"); + presenter.closeActivity(); + + verify(mockDataManager, never()).updateNote(any(Note.class)); + verify(mockDataManager, never()).deleteNote(any(Note.class)); + verify(mockView).closeNoteActivity(); + } + + @Test + public void newNoteWithOnlyAnAttachmentIsSavedOnClose() { + Note note = new Note().create("", "", new Date().getTime(), ""); + note.setValueJson( + "[{\"type\":\"attaches\",\"data\":{\"file\":{\"url\":\"editorjs://attachments/note_1/one.pdf\"}}}]"); + note.setAttachments("[{\"url\":\"editorjs://attachments/note_1/one.pdf\"}]"); + presenter.setNote(note); + + presenter.closeActivity(); + + verify(mockDataManager, times(1)).updateNote(note); + verify(mockDataManager, never()).deleteNote(any(Note.class)); + } +}