feat: migration of group-as-user to group-as-category, remote category purging, more tests

src/activitypub/actors.js

@@ -9,6 +9,7 @@ const meta = require('../meta');
 const batch = require('../batch');
 const categories = require('../categories');
 const user = require('../user');
+const topics = require('../topics');
 const utils = require('../utils');
 const TTLCache = require('../cache/ttl');
 
@@ -98,8 +99,8 @@ Actors.assert = async (ids, options = {}) => {
 	 */
 
 	ids = await Actors.qualify(ids, options);
-	if (!ids.length) {
+	if (!ids) {
-		return true;
+		return ids;
 	}
 
 	activitypub.helpers.log(`[activitypub/actors] Asserting ${ids.length} actor(s)`);
@@ -179,6 +180,9 @@ Actors.assert = async (ids, options = {}) => {
 		}
 	}));
 	actors = actors.filter(Boolean); // remove unresolvable actors
+	if (!actors.length && !categories.size) {
+		return [];
+	}
 
 	// Build userData object for storage
 	const profiles = (await activitypub.mocks.profile(actors)).filter(Boolean);
@@ -237,6 +241,18 @@ Actors.assert = async (ids, options = {}) => {
 		db.setObject('handle:uid', queries.handleAdd),
 	]);
 
+	// Handle any actors that should be asserted as a group instead
+	if (categories.size) {
+		const assertion = await Actors.assertGroup(Array.from(categories), options);
+		if (assertion === false) {
+			return false;
+		} else if (Array.isArray(assertion)) {
+			return [...actors, ...assertion];
+		}
+
+		// otherwise, assertGroup returned true and output can be safely ignored.
+	}
+
 	return actors;
 };
 
