OpenSourceArk/Ghost
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Refactored UrlHistory to use static factory method

Browse Source 
This keeps the constructor clean, relying on types for validation,
whilst preserving the validation when creating the instance. The
constructor is now private so that the factory which handles
validation is always used.

The tests have also been updated to test the public factory interface
rather than the internal validation methods. Validation has been
rolled into a single method and slightly improved in the way of
readability.
This commit is contained in:
Fabien "egg" O'Carroll 2022-08-25 12:21:08 -04:00
parent 75f08f55cf
commit f523e1eb6b
4 changed files with 95 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
ghost/member-attribution/lib/history.js
View File

@ -1,19 +1,24 @@
/**
* @typedef {UrlHistoryItem[]} UrlHistoryArray
*/
/**
* @typedef {Object} UrlHistoryItem
* @prop {string} path
* @prop {number} time
*/
/**
* @typedef {UrlHistoryItem[]} UrlHistoryArray
*/
/**
* Represents a validated history
*/
class UrlHistory {
/**
* @private
* @param {UrlHistoryArray} urlHistory
*/
constructor(urlHistory) {
this.history = urlHistory && UrlHistory.isValidHistory(urlHistory) ? urlHistory : [];
/** @private */
this.history = urlHistory;
}
get length() {
@ -27,12 +32,34 @@ class UrlHistory {
yield* this.history.slice().reverse();
}
/**
* @private
* @param {any[]} history
* @returns {boolean}
*/
static isValidHistory(history) {
return Array.isArray(history) && !history.find(item => !this.isValidHistoryItem(item));
for (const item of history) {
if (typeof item?.path !== 'string' || !Number.isSafeInteger(item?.time)) {
return false;
}
}
return true;
}
static isValidHistoryItem(item) {
return !!item && !!item.path && !!item.time && typeof item.path === 'string' && typeof item.time === 'number' && Number.isSafeInteger(item.time);
/**
* @param {unknown} urlHistory
* @returns {UrlHistory}
*/
static create(urlHistory) {
if (!Array.isArray(urlHistory)) {
return new UrlHistory([]);
}
if (!this.isValidHistory(urlHistory)) {
return new UrlHistory([]);
}
return new UrlHistory(urlHistory);
}
}

14
ghost/member-attribution/lib/service.js
View File

@ -2,8 +2,8 @@ const UrlHistory = require('./history');
class MemberAttributionService {
/**
*
* @param {Object} deps
*
* @param {Object} deps
* @param {Object} deps.attributionBuilder
* @param {Object} deps.models
* @param {Object} deps.models.MemberCreatedEvent
@ -15,12 +15,12 @@ class MemberAttributionService {
}
/**
*
* @param {import('./history').UrlHistoryArray} historyArray
*
* @param {import('./history').UrlHistoryArray} historyArray
* @returns {import('./attribution').Attribution}
*/
getAttribution(historyArray) {
const history = new UrlHistory(historyArray);
const history = UrlHistory.create(historyArray);
return this.attributionBuilder.getAttribution(history);
}
@ -60,7 +60,7 @@ class MemberAttributionService {
/**
* Returns the parsed attribution for a member creation event
* @param {string} memberId
* @param {string} memberId
* @returns {Promise<import('./attribution').AttributionResource|null>}
*/
async getMemberCreatedAttribution(memberId) {
@ -78,7 +78,7 @@ class MemberAttributionService {
/**
* Returns the last attribution for a given subscription ID
* @param {string} subscriptionId
* @param {string} subscriptionId
* @returns {Promise<import('./attribution').AttributionResource|null>}
*/
async getSubscriptionCreatedAttribution(subscriptionId) {

24
ghost/member-attribution/test/attribution.test.js
View File

@ -56,18 +56,18 @@ describe('AttributionBuilder', function () {
});
it('Returns empty if empty history', function () {
const history = new UrlHistory([]);
const history = UrlHistory.create([]);
should(attributionBuilder.getAttribution(history)).match({id: null, type: null, url: null});
});
it('Returns last url', function () {
const history = new UrlHistory([{path: '/dir/not-last', time: 123}, {path: '/dir/test/', time: 123}]);
const history = UrlHistory.create([{path: '/dir/not-last', time: 123}, {path: '/dir/test/', time: 123}]);
should(attributionBuilder.getAttribution(history)).match({type: 'url', id: null, url: '/test/'});
});
it('Returns last post', function () {
const history = new UrlHistory([
{path: '/dir/my-post', time: 123},
const history = UrlHistory.create([
{path: '/dir/my-post', time: 123},
{path: '/dir/test', time: 124},
{path: '/dir/unknown-page', time: 125}
]);
@ -75,25 +75,25 @@ describe('AttributionBuilder', function () {
});
it('Returns last post even when it found pages', function () {
const history = new UrlHistory([
{path: '/dir/my-post', time: 123},
{path: '/dir/my-page', time: 124},
const history = UrlHistory.create([
{path: '/dir/my-post', time: 123},
{path: '/dir/my-page', time: 124},
{path: '/dir/unknown-page', time: 125}
]);
should(attributionBuilder.getAttribution(history)).match({type: 'post', id: 123, url: '/my-post'});
});
it('Returns last page if no posts', function () {
const history = new UrlHistory([
{path: '/dir/other', time: 123},
{path: '/dir/my-page', time: 124},
const history = UrlHistory.create([
{path: '/dir/other', time: 123},
{path: '/dir/my-page', time: 124},
{path: '/dir/unknown-page', time: 125}
]);
should(attributionBuilder.getAttribution(history)).match({type: 'page', id: 845, url: '/my-page'});
});
it('Returns all null for invalid histories', function () {
const history = new UrlHistory('invalid');
const history = UrlHistory.create('invalid');
should(attributionBuilder.getAttribution(history)).match({
type: null,
id: null,
@ -102,7 +102,7 @@ describe('AttributionBuilder', function () {
});
it('Returns all null for empty histories', function () {
const history = new UrlHistory([]);
const history = UrlHistory.create([]);
should(attributionBuilder.getAttribution(history)).match({
type: null,
id: null,

98
ghost/member-attribution/test/history.test.js
View File

@ -4,72 +4,56 @@ require('./utils');
const UrlHistory = require('../lib/history');
describe('UrlHistory', function () {
describe('Constructor', function () {
it('sets history to empty array if invalid', function () {
const history = new UrlHistory('invalid');
should(history.history).eql([]);
});
it('sets history to empty array if missing', function () {
const history = new UrlHistory();
should(history.history).eql([]);
});
});
describe('Validation', function () {
it('isValidHistory returns false for non arrays', function () {
should(UrlHistory.isValidHistory('string')).eql(false);
should(UrlHistory.isValidHistory()).eql(false);
should(UrlHistory.isValidHistory(12)).eql(false);
should(UrlHistory.isValidHistory(null)).eql(false);
should(UrlHistory.isValidHistory({})).eql(false);
should(UrlHistory.isValidHistory(NaN)).eql(false);
should(UrlHistory.isValidHistory([
it('sets history to empty array if invalid', function () {
const inputs = [
'string',
undefined,
12,
null,
{},
NaN,
[
{
time: 1,
path: '/test'
},
't'
])).eql(false);
});
it('isValidHistory returns true for valid arrays', function () {
should(UrlHistory.isValidHistory([])).eql(true);
should(UrlHistory.isValidHistory([
{
time: 1,
path: '/test'
}
])).eql(true);
});
it('isValidHistoryItem returns false for invalid objects', function () {
should(UrlHistory.isValidHistoryItem({})).eql(false);
should(UrlHistory.isValidHistoryItem('test')).eql(false);
should(UrlHistory.isValidHistoryItem(0)).eql(false);
should(UrlHistory.isValidHistoryItem()).eql(false);
should(UrlHistory.isValidHistoryItem(NaN)).eql(false);
should(UrlHistory.isValidHistoryItem([])).eql(false);
should(UrlHistory.isValidHistoryItem({
],
[{}],
['test'],
[0],
[undefined],
[NaN],
[[]],
[{
time: 'test',
path: 'test'
})).eql(false);
should(UrlHistory.isValidHistoryItem({
}],
[{
path: 'test'
})).eql(false);
should(UrlHistory.isValidHistoryItem({
}],
[{
time: 123
})).eql(false);
});
}]
];
it('isValidHistoryItem returns true for valid objects', function () {
should(UrlHistory.isValidHistoryItem({
time: 123,
path: '/time'
})).eql(true);
});
for (const input of inputs) {
const history = UrlHistory.create(input);
should(history.history).eql([]);
}
});
it('sets history for valid arrays', function () {
const inputs = [
[],
[{
time: Date.now(),
path: '/test'
}]
];
for (const input of inputs) {
const history = UrlHistory.create(input);
should(history.history).eql(input);
}
});
});