Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow feed URL to be given during sign up #3768

Merged
merged 1 commit into from
Dec 9, 2022
Merged

Allow feed URL to be given during sign up #3768

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/api/feed-discovery/src/middleware.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const { logger, createError } = require('@senecacdot/satellite');

const { isTwitchUrl, toTwitchFeedUrl, getBlogBody, getFeedUrls } = require('./util');
const { isTwitchUrl, toTwitchFeedUrl, isFeedUrl, getBlogBody, getFeedUrls } = require('./util');

// A middleware to ensure we get an array
module.exports.checkForArray = () => {
Expand Down Expand Up @@ -43,6 +43,15 @@ module.exports.discoverFeedUrls = () => {
};
}

if (await isFeedUrl(url)) {
return [
{
feedUrl: url,
type: 'blog',
},
];
}

// Otherwise, try to parse out the feed URL from the body of the page
const body = await getBlogBody(url);
if (!body) {
Expand Down
22 changes: 22 additions & 0 deletions src/api/feed-discovery/src/util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,27 @@ const toTwitchFeedUrl = (twitchChannelUrl) => {
throw new Error('not a Twitch URL');
};

const isFeedUrl = async (url) => {
try {
const { statusCode, headers } = await got(url);
const contentType = headers['content-type'];
const validContentTypes = [
'application/xml',
'application/rss+xml',
'application/atom+xml',
'application/x.atom+xml',
'application/x-atom+xml',
'application/json',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that the rss-parser module we use (see https://github.com/rbren/rss-parser) is capable of parsing all of these types. We might want to limit it to valid Atom + RSS only.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we limit these here, should we also limit the link types that the getFeedUrls function further down this file checks to get feed urls from blog url?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's leave it for now and circle back if/when it becomes a problem.

'application/json+oembed',
'application/xml+oembed',
];

return statusCode === 200 && validContentTypes.some((ct) => contentType.includes(ct));
} catch (err) {
return false;
}
};

const getBlogBody = async (blogUrl) => {
try {
logger.debug({ blogUrl }, 'Getting blog body');
Expand Down Expand Up @@ -101,3 +122,4 @@ module.exports.getBlogBody = getBlogBody;
module.exports.getFeedUrls = getFeedUrls;
module.exports.isTwitchUrl = isTwitchUrl;
module.exports.toTwitchFeedUrl = toTwitchFeedUrl;
module.exports.isFeedUrl = isFeedUrl;
49 changes: 40 additions & 9 deletions src/api/feed-discovery/test/router.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe('POST /', () => {
};

// Mocking the response body html when call GET request to blog url
nock(blogUrl).get('/').reply(200, mockBlogUrlResponseBody, {
nock(blogUrl).get('/').twice().reply(200, mockBlogUrlResponseBody, {
'Content-Type': 'text/html',
});

Expand All @@ -56,7 +56,7 @@ describe('POST /', () => {
`;

// Mocking the response body html when call GET request to blog url
nock(blogUrl1).get('/').reply(200, mockBlogUrl1ResponseBody, {
nock(blogUrl1).get('/').twice().reply(200, mockBlogUrl1ResponseBody, {
'Content-Type': 'text/html',
});

Expand All @@ -72,7 +72,7 @@ describe('POST /', () => {
`;

// Mocking the response body html when call GET request to blog url
nock(blogUrl2).get('/').reply(200, mockBlogUrl2ResponseBody, {
nock(blogUrl2).get('/').twice().reply(200, mockBlogUrl2ResponseBody, {
'Content-Type': 'text/html',
});

Expand Down Expand Up @@ -131,7 +131,7 @@ describe('POST /', () => {
</html>`;

// Mocking the response body html (send back nothing) when call GET request to blog url
nock(blogUrl).get('/').reply(200, mockBlogBody, {
nock(blogUrl).get('/').twice().reply(200, mockBlogBody, {
'Content-Type': 'text/html',
});

Expand Down Expand Up @@ -199,7 +199,7 @@ describe('POST /', () => {
};

// Mocking the response body html when call GET request to blog url
nock(blogUrl).get('/').reply(200, mockBlogUrlResponseBody, {
nock(blogUrl).get('/').twice().reply(200, mockBlogUrlResponseBody, {
'Content-Type': 'text/html',
});

Expand Down Expand Up @@ -227,7 +227,7 @@ describe('POST /', () => {
};

// Mocking the response body html when call GET request to blog url
nock(blogUrl).get('/').reply(200, mockBlogUrlResponseBody, {
nock(blogUrl).get('/').twice().reply(200, mockBlogUrlResponseBody, {
'Content-Type': 'text/html',
});

Expand Down Expand Up @@ -255,7 +255,7 @@ describe('POST /', () => {
};

// Mocking the response body html when call GET request to blog url
nock(blogUrl).get('/').reply(200, mockBlogUrlResponseBody, {
nock(blogUrl).get('/').twice().reply(200, mockBlogUrlResponseBody, {
'Content-Type': 'text/html',
});

Expand Down Expand Up @@ -294,7 +294,7 @@ describe('POST /', () => {
};

// Mocking the response body html when call GET request to blog url
nock(blogUrl).get('/').reply(200, mockBlogUrlResponseBody, {
nock(blogUrl).get('/').twice().reply(200, mockBlogUrlResponseBody, {
'Content-Type': 'text/html',
});

Expand Down Expand Up @@ -329,7 +329,7 @@ describe('POST /', () => {
};

// Mocking the response body html when call GET request to blog url
nock(youTubeDomain).get(channelUri).reply(200, mockYouTubeChannelUrlResponseBody, {
nock(youTubeDomain).get(channelUri).twice().reply(200, mockYouTubeChannelUrlResponseBody, {
'Content-Type': 'text/html',
});

Expand All @@ -345,6 +345,37 @@ describe('POST /', () => {
const res = await request(app).post('/').send(['https://test321.blogspot.com/']);
expect(res.status).toBe(401);
});

it.concurrent.each([
'application/xml',
'application/rss+xml',
'application/atom+xml',
'application/x.atom+xml',
'application/x-atom+xml',
'application/json',
'application/json+oembed',
'application/xml+oembed',
])('should return 200 and the url when url response content type is %s', async (type) => {
const feedUrl = 'https://test321.blogspot.com/feed/user/';

const result = {
feedUrls: [
{
feedUrl,
type: 'blog',
},
],
};

nock(feedUrl).get('/').reply(200, undefined, { 'Content-Type': type });
const res = await request(app)
.post('/')
.set('Authorization', `bearer ${createServiceToken()}`)
.send([feedUrl]);

expect(res.status).toBe(200);
expect(res.body).toEqual(result);
});
});

it('should return 200 and 1 Atom feed url using RSS Bridge if the link is from a Twitch channel', async () => {
Expand Down
39 changes: 39 additions & 0 deletions src/api/feed-discovery/test/util.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const nock = require('nock');
const {
isTwitchUrl,
toTwitchFeedUrl,
isFeedUrl,
getBlogBody,
getFeedUrlType,
getFeedUrls,
Expand Down Expand Up @@ -31,6 +32,44 @@ describe('util.js', () => {
);
});

test('isFeedUrl returns true for a feed url', () => {
const feedUrl = 'https://blog.com/feed/user/';

[
'application/xml',
'application/rss+xml',
'application/atom+xml',
'application/x.atom+xml',
'application/x-atom+xml',
'application/json',
'application/json+oembed',
'application/xml+oembed',
].forEach(async (type) => {
nock(feedUrl).get('/').reply(200, undefined, { 'Content-Type': type });
expect(await isFeedUrl(feedUrl)).toBe(true);
});
});

test('isFeedUrl returns false when given URL returns a non 200 status', async () => {
const feedUrl = 'https://blog.com/feed/user/';
nock(feedUrl).get('/').reply(404, 'Not Found');

expect(await isFeedUrl(feedUrl)).toBe(false);
});

test('isFeedUrl returns false if given URL returns a non feed content type', async () => {
const feedUrl = 'https://blog.com/user/';
nock(feedUrl).get('/').reply(200, '<html></html>', { 'Content-Type': 'text/html' });

expect(await isFeedUrl(feedUrl)).toBe(false);
});

test('isFeedUrl returns false if given an invalid URL is given', async () => {
const feedUrl = 'Not a URL';

expect(await isFeedUrl(feedUrl)).toBe(false);
});

test('getBlogBody returns the expected body for a given URL', async () => {
const blogUrl = 'https://test321.blogspot.com/';
const mockBlogUrlResponseBody = `
Expand Down