@@ -252,8 +268,8 @@ Actors.assertGroup = async (ids, options = {}) => {
 	 */
 
 	ids = await Actors.qualify(ids, options);
-	if (!ids.length) {
+	if (!ids) {
-		return true;
+		return ids;
 	}
 
 	activitypub.helpers.log(`[activitypub/actors] Asserting ${ids.length} group(s)`);
@@ -263,24 +279,14 @@ Actors.assertGroup = async (ids, options = {}) => {
 	const urlMap = new Map();
 	const followersUrlMap = new Map();
 	const pubKeysMap = new Map();
-	const users = new Set();
 	let groups = await Promise.all(ids.map(async (id) => {
 		try {
 			activitypub.helpers.log(`[activitypub/actors] Processing group ${id}`);
 			const actor = (typeof id === 'object' && id.hasOwnProperty('id')) ? id : await activitypub.get('uid', 0, id, { cache: process.env.CI === 'true' });
 
-			let typeOk = false;
+			const typeOk = Array.isArray(actor.type) ?
-			if (Array.isArray(actor.type)) {
+				actor.type.some(type => activitypub._constants.acceptableGroupTypes.has(type)) :
-				typeOk = actor.type.some(type => activitypub._constants.acceptableGroupTypes.has(type));
+				activitypub._constants.acceptableGroupTypes.has(actor.type);
-				if (!typeOk && actor.type.some(type => activitypub._constants.acceptableActorTypes.has(type))) {
-					users.add(actor.id);
-				}
-			} else {
-				typeOk = activitypub._constants.acceptableGroupTypes.has(actor.type);
-				if (!typeOk && activitypub._constants.acceptableActorTypes.has(actor.type)) {
-					users.add(actor.id);
-				}
-			}
 
 			if (
 				!typeOk ||
@@ -368,14 +374,41 @@ Actors.assertGroup = async (ids, options = {}) => {
 
 	await Promise.all([
 		db.setObjectBulk(bulkSet),
-		db.sortedSetAdd('usersRemote:lastCrawled', groups.map(() => now), groups.map(p => p.uid)),
+		db.sortedSetAdd('usersRemote:lastCrawled', groups.map(() => now), groups.map(p => p.id)),
 		// db.sortedSetAddBulk(queries.searchAdd),
 		db.setObject('handle:cid', queries.handleAdd),
+		_migratePersonToGroup(categoryObjs),
 	]);
 
 	return categoryObjs;
 };
 
+async function _migratePersonToGroup(categoryObjs) {
+	// 4.0.0-4.1.x asserted as:Group as users. This moves relevant stuff over and deletes the now-duplicate user.
+	let ids = categoryObjs.map(category => category.cid);
+	const slugs = categoryObjs.map(category => category.slug);
+	const isUser = await db.isObjectFields('handle:uid', slugs);
+	ids = ids.filter((id, idx) => isUser[idx]);
+	if (!ids.length) {
+		return;
+	}
+
+	await Promise.all(ids.map(async (id) => {
+		const shares = await db.getSortedSetMembers(`uid:${id}:shares`);
+		const exists = await topics.exists(shares);
+		await Promise.all(shares.map(async (share, idx) => {
+			if (exists[idx]) {
+				await topics.tools.move(share, {
+					cid: id,
+					uid: 0,
+				});
+			}
+		}));
+		await user.deleteAccount(id);
+	}));
+	await categories.onTopicsMoved(ids);
+}
+
 Actors.getLocalFollowers = async (id) => {
 	const response = {
 		uids: new Set(),
@@ -464,6 +497,41 @@ Actors.remove = async (id) => {
 	]);
 };
 
+Actors.removeGroup = async (id) => {
+	/**
+	 * Remove ActivityPub related metadata pertaining to a remote id
+	 *
+	 * Note: don't call this directly! It is called as part of categories.purge
+	 */
+	const exists = await db.isSortedSetMember('usersRemote:lastCrawled', id);
+	if (!exists) {
+		return false;
+	}
+
+	let { slug, /* fullname, */url, followersUrl } = await categories.getCategoryFields(id, ['slug', /* 'fullname', */ 'url', 'followersUrl']);
+	slug = slug.toLowerCase();
+
+	// const bulkRemove = [
+	// 	['ap.preferredUsername:sorted', `${name}:${id}`],
+	// ];
+	// if (fullname) {
+	// 	bulkRemove.push(['ap.name:sorted', `${fullname.toLowerCase()}:${id}`]);
+	// }
+
+	await Promise.all([
+		// db.sortedSetRemoveBulk(bulkRemove),
+		db.deleteObjectField('handle:cid', slug),
+		db.deleteObjectField('followersUrl:cid', followersUrl),
+		db.deleteObjectField('remoteUrl:cid', url),
+		db.delete(`categoryRemote:${id}:keys`),
+	]);
+
+	await Promise.all([
+		db.delete(`categoryRemote:${id}`),
+		db.sortedSetRemove('usersRemote:lastCrawled', id),
+	]);
+};
+
 Actors.prune = async () => {
 	/**
 	 * Clear out remote user accounts that do not have content on the forum anywhere

src/categories/data.js

@@ -87,11 +87,11 @@ module.exports = function (Categories) {
 	};
 
 	Categories.setCategoryField = async function (cid, field, value) {
-		await db.setObjectField(`category:${cid}`, field, value);
+		await db.setObjectField(`${utils.isNumber(cid) ? 'category' : 'categoryRemote'}:${cid}`, field, value);
 	};
 
 	Categories.incrementCategoryFieldBy = async function (cid, field, value) {
-		await db.incrObjectFieldBy(`category:${cid}`, field, value);
+		await db.incrObjectFieldBy(`${utils.isNumber(cid) ? 'category' : 'categoryRemote'}:${cid}`, field, value);
 	};
 };
 

src/categories/delete.js

@@ -7,7 +7,9 @@ const plugins = require('../plugins');
 const topics = require('../topics');
 const groups = require('../groups');
 const privileges = require('../privileges');
+const activitypub = require('../activitypub');
 const cache = require('../cache');
+const utils = require('../utils');
 
 module.exports = function (Categories) {
 	Categories.purge = async function (cid, uid) {
@@ -38,6 +40,7 @@ module.exports = function (Categories) {
 
 		await removeFromParent(cid);
 		await deleteTags(cid);
+		await activitypub.actors.removeGroup(cid);
 		await db.deleteAll([
 			`cid:${cid}:tids`,
 			`cid:${cid}:tids:pinned`,
@@ -51,7 +54,7 @@ module.exports = function (Categories) {
 			`cid:${cid}:uid:watch:state`,
 			`cid:${cid}:children`,
 			`cid:${cid}:tag:whitelist`,
-			`category:${cid}`,
+			`${utils.isNumber(cid) ? 'category' : 'categoryRemote'}:${cid}`,
 		]);
 		const privilegeList = await privileges.categories.getPrivilegeList();
 		await groups.destroy(privilegeList.map(privilege => `cid:${cid}:privileges:${privilege}`));

src/topics/delete.js

@@ -145,8 +145,8 @@ module.exports = function (Topics) {
 		const postCountChange = incr * topicData.postcount;
 		await Promise.all([
 			db.incrObjectFieldBy('global', 'postCount', postCountChange),
-			db.incrObjectFieldBy(`category:${topicData.cid}`, 'post_count', postCountChange),
+			db.incrObjectFieldBy(`${utils.isNumber(topicData.cid) ? 'category' : 'categoryRemote'}:${topicData.cid}`, 'post_count', postCountChange),
-			db.incrObjectFieldBy(`category:${topicData.cid}`, 'topic_count', incr),
+			db.incrObjectFieldBy(`${utils.isNumber(topicData.cid) ? 'category' : 'categoryRemote'}:${topicData.cid}`, 'topic_count', incr),
 		]);
 	}
 };

src/topics/tools.js

@@ -233,7 +233,7 @@ module.exports = function (Topics) {
 	};
 
 	topicTools.move = async function (tid, data) {
-		const cid = parseInt(data.cid, 10);
+		const cid = utils.isNumber(data.cid) ? parseInt(data.cid, 10) : data.cid;
 		const topicData = await Topics.getTopicData(tid);
 		if (!topicData) {
 			throw new Error('[[error:no-topic]]');

test/activitypub/actors.js

@@ -5,6 +5,7 @@ const nconf = require('nconf');
 
 const db = require('../mocks/databasemock');
 const meta = require('../../src/meta');
+const install = require('../../src/install');
 const categories = require('../../src/categories');
 const user = require('../../src/user');
 const topics = require('../../src/topics');
@@ -16,6 +17,11 @@ const slugify = require('../../src/slugify');
 const helpers = require('./helpers');
 
 describe('Actor asserton', () => {
+	before(async () => {
+		meta.config.activitypubEnabled = 1;
+		await install.giveWorldPrivileges();
+	});
+
 	describe('happy path', () => {
 		let uid;
 		let actorUri;
@@ -62,12 +68,37 @@ describe('Actor asserton', () => {
 		});
 
 		it('should assert group actors by calling actors.assertGroup', async () => {
-			assert(false);
+			const { id, actor } = helpers.mocks.group();
+			const assertion = await activitypub.actors.assert([id]);
+
+			assert(assertion);
+			assert.strictEqual(assertion.length, 1);
+			assert.strictEqual(assertion[0].cid, actor.id);
 		});
 
 		it('should migrate a user to a category if on re-assertion it identifies as an as:Group', async () => {
 			// This is to handle previous behaviour that saved all as:Group actors as NodeBB users.
-			assert(false);
+			const { id } = helpers.mocks.person();
+			await activitypub.actors.assert([id]);
+
+			// Two shares
+			for (let x = 0; x < 2; x++) {
+				const { id: pid } = helpers.mocks.note();
+				// eslint-disable-next-line no-await-in-loop
+				const { tid } = await activitypub.notes.assert(0, pid, { skipChecks: 1 });
+				// eslint-disable-next-line no-await-in-loop
+				await db.sortedSetAdd(`uid:${id}:shares`, Date.now(), tid);
+			}
+
+			const { actor } = helpers.mocks.group({ id });
+			const assertion = await activitypub.actors.assert([id], { update: true });
+
+			const { topic_count, post_count } = await categories.getCategoryData(id);
+			assert.strictEqual(topic_count, 2);
+			assert.strictEqual(post_count, 2);
+
+			const exists = await user.exists(id);
+			assert.strictEqual(exists, false);
 		});
 	});
 
@@ -101,11 +132,36 @@ describe('Actor asserton', () => {
 	});
 
 	describe('deletion', () => {
-		// todo...
+		it('should delete a remote category when Categories.purge is called', async () => {
+			const { id } = helpers.mocks.group();
+			await activitypub.actors.assertGroup([id]);
+
+			let exists = await categories.exists(id);
+			assert(exists);
+
+			await categories.purge(id, 0);
+
+			exists = await categories.exists(id);
+			assert(!exists);
+
+			exists = await db.exists(`categoryRemote:${id}`);
+			assert(!exists);
+		});
+
+		it('should also delete AP-specific keys that were added by assertGroup', async () => {
+			const { id } = helpers.mocks.group();
+			const assertion = await activitypub.actors.assertGroup([id]);
+			const [{ slug }] = assertion;
+
+			await categories.purge(id, 0);
+
+			const isMember = await db.isObjectField('handle:cid', slug);
+			assert(!isMember);
+		});
 	});
 });
 
-describe.only('Group assertion', () => {
+describe('Group assertion', () => {
 	let actorUri;
 
 	before(async () => {
@@ -135,8 +191,11 @@ describe.only('Group assertion', () => {
 		assert.strictEqual(category.cid, actorUri);
 	});
 
-	it('should assert non-group users by calling actors.assert', async () => {
+	it('should not assert non-group users when called', async () => {
-		assert(false);
+		const { id } = helpers.mocks.person();
+		const assertion = await activitypub.actors.assertGroup([id]);
+
+		assert(Array.isArray(assertion) && !assertion.length);
 	});
 });