Fix non-rendered previews when differ by trailing slash.

This commit is contained in:
Alan Evans 2021-01-05 17:07:18 -04:00
parent 86542febf9
commit 64312f9c7f
4 changed files with 137 additions and 12 deletions

View file

@ -57,7 +57,6 @@ import org.thoughtcrime.securesms.jobmanager.Job;
import org.thoughtcrime.securesms.jobmanager.JobManager;
import org.thoughtcrime.securesms.jobmanager.impl.NetworkConstraint;
import org.thoughtcrime.securesms.keyvalue.SignalStore;
import org.thoughtcrime.securesms.linkpreview.Link;
import org.thoughtcrime.securesms.linkpreview.LinkPreview;
import org.thoughtcrime.securesms.linkpreview.LinkPreviewUtil;
import org.thoughtcrime.securesms.mms.IncomingMediaMessage;
@ -1797,9 +1796,10 @@ public final class PushProcessMessageJob extends BaseJob {
}
private Optional<List<LinkPreview>> getLinkPreviews(Optional<List<Preview>> previews, @NonNull String message) {
if (!previews.isPresent()) return Optional.absent();
if (!previews.isPresent() || previews.get().isEmpty()) return Optional.absent();
List<LinkPreview> linkPreviews = new ArrayList<>(previews.get().size());
List<LinkPreview> linkPreviews = new ArrayList<>(previews.get().size());
LinkPreviewUtil.Links urlsInMessage = LinkPreviewUtil.findValidPreviewUrls(message);
for (Preview preview : previews.get()) {
Optional<Attachment> thumbnail = PointerAttachment.forPointer(preview.getImage());
@ -1807,7 +1807,7 @@ public final class PushProcessMessageJob extends BaseJob {
Optional<String> title = Optional.fromNullable(preview.getTitle());
Optional<String> description = Optional.fromNullable(preview.getDescription());
boolean hasTitle = !TextUtils.isEmpty(title.or(""));
boolean presentInBody = url.isPresent() && Stream.of(LinkPreviewUtil.findValidPreviewUrls(message)).map(Link::getUrl).collect(Collectors.toSet()).contains(url.get());
boolean presentInBody = url.isPresent() && urlsInMessage.containsUrl(url.get());
boolean validDomain = url.isPresent() && LinkPreviewUtil.isValidPreviewUrl(url.get());
if (hasTitle && presentInBody && validDomain) {

View file

@ -11,6 +11,7 @@ import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;
import com.annimon.stream.Collectors;
import com.annimon.stream.Stream;
import org.signal.core.util.logging.Log;
@ -50,18 +51,18 @@ public final class LinkPreviewUtil {
/**
* @return All whitelisted URLs in the source text.
*/
public static @NonNull List<Link> findValidPreviewUrls(@NonNull String text) {
public static @NonNull Links findValidPreviewUrls(@NonNull String text) {
SpannableString spannable = new SpannableString(text);
boolean found = Linkify.addLinks(spannable, Linkify.WEB_URLS);
if (!found) {
return Collections.emptyList();
return Links.EMPTY;
}
return Stream.of(spannable.getSpans(0, spannable.length(), URLSpan.class))
.map(span -> new Link(span.getURL(), spannable.getSpanStart(span)))
.filter(link -> isValidPreviewUrl(link.getUrl()))
.toList();
return new Links(Stream.of(spannable.getSpans(0, spannable.length(), URLSpan.class))
.map(span -> new Link(span.getURL(), spannable.getSpanStart(span)))
.filter(link -> isValidPreviewUrl(link.getUrl()))
.toList());
}
/**
@ -217,4 +218,39 @@ public final class LinkPreviewUtil {
public interface HtmlDecoder {
@NonNull String fromEncoded(@NonNull String html);
}
public static class Links {
static final Links EMPTY = new Links(Collections.emptyList());
private final List<Link> links;
private final Set<String> urlSet;
private Links(@NonNull List<Link> links) {
this.links = links;
this.urlSet = Stream.of(links)
.map(link -> trimTrailingSlash(link.getUrl()))
.collect(Collectors.toSet());
}
public Optional<Link> findFirst() {
return links.isEmpty() ? Optional.absent()
: Optional.of(links.get(0));
}
/**
* Slightly forgiving comparison where it will ignore trailing '/' on the supplied url.
*/
public boolean containsUrl(@NonNull String url) {
return urlSet.contains(trimTrailingSlash(url));
}
private @NonNull String trimTrailingSlash(@NonNull String url) {
return url.endsWith("/") ? url.substring(0, url.length() - 1)
: url;
}
public int size() {
return links.size();
}
}
}

View file

@ -75,8 +75,8 @@ public class LinkPreviewViewModel extends ViewModel {
return;
}
List<Link> links = LinkPreviewUtil.findValidPreviewUrls(text);
Optional<Link> link = links.isEmpty() ? Optional.absent() : Optional.of(links.get(0));
Optional<Link> link = LinkPreviewUtil.findValidPreviewUrls(text)
.findFirst();
if (link.isPresent() && link.get().getUrl().equals(activeUrl)) {
return;

View file

@ -0,0 +1,89 @@
package org.thoughtcrime.securesms.linkpreview;
import android.app.Application;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.robolectric.RobolectricTestRunner;
import org.robolectric.annotation.Config;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
@RunWith(RobolectricTestRunner.class)
@Config(manifest = Config.NONE, application = Application.class)
public final class LinkPreviewUtilTest_findValidPreviewUrls {
@Test
public void no_links() {
LinkPreviewUtil.Links links = LinkPreviewUtil.findValidPreviewUrls("No links");
assertEquals(0, links.size());
assertSame(LinkPreviewUtil.Links.EMPTY, links);
}
@Test
public void contains_a_link() {
LinkPreviewUtil.Links links = LinkPreviewUtil.findValidPreviewUrls("https://signal.org");
assertEquals(1, links.size());
assertTrue(links.containsUrl("https://signal.org"));
}
@Test
public void does_not_contain_link() {
LinkPreviewUtil.Links links = LinkPreviewUtil.findValidPreviewUrls("https://signal.org");
assertEquals(1, links.size());
assertFalse(links.containsUrl("https://signal.org/page"));
}
@Test
public void contains_two_links() {
LinkPreviewUtil.Links links = LinkPreviewUtil.findValidPreviewUrls("Links https://signal.org https://android.com");
assertEquals(2, links.size());
assertTrue(links.containsUrl("https://signal.org"));
assertTrue(links.containsUrl("https://android.com"));
}
@Test
public void link_trailing_slash_insensitivity() {
LinkPreviewUtil.Links links = LinkPreviewUtil.findValidPreviewUrls("Links https://signal.org/ https://android.com");
assertEquals(2, links.size());
assertTrue(links.containsUrl("https://signal.org"));
assertTrue(links.containsUrl("https://android.com"));
assertTrue(links.containsUrl("https://signal.org/"));
assertTrue(links.containsUrl("https://android.com/"));
}
@Test
public void link_trailing_slash_insensitivity_where_last_url_has_trailing_slash() {
LinkPreviewUtil.Links links = LinkPreviewUtil.findValidPreviewUrls("Links https://signal.org https://android.com/");
assertEquals(2, links.size());
assertTrue(links.containsUrl("https://signal.org"));
assertTrue(links.containsUrl("https://android.com"));
assertTrue(links.containsUrl("https://signal.org/"));
assertTrue(links.containsUrl("https://android.com/"));
}
@Test
public void multiple_trailing_slashes_are_not_stripped() {
LinkPreviewUtil.Links links = LinkPreviewUtil.findValidPreviewUrls("Link https://android.com/");
assertEquals(1, links.size());
assertTrue(links.containsUrl("https://android.com"));
assertTrue(links.containsUrl("https://android.com/"));
assertFalse(links.containsUrl("https://android.com//"));
}
}