From 0a39edcf33245fe7c7e8ec1e868d0e3091611d37 Mon Sep 17 00:00:00 2001 From: Tue Nguyen Date: Wed, 13 Apr 2022 10:36:41 -0400 Subject: [PATCH] - Change any invalid/flagged feed code to use supabase - Fix test accordingly --- docker/development.yml | 2 +- src/api/parser/env.local | 7 +- src/api/parser/src/data/feed.js | 10 ++- src/api/parser/src/index.js | 2 +- .../parser/src/utils/__mocks__/supabase.js | 41 +++++++++++ src/api/parser/src/utils/storage.js | 40 ++--------- src/api/parser/src/utils/supabase.js | 72 +++++++++++++++++++ src/api/parser/test/feed.test.js | 31 +++----- src/api/parser/test/storage.test.js | 46 ++++++------ 9 files changed, 165 insertions(+), 86 deletions(-) create mode 100644 src/api/parser/src/utils/__mocks__/supabase.js diff --git a/docker/development.yml b/docker/development.yml index aa7aec884d..4ea02b87cc 100644 --- a/docker/development.yml +++ b/docker/development.yml @@ -94,7 +94,7 @@ services: # In development and testing, the SSO service needs to contact the Supabase # service directly via Docker vs through the http://localhost/v1/supabase domain. # Using staging database - - SUPABASE_URL=https://dev.api.telescope.cdot.systems/v1/supabase + - SUPABASE_URL=http://kong:8000 depends_on: - elasticsearch - traefik diff --git a/src/api/parser/env.local b/src/api/parser/env.local index 813711ac1b..5e417c1675 100644 --- a/src/api/parser/env.local +++ b/src/api/parser/env.local @@ -38,6 +38,7 @@ PARSER_PORT = 10000 ################################################################################ # Supabase Secrets -# Using staging database -#SUPABASE_URL=http://localhost/v1/supabase -SUPABASE_URL=https://dev.supabase.telescope.cdot.systems/ +# Staging database +#SUPABASE_URL=https://dev.supabase.telescope.cdot.systems/ +SUPABASE_URL=http://localhost/v1/supabase + diff --git a/src/api/parser/src/data/feed.js b/src/api/parser/src/data/feed.js index 564f6007f3..1ad59c3f3c 100644 --- a/src/api/parser/src/data/feed.js +++ b/src/api/parser/src/data/feed.js @@ -6,17 +6,21 @@ const { getFeeds, addFeed, removeFeed, - setInvalidFeed, - isInvalid, setDelayedFeed, isDelayed, removePost, getPost, getPosts, +} = require('../utils/storage'); + +const { + isInvalid, + setInvalidFeed, getFlaggedFeeds, setFlaggedFeed, unsetFlaggedFeed, -} = require('../utils/storage'); +} = require('../utils/supabase'); + const { deletePost } = require('../utils/indexer'); const urlToId = (url) => hash(normalizeUrl(url)); diff --git a/src/api/parser/src/index.js b/src/api/parser/src/index.js index 2ec36bde5e..e9676b6783 100644 --- a/src/api/parser/src/index.js +++ b/src/api/parser/src/index.js @@ -18,7 +18,7 @@ feedQueue.on('drained', loadFeedsIntoQueue); */ feedQueue.on('failed', (job, err) => invalidateFeed(job.data.id, err).catch((error) => - logger.error({ error }, 'Unable to invalidate feed') + logger.error({ error }, `Unable to invalidate feed ${job.data.id}`) ) ); diff --git a/src/api/parser/src/utils/__mocks__/supabase.js b/src/api/parser/src/utils/__mocks__/supabase.js new file mode 100644 index 0000000000..a533a2b1d1 --- /dev/null +++ b/src/api/parser/src/utils/__mocks__/supabase.js @@ -0,0 +1,41 @@ +let feeds = []; + +module.exports = { + __setMockFeeds: (redisFeeds) => { + const mockFeeds = redisFeeds.map((feed) => { + return { + id: feed.id, + flagged: false, + }; + }); + feeds = mockFeeds; + }, + // Flagged feed related functions + getAllFeeds: jest.fn().mockImplementation(() => Promise.resolve(feeds)), + setFlaggedFeed: jest.fn().mockImplementation((id) => { + feeds.forEach((feed) => { + if (feed.id === id) { + feed.flagged = true; + } + }); + return Promise.resolve(); + }), + unsetFlaggedFeed: jest.fn().mockImplementation((id) => { + feeds.forEach((feed) => { + if (feed.id === id) { + feed.flagged = false; + } + }); + return Promise.resolve(); + }), + + getFlaggedFeeds: jest.fn().mockImplementation(() => { + const flaggedFeedIds = feeds.filter((feed) => feed.flagged).map((feed) => feed.id); + return Promise.resolve(flaggedFeedIds); + }), + isFlagged: jest.fn().mockImplementation((id) => { + const targetFeed = feeds.filter((feed) => feed.id === id)[0]; + + return Promise.resolve(targetFeed.flagged); + }), +}; diff --git a/src/api/parser/src/utils/storage.js b/src/api/parser/src/utils/storage.js index 7fa975af2d..45767da5a3 100644 --- a/src/api/parser/src/utils/storage.js +++ b/src/api/parser/src/utils/storage.js @@ -1,26 +1,24 @@ const { logger, Redis } = require('@senecacdot/satellite'); +const { isFlagged } = require('./supabase'); const redis = Redis(); // Redis Keys const feedsKey = 't:feeds'; -const flaggedFeedsKey = 't:feeds:flagged'; const postsKey = 't:posts'; // Namespaces const feedNamespace = 't:feed:'; const postNamespace = 't:post:'; // Suffixes -const invalidSuffix = ':invalid'; const delayedSuffix = ':delayed'; // "6Xoj0UXOW3" to "t:post:6Xoj0UXOW3" const createPostKey = (id) => postNamespace.concat(id); // "NirlSYranl" to "t:feed:NirlSYranl" const createFeedKey = (id) => feedNamespace.concat(id); -// "NirlSYranl" to "t:feed:NirlSYranl:invalid" -const createInvalidFeedKey = (id) => createFeedKey(id).concat(invalidSuffix); + // "NirlSYranl" to "t:feed:NirlSYranl:delayed" const createDelayedFeedKey = (id) => createFeedKey(id).concat(delayedSuffix); @@ -50,7 +48,7 @@ module.exports = { addFeed: async (feed) => { // Check if feed being added already exists in flagged feeds set // If it is, do nothing - if (await redis.sismember(flaggedFeedsKey, feed.id)) return; + if (await isFlagged(feed.id)) return; const key = createFeedKey(feed.id); await redis @@ -78,20 +76,6 @@ module.exports = { getFeeds: () => redis.smembers(feedsKey), - getInvalidFeeds: async () => { - const invalidKeys = await getFeedKeysUsingScanStream(`${feedNamespace}*${invalidSuffix}`); - return Promise.all( - invalidKeys.map(async (key) => { - const reason = await redis.get(key); - const id = key.replace(feedNamespace, '').replace(invalidSuffix, ''); - return { - id, - reason: reason.replace(/\n/g, ' '), - }; - }) - ); - }, - getDelayedFeeds: async () => { const delayedKeys = await getFeedKeysUsingScanStream(`${feedNamespace}*${delayedSuffix}`); return delayedKeys.map((key) => { @@ -102,31 +86,21 @@ module.exports = { }); }, - getFlaggedFeeds: () => redis.smembers(flaggedFeedsKey), - getFeed: (id) => redis.hgetall(feedNamespace.concat(id)), getFeedsCount: () => redis.scard(feedsKey), - setInvalidFeed: (id, reason) => { - const key = createInvalidFeedKey(id); - const sevenDaysInSeconds = 60 * 60 * 24 * 7; // Expire after 7 days - return redis.set(key, reason, 'EX', sevenDaysInSeconds); - }, - /** * Removes a feed entry from redis * @param id id of feed to be removed */ removeFeed: async (id) => { const key = createFeedKey(id); - // Checks which set the feed is currently in - const redisKey = (await redis.sismember(feedsKey, id)) ? feedsKey : flaggedFeedsKey; try { await redis .multi() .hdel(key, 'id', 'author', 'url', 'user', 'link', 'etag', 'lastModified') - .srem(redisKey, id) + .srem(feedsKey, id) .exec(); } catch (error) { logger.error({ error }, `Error removing Feed ${id} from Redis`); @@ -134,12 +108,6 @@ module.exports = { } }, - setFlaggedFeed: (id) => redis.smove(feedsKey, flaggedFeedsKey, id), - - unsetFlaggedFeed: (id) => redis.smove(flaggedFeedsKey, feedsKey, id), - - isInvalid: (id) => redis.exists(createInvalidFeedKey(id)), - setDelayedFeed: (id, seconds) => redis.set(createDelayedFeedKey(id), seconds, 'EX', seconds), isDelayed: (id) => redis.exists(createDelayedFeedKey(id)), diff --git a/src/api/parser/src/utils/supabase.js b/src/api/parser/src/utils/supabase.js index 47398ce018..963e071b18 100644 --- a/src/api/parser/src/utils/supabase.js +++ b/src/api/parser/src/utils/supabase.js @@ -27,4 +27,76 @@ module.exports = { url: feed.url, })); }, + + // Invalid feed related functions + async setInvalidFeed(id, reason) { + const { error } = await supabase.from('feeds').update({ invalid: true }).eq('id', id); + + if (error) { + logger.error({ error }); + throw Error(error.message, `can't invalidate feed ${id} in supabase`); + } + }, + + async getInvalidFeeds() { + const { data: invalidFeeds, error } = await supabase.from('feeds').select().eq('invalid', true); + if (error) { + logger.error({ error }); + throw Error(error.message, "can't fetch invalid feeds in supabase"); + } + return invalidFeeds; + }, + async isInvalid(id) { + const { data: invalidFeed, error } = await supabase + .from('feeds') + .select('invalid') + .eq('id', id) + .limit(1); + + if (error) { + logger.error({ error }); + throw Error(error.message, `can't fetch feed ${id} from supabase`); + } + return invalidFeed.invalid; + }, + + // Flagged feed related functions + async setFlaggedFeed(id) { + const { error } = await supabase.from('feeds').update({ flagged: true }).eq('id', id); + + if (error) { + logger.error({ error }); + throw Error(error.message, `can't flag feed ${id} in supabase`); + } + }, + async unsetFlaggedFeed(id) { + const { error } = await supabase.from('feeds').update({ flagged: false }).eq('id', id); + + if (error) { + logger.error({ error }); + throw Error(error.message, `can't unflag feed ${id} in supabase`); + } + }, + async getFlaggedFeeds() { + const { data: flaggedFeeds, error } = await supabase.from('feeds').select().eq('flagged', true); + + if (error) { + logger.error({ error }); + throw Error(error.message, `can't flagged feeds from supabase`); + } + return flaggedFeeds.map((feed) => feed.id); + }, + async isFlagged(id) { + const { data: flaggedFeed, error } = await supabase + .from('feeds') + .select('flagged') + .eq('id', id) + .limit(1); + + if (error) { + logger.error({ error }); + throw Error(error.message, `can't fetch feed ${id} from supabase`); + } + return flaggedFeed.flagged; + }, }; diff --git a/src/api/parser/test/feed.test.js b/src/api/parser/test/feed.test.js index a8d6ef77e8..dd3a0fca83 100644 --- a/src/api/parser/test/feed.test.js +++ b/src/api/parser/test/feed.test.js @@ -7,6 +7,9 @@ const { search } = require('../src/utils/indexer'); const urlToId = (url) => hash(normalizeUrl(url)); +jest.mock('../src/utils/supabase'); +const { __setMockFeeds } = require('../src/utils/supabase'); + describe('Post data class tests', () => { const data = { author: 'Post Author', @@ -262,54 +265,42 @@ describe('Post data class tests', () => { // Teardown removing the added feed await Promise.all([await feed.delete(), await feed2.delete(), await feed3.delete()]); }); - - test('Flagged feed should appear in t:feeds:flagged and not t:feeds', async () => { + }); + describe('Set and get flagged feeds objects from Supabase', () => { + test('Flagged feed should be set and retrieved correctly', async () => { const feedData = new Feed(data.author, data.url, data.user, data.link); const feedData2 = new Feed(data2.author, data2.url, data2.user, data2.link); const feedData3 = new Feed(data3.author, data3.url, data3.user, data3.link); + __setMockFeeds([feedData, feedData2, feedData3]); // Check all three feeds are created const feed = await Feed.byId(await Feed.create(feedData)); const feed2 = await Feed.byId(await Feed.create(feedData2)); const feed3 = await Feed.byId(await Feed.create(feedData3)); - let unFlaggedFeeds = await Feed.all(); + const unFlaggedFeeds = await Feed.all(); expect(unFlaggedFeeds.length).toBe(3); // Test flag() await Promise.all([feed.flag(), feed2.flag()]); - unFlaggedFeeds = await Feed.all(); - expect(unFlaggedFeeds.length).toBe(1); - let flaggedFeeds = await Feed.flagged(); expect(flaggedFeeds.length).toBe(2); - // Feed should not appear in unflagged set if Feed is flagged and added again - await Feed.create(feedData); - unFlaggedFeeds = await Feed.all(); - expect(unFlaggedFeeds.length).toBe(1); - // Flagged feeds should have same data as feed + feed2 expect(flaggedFeeds.some((flaggedFeed) => flaggedFeed.id === feed.id)).toBe(true); expect(flaggedFeeds.some((flaggedFeed) => flaggedFeed.id === feed2.id)).toBe(true); // Test unflag(); await feed2.unflag(); - unFlaggedFeeds = await Feed.all(); - expect(unFlaggedFeeds.length).toBe(2); - flaggedFeeds = await Feed.flagged(); expect(flaggedFeeds.length).toBe(1); - // Testing delete() as part of teardown, feed should be removed from t:feeds:flagged - await feed.delete(); + await feed.unflag(); flaggedFeeds = await Feed.flagged(); expect(flaggedFeeds.length).toBe(0); - await feed2.delete(); - await feed3.delete(); - // Testing whether removing an already removed feed will error - await feed.delete(); + // Testing delete() as part of teardown, feed should be removed from t:feeds:flagged + await Promise.all([await feed.delete(), await feed2.delete(), await feed3.delete()]); }); }); }); diff --git a/src/api/parser/test/storage.test.js b/src/api/parser/test/storage.test.js index cbcc0ce20c..e69d150472 100644 --- a/src/api/parser/test/storage.test.js +++ b/src/api/parser/test/storage.test.js @@ -3,20 +3,19 @@ const { addFeed, getFeed, getFeeds, - getFlaggedFeeds, getFeedsCount, removeFeed, - setFlaggedFeed, - unsetFlaggedFeed, addPost, getPost, getPosts, getPostsCount, removePost, } = require('../src/utils/storage'); - const Feed = require('../src/data/feed'); +jest.mock('../src/utils/supabase'); +const { __setMockFeeds } = require('../src/utils/__mocks__/supabase'); + describe('Storage tests for feeds', () => { const feed1 = new Feed('James Smith', 'http://seneca.co/jsmith', 'user'); const feed2 = new Feed('James Smith 2', 'http://seneca.co/jsmith/2', 'user'); @@ -30,7 +29,10 @@ describe('Storage tests for feeds', () => { 'last-modified' ); - beforeAll(() => Promise.all([addFeed(feed1), addFeed(feed2), addFeed(feed3), addFeed(feed4)])); + beforeAll(async () => { + __setMockFeeds([feed1, feed2, feed3, feed4]); + await Promise.all([addFeed(feed1), addFeed(feed2), addFeed(feed3), addFeed(feed4)]); + }); it('should allow retrieving a feed by id after inserting', async () => { const feed = await getFeed(feed1.id); @@ -86,23 +88,23 @@ describe('Storage tests for feeds', () => { expect(feeds.includes(feed.id)).toBe(false); }); - it('feed3 should appear in flaggedFeed set after being flagged', async () => { - const feed = await getFeed(feed3.id); - await setFlaggedFeed(feed3.id); - const feeds = await getFeeds(); - const flaggedFeeds = await getFlaggedFeeds(); - expect(flaggedFeeds.includes(feed.id)).toBe(true); - expect(feeds.includes(feed.id)).toBe(false); - }); - - it('feed3 should not appear in flaggedFeed set after being unflagged', async () => { - const feed = await getFeed(feed3.id); - await unsetFlaggedFeed(feed3.id); - const feeds = await getFeeds(); - const flaggedFeeds = await getFlaggedFeeds(); - expect(feeds.includes(feed.id)).toBe(true); - expect(flaggedFeeds.includes(feed.id)).toBe(false); - }); + // it('feed3 should appear in flaggedFeed set after being flagged', async () => { + // const feed = await getFeed(feed3.id); + // await setFlaggedFeed(feed3.id); + // const feeds = await getFeeds(); + // const flaggedFeeds = await getFlaggedFeeds(); + // expect(flaggedFeeds.includes(feed.id)).toBe(true); + // expect(feeds.includes(feed.id)).toBe(false); + // }); + + // it('feed3 should not appear in flaggedFeed set after being unflagged', async () => { + // const feed = await getFeed(feed3.id); + // await unsetFlaggedFeed(feed3.id); + // const feeds = await getFeeds(); + // const flaggedFeeds = await getFlaggedFeeds(); + // expect(feeds.includes(feed.id)).toBe(true); + // expect(flaggedFeeds.includes(feed.id)).toBe(false); + // }); }); describe('Storage tests for posts', () => {