Ensure that internal notes links are not persisted

This commit is contained in:
Stefan Niedermann 2021-01-05 12:35:58 +01:00
parent c92f9d251f
commit dd8cf671c6
8 changed files with 25 additions and 234 deletions

View file

@ -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();

View file

@ -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, ""));
}
}

View file

@ -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<String> existingNoteRemoteIds;
public NoteLinksProcessor(Set<String> existingNoteRemoteIds) {
this.existingNoteRemoteIds = existingNoteRemoteIds;
}
/**
* Replaces all links to other notes of the form `[<link-text>](<note-file-id>)`
* 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<String> existingNoteRemoteIds) {
Pattern noteLinkCandidates = Pattern.compile(linksThatLookLikeNoteLinksRegEx);
Matcher matcher = noteLinkCandidates.matcher(markdown);
Set<String> 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));
}
}

View file

@ -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);
}

View file

@ -1,12 +0,0 @@
package it.niedermann.owncloud.notes.shared.util.text;
import java.util.LinkedList;
public class TextProcessorChain extends LinkedList<TextProcessor> {
public String apply(String s) {
for (TextProcessor textProcessor : this) {
s = textProcessor.process(s);
}
return s;
}
}

View file

@ -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<String> 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);
}
}

View file

@ -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<String> 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();
}
}
}

View file

@ -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<Function<String, Boolean>> onLinkClickCallbacks;
@ -19,11 +22,17 @@ public class InterceptedURLSpan extends URLSpan {
@Override
public void onClick(View widget) {
for (Function<String, Boolean> callback : onLinkClickCallbacks) {
if (callback.apply(getURL())) {
return;
new Thread(() -> {
for (Function<String, Boolean> callback : onLinkClickCallbacks) {
try {
if (callback.apply(getURL())) {
return;
}
} catch (Throwable t) {
Log.w(TAG, t.getMessage(), t);
}
}
}
super.onClick(widget);
super.onClick(widget);
}).start();
}
}