feat: create post www dedup (#3946)

Author: capJavert

__tests__/posts.ts

@@ -53,7 +53,7 @@ import {
   postTagsFixture,
   videoPostsFixture,
 } from './fixture/post';
-import { DataSource, DeepPartial, IsNull, Not } from 'typeorm';
+import { DataSource, DeepPartial, In, IsNull, Not } from 'typeorm';
 import createOrGetConnection from '../src/db';
 import {
   createSquadWelcomePost,
@@ -3958,6 +3958,52 @@ describe('mutation createPostInMultipleSources', () => {
       expect(moderationItem.sharedPostId).toBe(existingArticle.id);
       expect(userSourcePost.sharedPostId).toBe(existingArticle.id);
     });
+
+    it('should share existing post when url differs only by www', async () => {
+      loggedUser = '1';
+      await con.getRepository(ArticlePost).save({
+        id: 'pmswww',
+        shortId: 'pmswww',
+        url: 'http://www.multisource-www.com/a',
+        canonicalUrl: 'http://www.multisource-www.com/a',
+        sourceId: 'a',
+        visible: true,
+        createdAt: new Date(),
+        type: PostType.Article,
+        origin: PostOrigin.Squad,
+      });
+
+      const res = await client.mutate(MUTATION, {
+        variables: {
+          ...freeformParams,
+          externalLink: 'http://multisource-www.com/a',
+        },
+      });
+
+      expect(res.errors).toBeFalsy();
+      const [first, second, third] = res.data.createPostInMultipleSources;
+
+      const [post, moderationItem, userSourcePost] = await Promise.all([
+        con.getRepository(SharePost).findOneByOrFail({ id: first.id }),
+        con.getRepository(SourcePostModeration).findOneOrFail({
+          select: ['sharedPostId'],
+          where: { id: second.id },
+        }),
+        con.getRepository(SharePost).findOneByOrFail({ id: third.id }),
+      ]);
+
+      expect(post.sharedPostId).toBe('pmswww');
+      expect(moderationItem.sharedPostId).toBe('pmswww');
+      expect(userSourcePost.sharedPostId).toBe('pmswww');
+
+      const articlePosts = await con.getRepository(ArticlePost).findBy({
+        url: In([
+          'http://multisource-www.com/a',
+          'http://www.multisource-www.com/a',
+        ]),
+      });
+      expect(articlePosts).toHaveLength(1);
+    });
   });
 
   describe('poll post creation', () => {
@@ -4587,6 +4633,25 @@ describe('mutation submitExternalLink', () => {
     expect(sharedPost.visible).toEqual(true);
   });
 
+  it('should share existing post to squad when url differs only by www', async () => {
+    loggedUser = '1';
+    const res = await client.mutate(MUTATION, {
+      variables: { ...variables, url: 'http://www.p6.com' },
+    });
+    expect(res.errors).toBeFalsy();
+
+    const articlePosts = await con
+      .getRepository(ArticlePost)
+      .findBy({ url: In(['http://p6.com', 'http://www.p6.com']) });
+    expect(articlePosts).toHaveLength(1);
+    expect(articlePosts[0].id).toEqual('p6');
+
+    const sharedPost = await con
+      .getRepository(SharePost)
+      .findOneByOrFail({ sharedPostId: 'p6' });
+    expect(sharedPost.authorId).toEqual('1');
+  });
+
   it('should trigger content request when sharing existing post without title and summary', async () => {
     loggedUser = '1';
 

__tests__/routes/private/rpc.ts

@@ -7,7 +7,7 @@ import {
   SourceType,
 } from '../../../src/entity';
 import { sourcesFixture } from '../../fixture/source';
-import { DataSource } from 'typeorm';
+import { DataSource, In } from 'typeorm';
 import createOrGetConnection from '../../../src/db';
 import {
   PageIngestion,
@@ -174,6 +174,98 @@ describe('PostService', () => {
     });
   });
 
+  it('should deduplicate www against non www urls', async () => {
+    const { postId } = await mockClient.create(
+      {
+        url: 'http://www.example.com/service/1',
+        sourceId: 'a',
+      },
+      defaultClientAuthOptions,
+    );
+
+    const result = await mockClient.create(
+      {
+        url: 'http://example.com/service/1',
+        sourceId: 'a',
+      },
+      defaultClientAuthOptions,
+    );
+
+    expect(result).toEqual({
+      postId,
+      url: 'http://www.example.com/service/1',
+    });
+
+    const posts = await con.getRepository(ArticlePost).findBy({
+      url: In([
+        'http://example.com/service/1',
+        'http://www.example.com/service/1',
+      ]),
+    });
+    expect(posts).toHaveLength(1);
+    expect(posts[0].url).toEqual('http://www.example.com/service/1');
+  });
+
+  it('should deduplicate non www against www urls', async () => {
+    const { postId } = await mockClient.create(
+      {
+        url: 'http://example.com/service/2',
+        sourceId: 'a',
+      },
+      defaultClientAuthOptions,
+    );
+
+    const result = await mockClient.create(
+      {
+        url: 'http://www.example.com/service/2',
+        sourceId: 'a',
+      },
+      defaultClientAuthOptions,
+    );
+
+    expect(result).toEqual({
+      postId,
+      url: 'http://example.com/service/2',
+    });
+
+    const posts = await con.getRepository(ArticlePost).findBy({
+      url: In([
+        'http://example.com/service/2',
+        'http://www.example.com/service/2',
+      ]),
+    });
+    expect(posts).toHaveLength(1);
+    expect(posts[0].url).toEqual('http://example.com/service/2');
+  });
+
+  it('should not deduplicate other subdomain urls', async () => {
+    const first = await mockClient.create(
+      {
+        url: 'http://example.com/service/3',
+        sourceId: 'a',
+      },
+      defaultClientAuthOptions,
+    );
+
+    const second = await mockClient.create(
+      {
+        url: 'http://blog.example.com/service/3',
+        sourceId: 'a',
+      },
+      defaultClientAuthOptions,
+    );
+
+    expect(second.postId).not.toEqual(first.postId);
+
+    const posts = await con.getRepository(ArticlePost).findBy({
+      url: In([
+        'http://example.com/service/3',
+        'http://blog.example.com/service/3',
+      ]),
+    });
+    expect(posts).toHaveLength(2);
+  });
+
   it('should throw on invalid source', async () => {
     await expect(
       mockClient.create(

__tests__/workers/cdc/primary.ts

@@ -6333,6 +6333,38 @@ describe('source_post_moderation', () => {
       expect(list.length).toEqual(3); // to ensure nothing new was created other than the share post
     });
 
+    it('should create share post from external link that differs only by www', async () => {
+      const repo = con.getRepository(Post);
+      const existing = await repo.save({
+        ...postsFixture[0],
+        sourceId: 'c',
+        url: 'https://www.daily.dev/blog-post/sauron',
+        canonicalUrl: 'https://www.daily.dev/blog-post/sauron',
+      });
+      const before = await repo.find();
+      expect(before.length).toEqual(2);
+      const after = {
+        ...base,
+        sourceId: 'a',
+        type: PostType.Share,
+        status: SourcePostModerationStatus.Approved,
+        title: 'Test',
+        content: '# Sample',
+        contentHtml: '# Sample',
+        externalLink: 'https://daily.dev/blog-post/sauron',
+      };
+      await mockUpdate(after);
+      const share = (await repo.findOneBy({
+        sourceId: 'a',
+      })) as SharePost;
+      expect(share).toBeTruthy();
+      expect(share.type).toEqual(PostType.Share);
+      expect(share.sharedPostId).toEqual(existing.id);
+
+      const list = await repo.find();
+      expect(list.length).toEqual(3); // deduped to the www variant, only the share was added
+    });
+
     it('should update the content if post id is present', async () => {
       const repo = con.getRepository(Post);
       await saveFixtures(con, Post, [postsFixture[0]]);

src/common/links.ts

@@ -84,6 +84,19 @@ const subtractDomain = (url: string): string | null => {
   return matches && matches[1];
 };
 
+// The same article is often reachable under both www. and non-www. hosts.
+// Return the url alongside its www.-toggled form so dedup lookups can match
+// either without rewriting the url we persist.
+export const getUrlWwwVariants = (url: string): string[] => {
+  const match = url?.match(/^(https?:\/\/)(www\.)?(.*)$/i);
+  if (!match) {
+    return [url];
+  }
+
+  const [, scheme, www, rest] = match;
+  return [url, www ? `${scheme}${rest}` : `${scheme}www.${rest}`];
+};
+
 export const standardizeURL = (
   inputUrl: string,
 ): { url: string; canonicalUrl: string } => {

src/common/post.ts

@@ -25,7 +25,7 @@ import {
   WelcomePost,
 } from '../entity';
 import { ForbiddenError, ValidationError } from 'apollo-server-errors';
-import { isValidHttpUrl, standardizeURL } from './links';
+import { getUrlWwwVariants, isValidHttpUrl, standardizeURL } from './links';
 import {
   findMarkdownTag,
   markdown,
@@ -915,7 +915,10 @@ export const getExistingPost = async (
   manager
     .createQueryBuilder(Post, 'post')
     .select(['post.id', 'post.deleted', 'post.visible'])
-    .where([{ canonicalUrl: canonicalUrl }, { url: url }])
+    .where([
+      { canonicalUrl: In(getUrlWwwVariants(canonicalUrl)) },
+      { url: In(getUrlWwwVariants(url)) },
+    ])
     .getOne();
 
 const extractPostIdOrSlugFromUrl = (url: string): string | undefined => {
@@ -963,8 +966,8 @@ export const findPostByUrl = async <T extends keyof ArticlePost>(
     ]);
   } else {
     queryBuilder = queryBuilder.andWhere([
-      { canonicalUrl: canonicalUrl },
-      { url: cleanUrl },
+      { canonicalUrl: In(getUrlWwwVariants(canonicalUrl)) },
+      { url: In(getUrlWwwVariants(cleanUrl)) },
     ]);
   }
 

src/routes/private/rpc.ts

@@ -9,7 +9,11 @@ import {
 } from '@dailydotdev/schema';
 import { ArticlePost, SourceRequest } from '../../entity';
 import { baseRpcContext } from '../../common/connectRpc';
-import { isValidHttpUrl, standardizeURL } from '../../common/links';
+import {
+  getUrlWwwVariants,
+  isValidHttpUrl,
+  standardizeURL,
+} from '../../common/links';
 import createOrGetConnection from '../../db';
 import { TypeOrmError, TypeORMQueryFailedError } from '../../errors';
 import { generateShortId } from '../../ids';
@@ -47,6 +51,46 @@ const getDuplicatePost = async ({
   }
 };
 
+/**
+ * Check www or non www variant of url to prevent duplicates
+ *
+ * Unique index only covers exact match
+ *
+ * @param {{
+ *   req: CreatePostRequest;
+ *   con: DataSource;
+ * }} {
+ *   req,
+ *   con,
+ * }
+ * @return {*}  {(Promise<CreatePostResponse | undefined>)}
+ */
+const getWwwVariantPost = async ({
+  req,
+  con,
+}: {
+  req: CreatePostRequest;
+  con: DataSource;
+}): Promise<CreatePostResponse | undefined> => {
+  const variantUrl = getUrlWwwVariants(req.url).find((url) => url !== req.url);
+  if (req.yggdrasilId || !variantUrl) {
+    return undefined;
+  }
+
+  const existingPost = await con
+    .getRepository(ArticlePost)
+    .findOneBy({ url: variantUrl });
+
+  if (!existingPost) {
+    return undefined;
+  }
+
+  return new CreatePostResponse({
+    postId: existingPost.id,
+    url: existingPost.url || undefined,
+  });
+};
+
 const validateCreatePostRequest = (req: CreatePostRequest): never | void => {
   // collection-style sources (collections + trends) are yggdrasil-generated
   // and have no URL; they require a yggdrasil id instead.
@@ -80,6 +124,11 @@ export default function (router: ConnectRouter) {
 
       validateCreatePostRequest(req);
 
+      const wwwVariantPost = await getWwwVariantPost({ req, con });
+      if (wwwVariantPost) {
+        return wwwVariantPost;
+      }
+
       const postId = await generateShortId();
       const postEntity = con.getRepository(ArticlePost).create({
         ...req,