From dd8cf671c6eeba486a42e6074dcf1d7b270e447c Mon Sep 17 00:00:00 2001 From: Stefan Niedermann Date: Tue, 5 Jan 2021 12:35:58 +0100 Subject: [PATCH] Ensure that internal notes links are not persisted --- .../notes/edit/NotePreviewFragment.java | 28 +++----- .../notes/shared/util/NoteLinksUtils.java | 26 ------- .../shared/util/text/NoteLinksProcessor.java | 57 --------------- .../notes/shared/util/text/TextProcessor.java | 10 --- .../shared/util/text/TextProcessorChain.java | 12 ---- .../util/text/NoteLinksProcessorTest.java | 72 ------------------- .../util/text/TextProcessorChainTest.java | 35 --------- .../markwon/span/InterceptedURLSpan.java | 19 +++-- 8 files changed, 25 insertions(+), 234 deletions(-) delete mode 100644 app/src/main/java/it/niedermann/owncloud/notes/shared/util/NoteLinksUtils.java delete mode 100644 app/src/main/java/it/niedermann/owncloud/notes/shared/util/text/NoteLinksProcessor.java delete mode 100644 app/src/main/java/it/niedermann/owncloud/notes/shared/util/text/TextProcessor.java delete mode 100644 app/src/main/java/it/niedermann/owncloud/notes/shared/util/text/TextProcessorChain.java delete mode 100644 app/src/test/java/it/niedermann/owncloud/notes/shared/util/text/NoteLinksProcessorTest.java delete mode 100644 app/src/test/java/it/niedermann/owncloud/notes/shared/util/text/TextProcessorChainTest.java diff --git a/app/src/main/java/it/niedermann/owncloud/notes/edit/NotePreviewFragment.java b/app/src/main/java/it/niedermann/owncloud/notes/edit/NotePreviewFragment.java index e220f3e6..754b4cb5 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/edit/NotePreviewFragment.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/edit/NotePreviewFragment.java @@ -6,6 +6,7 @@ import android.graphics.Typeface; import android.os.Bundle; import android.text.Layout; import android.text.method.LinkMovementMethod; +import android.util.Log; import android.util.TypedValue; import android.view.LayoutInflater; import android.view.Menu; @@ -28,17 +29,15 @@ import com.nextcloud.android.sso.model.SingleSignOnAccount; import it.niedermann.owncloud.notes.R; import it.niedermann.owncloud.notes.databinding.FragmentNotePreviewBinding; import it.niedermann.owncloud.notes.persistence.NotesDatabase; -import it.niedermann.owncloud.notes.shared.model.DBNote; -import it.niedermann.owncloud.notes.shared.util.NoteLinksUtils; import it.niedermann.owncloud.notes.shared.util.SSOUtil; -import it.niedermann.owncloud.notes.shared.util.text.NoteLinksProcessor; -import it.niedermann.owncloud.notes.shared.util.text.TextProcessorChain; import static androidx.core.view.ViewCompat.isAttachedToWindow; import static it.niedermann.owncloud.notes.shared.util.NoteUtil.getFontSizeFromPreferences; public class NotePreviewFragment extends SearchableBaseNoteFragment implements OnRefreshListener { + private static final String TAG = NotePreviewFragment.class.getSimpleName(); + private String changedText; protected FragmentNotePreviewBinding binding; @@ -83,18 +82,20 @@ public class NotePreviewFragment extends SearchableBaseNoteFragment implements O public void onActivityCreated(@Nullable Bundle savedInstanceState) { super.onActivityCreated(savedInstanceState); binding.singleNoteContent.registerOnLinkClickCallback((link) -> { - if (NoteLinksUtils.isNoteLink(link)) { - final long noteRemoteId = NoteLinksUtils.extractNoteRemoteId(link); - final long noteLocalId = db.getLocalIdByRemoteId(this.note.getAccountId(), noteRemoteId); + try { + final long noteLocalId = db.getLocalIdByRemoteId(this.note.getAccountId(), Long.parseLong(link)); final Intent intent = new Intent(requireActivity().getApplicationContext(), EditNoteActivity.class); intent.putExtra(EditNoteActivity.PARAM_NOTE_ID, noteLocalId); startActivity(intent); return true; + } catch (NumberFormatException e) { + Log.v(TAG, "Clicked link \"" + link + "\" is not a " + Long.class.getSimpleName() + ". Do not try to treat it as another note."); + } catch (IllegalArgumentException e) { + Log.i(TAG, "It looks like \"" + link + "\" might be a remote id of a note, but a note with this remote id could not be found.", e); } return false; }); - final TextProcessorChain chain = defaultTextProcessorChain(note); - binding.singleNoteContent.setMarkdownString(chain.apply(note.getContent())); + binding.singleNoteContent.setMarkdownString(note.getContent()); binding.singleNoteContent.setMovementMethod(LinkMovementMethod.getInstance()); changedText = note.getContent(); @@ -131,12 +132,11 @@ public class NotePreviewFragment extends SearchableBaseNoteFragment implements O if (db.getNoteServerSyncHelper().isSyncPossible() && SSOUtil.isConfigured(getContext())) { binding.swiperefreshlayout.setRefreshing(true); try { - TextProcessorChain chain = defaultTextProcessorChain(note); SingleSignOnAccount ssoAccount = SingleAccountHelper.getCurrentSingleSignOnAccount(requireContext()); db.getNoteServerSyncHelper().addCallbackPull(ssoAccount, () -> { note = db.getNote(note.getAccountId(), note.getId()); changedText = note.getContent(); - binding.singleNoteContent.setMarkdownString(chain.apply(note.getContent())); + binding.singleNoteContent.setMarkdownString(note.getContent()); binding.swiperefreshlayout.setRefreshing(false); }); db.getNoteServerSyncHelper().scheduleSync(ssoAccount, false); @@ -156,12 +156,6 @@ public class NotePreviewFragment extends SearchableBaseNoteFragment implements O binding.singleNoteContent.setHighlightColor(getTextHighlightBackgroundColor(requireContext(), mainColor, colorPrimary, colorAccent)); } - private TextProcessorChain defaultTextProcessorChain(DBNote note) { - TextProcessorChain chain = new TextProcessorChain(); - chain.add(new NoteLinksProcessor(db.getRemoteIds(note.getAccountId()))); - return chain; - } - public static BaseNoteFragment newInstance(long accountId, long noteId) { final BaseNoteFragment fragment = new NotePreviewFragment(); final Bundle args = new Bundle(); diff --git a/app/src/main/java/it/niedermann/owncloud/notes/shared/util/NoteLinksUtils.java b/app/src/main/java/it/niedermann/owncloud/notes/shared/util/NoteLinksUtils.java deleted file mode 100644 index 668d2746..00000000 --- a/app/src/main/java/it/niedermann/owncloud/notes/shared/util/NoteLinksUtils.java +++ /dev/null @@ -1,26 +0,0 @@ -package it.niedermann.owncloud.notes.shared.util; - -import it.niedermann.owncloud.notes.shared.util.text.NoteLinksProcessor; - -public class NoteLinksUtils { - - /** - * Tests if the given link is a note-link (which was transformed in {@link it.niedermann.owncloud.notes.shared.util.text.NoteLinksProcessor}) or not - * - * @param link Link under test - * @return true if the link is a note-link - */ - public static boolean isNoteLink(String link) { - return link.startsWith(NoteLinksProcessor.RELATIVE_LINK_WORKAROUND_PREFIX); - } - - /** - * Extracts the remoteId back from links that were transformed in {@link it.niedermann.owncloud.notes.shared.util.text.NoteLinksProcessor}. - * - * @param link Link that was transformed in {@link it.niedermann.owncloud.notes.shared.util.text.NoteLinksProcessor} - * @return the remoteId of the linked note - */ - public static long extractNoteRemoteId(String link) { - return Long.parseLong(link.replace(NoteLinksProcessor.RELATIVE_LINK_WORKAROUND_PREFIX, "")); - } -} diff --git a/app/src/main/java/it/niedermann/owncloud/notes/shared/util/text/NoteLinksProcessor.java b/app/src/main/java/it/niedermann/owncloud/notes/shared/util/text/NoteLinksProcessor.java deleted file mode 100644 index bc656dd8..00000000 --- a/app/src/main/java/it/niedermann/owncloud/notes/shared/util/text/NoteLinksProcessor.java +++ /dev/null @@ -1,57 +0,0 @@ -package it.niedermann.owncloud.notes.shared.util.text; - -import android.text.TextUtils; - -import java.util.HashSet; -import java.util.Set; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - -import androidx.annotation.VisibleForTesting; - -public class NoteLinksProcessor extends TextProcessor { - - public static final String RELATIVE_LINK_WORKAROUND_PREFIX = "https://nextcloudnotes/notes/"; - - @VisibleForTesting - private static final String linksThatLookLikeNoteLinksRegEx = "\\[[^]]*]\\((\\d+)\\)"; - private static final String replaceNoteRemoteIdsRegEx = "\\[([^\\]]*)\\]\\((%s)\\)"; - - private final Set existingNoteRemoteIds; - - public NoteLinksProcessor(Set existingNoteRemoteIds) { - this.existingNoteRemoteIds = existingNoteRemoteIds; - } - - /** - * Replaces all links to other notes of the form `[]()` - * in the markdown string with links to a dummy url. - * - * Why is this needed? - * See discussion in issue #623 - * - * @return Markdown with all note-links replaced with dummy-url-links - */ - @Override - public String process(String s) { - return replaceNoteLinksWithDummyUrls(s, existingNoteRemoteIds); - } - - private static String replaceNoteLinksWithDummyUrls(String markdown, Set existingNoteRemoteIds) { - Pattern noteLinkCandidates = Pattern.compile(linksThatLookLikeNoteLinksRegEx); - Matcher matcher = noteLinkCandidates.matcher(markdown); - - Set noteRemoteIdsToReplace = new HashSet<>(); - while (matcher.find()) { - String presumedNoteId = matcher.group(1); - if (existingNoteRemoteIds.contains(presumedNoteId)) { - noteRemoteIdsToReplace.add(presumedNoteId); - } - } - - String noteRemoteIdsCondition = TextUtils.join("|", noteRemoteIdsToReplace); - Pattern replacePattern = Pattern.compile(String.format(replaceNoteRemoteIdsRegEx, noteRemoteIdsCondition)); - Matcher replaceMatcher = replacePattern.matcher(markdown); - return replaceMatcher.replaceAll(String.format("[$1](%s$2)", RELATIVE_LINK_WORKAROUND_PREFIX)); - } -} diff --git a/app/src/main/java/it/niedermann/owncloud/notes/shared/util/text/TextProcessor.java b/app/src/main/java/it/niedermann/owncloud/notes/shared/util/text/TextProcessor.java deleted file mode 100644 index 4eb4e4f7..00000000 --- a/app/src/main/java/it/niedermann/owncloud/notes/shared/util/text/TextProcessor.java +++ /dev/null @@ -1,10 +0,0 @@ -package it.niedermann.owncloud.notes.shared.util.text; - -abstract public class TextProcessor { - /** - * Applies a specified transformation on a text string and returns the updated string. - * @param s Text to transform - * @return Transformed text - */ - abstract public String process(String s); -} \ No newline at end of file diff --git a/app/src/main/java/it/niedermann/owncloud/notes/shared/util/text/TextProcessorChain.java b/app/src/main/java/it/niedermann/owncloud/notes/shared/util/text/TextProcessorChain.java deleted file mode 100644 index 70af737a..00000000 --- a/app/src/main/java/it/niedermann/owncloud/notes/shared/util/text/TextProcessorChain.java +++ /dev/null @@ -1,12 +0,0 @@ -package it.niedermann.owncloud.notes.shared.util.text; - -import java.util.LinkedList; - -public class TextProcessorChain extends LinkedList { - public String apply(String s) { - for (TextProcessor textProcessor : this) { - s = textProcessor.process(s); - } - return s; - } -} diff --git a/app/src/test/java/it/niedermann/owncloud/notes/shared/util/text/NoteLinksProcessorTest.java b/app/src/test/java/it/niedermann/owncloud/notes/shared/util/text/NoteLinksProcessorTest.java deleted file mode 100644 index 8b96a207..00000000 --- a/app/src/test/java/it/niedermann/owncloud/notes/shared/util/text/NoteLinksProcessorTest.java +++ /dev/null @@ -1,72 +0,0 @@ -package it.niedermann.owncloud.notes.shared.util.text; - -import junit.framework.TestCase; - -import org.junit.Assert; - -import java.util.Collections; -import java.util.HashSet; -import java.util.Set; - -import static it.niedermann.owncloud.notes.shared.util.text.NoteLinksProcessor.RELATIVE_LINK_WORKAROUND_PREFIX; - -public class NoteLinksProcessorTest extends TestCase { - - public void testEmptyString() { - TextProcessor sut = new NoteLinksProcessor(Collections.emptySet()); - - String markdown = ""; - String result = sut.process(markdown); - Assert.assertEquals("", result); - } - - public void testDoNotChangeOtherMarkdownElements() { - TextProcessor sut = new NoteLinksProcessor(Collections.emptySet()); - - //language=md - String markdown = "\n" + - "# heading \n" + - " \n" + - "This is a _markdown_ document. \n" + - " \n" + - "But\n" + - " - there \n" + - " - are \n" + - " - no \n" + - "\n" + - "link elements.\n" + - "\n" + - "----\n" + - "**Everything** else could be in here.\n" + - "\n"; - - Assert.assertEquals(markdown, sut.process(markdown)); - } - - @SuppressWarnings("MarkdownUnresolvedFileReference") - public void testDoNotReplaceNormalLinks() { - TextProcessor sut = new NoteLinksProcessor(Collections.singleton("123456")); - - //language=md - String markdown = "[normal link](https://example.com) and another [note link](123456)"; - String result = sut.process(markdown); - Assert.assertEquals(String.format("[normal link](https://example.com) and another [note link](%s123456)", RELATIVE_LINK_WORKAROUND_PREFIX), result); - } - - public void testReplaceOnlyNotesInDB() { - Set remoteIdsOfExistingNotes = new HashSet<>(); - remoteIdsOfExistingNotes.add("123456"); - remoteIdsOfExistingNotes.add("321456"); - - TextProcessor sut = new NoteLinksProcessor(remoteIdsOfExistingNotes); - - String markdown = "[link to real note](123456) and another [link to non-existing note](654321) and one more [another link to real note](321456)"; - - String result = sut.process(markdown); - - String expected = String.format("[link to real note](%s123456) and another [link to non-existing note](654321) and one more [another link to real note](%s321456)", RELATIVE_LINK_WORKAROUND_PREFIX, RELATIVE_LINK_WORKAROUND_PREFIX); - Assert.assertEquals( - expected, - result); - } -} \ No newline at end of file diff --git a/app/src/test/java/it/niedermann/owncloud/notes/shared/util/text/TextProcessorChainTest.java b/app/src/test/java/it/niedermann/owncloud/notes/shared/util/text/TextProcessorChainTest.java deleted file mode 100644 index fcf38718..00000000 --- a/app/src/test/java/it/niedermann/owncloud/notes/shared/util/text/TextProcessorChainTest.java +++ /dev/null @@ -1,35 +0,0 @@ -package it.niedermann.owncloud.notes.shared.util.text; - -import junit.framework.TestCase; - -import org.junit.Assert; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; - -public class TextProcessorChainTest extends TestCase { - - public void testApplyAllInOrder() { - TextProcessorChain chain = new TextProcessorChain(); - chain.add(new SelfIdentifyingProcessor(1)); - chain.add(new SelfIdentifyingProcessor(2)); - - Assert.assertEquals("SelfIdentifyingProcessor 1\nSelfIdentifyingProcessor 2", chain.apply("")); - } - - static class SelfIdentifyingProcessor extends TextProcessor { - private int id; - - public SelfIdentifyingProcessor(int id) { - this.id = id; - } - - @Override - public String process(String s) { - List parts = new ArrayList<>(Arrays.asList(s.split("\n"))); - parts.add(String.format("%s %d", getClass().getSimpleName(), id)); - return String.join("\n", parts.toArray(new String[]{})).trim(); - } - } -} \ No newline at end of file diff --git a/markdown/src/main/java/it/niedermann/android/markdown/markwon/span/InterceptedURLSpan.java b/markdown/src/main/java/it/niedermann/android/markdown/markwon/span/InterceptedURLSpan.java index 0cc4a80b..34e0756b 100644 --- a/markdown/src/main/java/it/niedermann/android/markdown/markwon/span/InterceptedURLSpan.java +++ b/markdown/src/main/java/it/niedermann/android/markdown/markwon/span/InterceptedURLSpan.java @@ -1,6 +1,7 @@ package it.niedermann.android.markdown.markwon.span; import android.text.style.URLSpan; +import android.util.Log; import android.view.View; import androidx.annotation.NonNull; @@ -9,6 +10,8 @@ import java.util.Collection; import java.util.function.Function; public class InterceptedURLSpan extends URLSpan { + + private static final String TAG = InterceptedURLSpan.class.getSimpleName(); @NonNull private final Collection> onLinkClickCallbacks; @@ -19,11 +22,17 @@ public class InterceptedURLSpan extends URLSpan { @Override public void onClick(View widget) { - for (Function callback : onLinkClickCallbacks) { - if (callback.apply(getURL())) { - return; + new Thread(() -> { + for (Function callback : onLinkClickCallbacks) { + try { + if (callback.apply(getURL())) { + return; + } + } catch (Throwable t) { + Log.w(TAG, t.getMessage(), t); + } } - } - super.onClick(widget); + super.onClick(widget); + }).start(); } } \ No newline at end of file