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

Add debouncing support to UserActivityTracker #448

Merged
merged 7 commits into from
Nov 30, 2022
Merged

Add debouncing support to UserActivityTracker #448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4a6d5e8
Add debouncing support to UserActivityTracker
tadzik Oct 26, 2022
141695f
Change UserActivitySet to be a proper Map
tadzik Nov 10, 2022
b0c3d79
Fixup the tests for 141695feafa2e65222db9073ebab284b27cb4503
tadzik Nov 10, 2022
a6b3810
Fix UserActivityState.changed population when debouncing updates
tadzik Nov 10, 2022
1d31ddf
Linting
tadzik Nov 28, 2022
f525341
Pick 1 minute as a sensible debouncing default
tadzik Nov 28, 2022
b295746
Changelog
tadzik Nov 28, 2022
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
1 change: 1 addition & 0 deletions changelog.d/448.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add debouncing support to UserActivityTracker (defaults to 60 seconds).
92 changes: 58 additions & 34 deletions spec/unit/userActivity.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,17 @@ const USER_THREE = "@charlie:example.com";
const ONE_DAY = 24 * 60 * 60 * 1000;
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to note, I think these can be .ts files now


describe("userActivity", () => {
const emptyDataSet = () => { return { users: {} } };
const emptyDataSet = () => { return new Map() };
const instantConfig = {
...UserActivityTrackerConfig.DEFAULT,
debounceTimeMs: 0,
};
describe("updateUserActivity", () => {
it("can update a user's activity", async () => {
let userData;
const trackerPromise = new Promise((resolve, _) => {
const tracker = new UserActivityTracker(
UserActivityTrackerConfig.DEFAULT,
instantConfig,
emptyDataSet(),
(data) => resolve(data.dataSet),
);
Expand All @@ -28,13 +32,13 @@ describe("userActivity", () => {
});
// This data is comitted asyncronously.
const data = await trackerPromise;
expect(data).toEqual({
users: {[USER_ONE]: userData}
});
expect(data).toEqual(new Map([
[USER_ONE, userData],
]));
});
it("can update a user's activity with metadata", () => {
const tracker = new UserActivityTracker(
UserActivityTrackerConfig.DEFAULT,
instantConfig,
emptyDataSet(),
);
tracker.updateUserActivity(USER_ONE, { private: true }, DATE_NOW);
Expand All @@ -47,7 +51,7 @@ describe("userActivity", () => {
});
it("can update a user's activity twice", () => {
const tracker = new UserActivityTracker(
UserActivityTrackerConfig.DEFAULT,
instantConfig,
emptyDataSet(),
);
tracker.updateUserActivity(USER_ONE, undefined, DATE_MINUS_ONE);
Expand All @@ -62,7 +66,7 @@ describe("userActivity", () => {
});
it("will not remove metadata from a user", () => {
const tracker = new UserActivityTracker(
UserActivityTrackerConfig.DEFAULT,
instantConfig,
emptyDataSet(),
);
tracker.updateUserActivity(USER_ONE, { private: true}, DATE_MINUS_ONE);
Expand All @@ -79,7 +83,7 @@ describe("userActivity", () => {
});
it("will cut off a users activity after 31 days", () => {
const tracker = new UserActivityTracker(
UserActivityTrackerConfig.DEFAULT,
instantConfig,
emptyDataSet(),
);
const LAST_EXPECTED_DATE = (DATE_NOW.getTime() - (ONE_DAY * 30)) / 1000;
Expand All @@ -95,7 +99,7 @@ describe("userActivity", () => {
describe("countActiveUsers", () => {
it("should have no users when the dataset is blank", () => {
const tracker = new UserActivityTracker(
UserActivityTrackerConfig.DEFAULT,
instantConfig,
emptyDataSet(),
);
expect(tracker.countActiveUsers(DATE_NOW)).toEqual({
Expand All @@ -105,7 +109,7 @@ describe("userActivity", () => {
});
it("should have no users when the user hasn't been active for 3 days", () => {
const tracker = new UserActivityTracker(
UserActivityTrackerConfig.DEFAULT,
instantConfig,
emptyDataSet(),
);
tracker.updateUserActivity(USER_ONE, undefined, DATE_MINUS_ONE);
Expand All @@ -117,7 +121,7 @@ describe("userActivity", () => {
});
it("should have users when the user has been active for at least 3 days", () => {
const tracker = new UserActivityTracker(
UserActivityTrackerConfig.DEFAULT,
instantConfig,
emptyDataSet(),
);
tracker.updateUserActivity(USER_ONE, undefined, DATE_MINUS_TWO);
Expand All @@ -131,17 +135,15 @@ describe("userActivity", () => {
it("should not include 'active' users who have not talked in 32 days", () => {
const DATE_MINUS_THIRTY_TWO = new Date(Date.UTC(2020, 11, 30, 0));
const tracker = new UserActivityTracker(
UserActivityTrackerConfig.DEFAULT,
{
users: {
[USER_ONE]: {
ts: [DATE_MINUS_THIRTY_TWO.getTime() / 1000],
metadata: {
active: true,
}
instantConfig,
new Map([
[USER_ONE, {
ts: [DATE_MINUS_THIRTY_TWO.getTime() / 1000],
metadata: {
active: true,
}
}
},
}]
]),
);
expect(tracker.countActiveUsers(DATE_NOW)).toEqual({
allUsers: 0,
Expand All @@ -151,17 +153,15 @@ describe("userActivity", () => {
it("should include 'active' users who have talked in 31 days", () => {
const DATE_MINUS_THIRTY_ONE = new Date(Date.UTC(2020, 12, 1, 0));
const tracker = new UserActivityTracker(
UserActivityTrackerConfig.DEFAULT,
{
users: {
[USER_ONE]: {
ts: [DATE_MINUS_THIRTY_ONE.getTime() / 1000],
metadata: {
active: true,
}
instantConfig,
new Map([
[USER_ONE, {
ts: [DATE_MINUS_THIRTY_ONE.getTime() / 1000],
metadata: {
active: true,
}
}
},
}]
]),
);
expect(tracker.countActiveUsers(DATE_NOW)).toEqual({
allUsers: 1,
Expand All @@ -170,7 +170,7 @@ describe("userActivity", () => {
});
it("should mark user as private if metadata specifies it", () => {
const tracker = new UserActivityTracker(
UserActivityTrackerConfig.DEFAULT,
instantConfig,
emptyDataSet(),
);
tracker.updateUserActivity(USER_ONE, { private: true}, DATE_MINUS_TWO);
Expand All @@ -183,7 +183,7 @@ describe("userActivity", () => {
});
it("should handle multiple users", () => {
const tracker = new UserActivityTracker(
UserActivityTrackerConfig.DEFAULT,
instantConfig,
emptyDataSet(),
);
tracker.updateUserActivity(USER_ONE, { private: true }, DATE_MINUS_TWO);
Expand All @@ -199,4 +199,28 @@ describe("userActivity", () => {
});
});
})
describe("debouncing", () => {
it("should only send one update in a specified period", async () => {
const updates = [];
const tracker = new UserActivityTracker(
{
...UserActivityTrackerConfig.DEFAULT,
debounceTimeMs: 100,
},
emptyDataSet(),
(changes) => updates.push(changes),
);
tracker.updateUserActivity(USER_ONE, undefined, DATE_MINUS_ONE);
tracker.updateUserActivity(USER_TWO, undefined, DATE_MINUS_ONE);
tracker.updateUserActivity(USER_ONE, undefined, DATE_MINUS_ONE);
await new Promise(resolve => setTimeout(resolve, 200));
expect(updates.length).toEqual(1);
expect(updates[0].changed.sort()).toEqual([USER_ONE, USER_TWO]);

tracker.updateUserActivity(USER_ONE, undefined, DATE_NOW);
await new Promise(resolve => setTimeout(resolve, 200));
expect(updates.length).toEqual(2);
expect(updates[1].changed.length).toEqual(1);
});
});
})
8 changes: 4 additions & 4 deletions src/components/user-activity-store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,14 @@ export class UserActivityStore extends BridgeStore {

public async getActivitySet(): Promise<UserActivitySet> {
return this.select({}).then((records: any[]) => {
const users: {[mxid: string]: any} = {};
const userActivity: UserActivitySet = new Map();
for (const record of records) {
users[record.mxid] = {
userActivity.set(record.mxid, {
ts: record.ts,
metadata: record.metadata,
};
});
}
return { users } as UserActivitySet;
return userActivity;
});
}
}
47 changes: 24 additions & 23 deletions src/components/user-activity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,7 @@ interface UserActivityMetadata {
active?: true;
}

export interface UserActivitySet {
users: {[userId: string]: UserActivity};
}

// eslint-disable-next-line @typescript-eslint/no-namespace,no-redeclare
export namespace UserActivitySet {
export const DEFAULT: UserActivitySet = {
users: {}
};
}
export type UserActivitySet = Map<string, UserActivity>;

export interface UserActivity {
ts: number[];
Expand All @@ -45,13 +36,15 @@ export interface UserActivity {
export interface UserActivityTrackerConfig {
inactiveAfterDays: number;
minUserActiveDays: number;
debounceTimeMs: number;
}

// eslint-disable-next-line @typescript-eslint/no-namespace,no-redeclare
export namespace UserActivityTrackerConfig {
export const DEFAULT: UserActivityTrackerConfig = {
inactiveAfterDays: 31,
minUserActiveDays: 3,
debounceTimeMs: 60 * 1000, // 1 minute
};
}

Expand All @@ -78,14 +71,17 @@ const ONE_DAY = 24 * 60 * 60 * 1000;
* to not interfere with UserActivityTracker's operations.
*/
export class UserActivityTracker {
private debounceTimer: NodeJS.Timeout|undefined;
private debouncedChangedSet = new Set<string>();

constructor(
private readonly config: UserActivityTrackerConfig,
private readonly dataSet: UserActivitySet,
private readonly onChanges?: ChangesCallback,
) { }

public updateUserActivity(userId: string, metadata?: UserActivityMetadata, dateOverride?: Date): void {
let userObject = this.dataSet.users[userId];
let userObject = this.dataSet.get(userId);
if (!userObject) {
userObject = {
ts: [],
Expand Down Expand Up @@ -115,15 +111,20 @@ export class UserActivityTracker {
}
}

this.dataSet.users[userId] = userObject;
setImmediate(() => {
log.debug("Notifying the listener of RMAU changes");
this.onChanges?.({
changed: [userId],
dataSet: this.dataSet,
activeUsers: this.countActiveUsers().allUsers,
});
});
this.dataSet.set(userId, userObject);
this.debouncedChangedSet.add(userId);
if (!this.debounceTimer) {
this.debounceTimer = setTimeout(() => {
log.debug(`Notifying the listener of RMAU changes`);
this.onChanges?.({
changed: Array.from(this.debouncedChangedSet),
dataSet: this.dataSet,
activeUsers: this.countActiveUsers().allUsers,
});
this.debounceTimer = undefined;
this.debouncedChangedSet.clear();
}, this.config.debounceTimeMs);
}
}

/**
Expand All @@ -136,7 +137,7 @@ export class UserActivityTracker {
let allUsers = 0;
let privateUsers = 0;
const activeSince = ((dateNow?.getTime() || Date.now()) - this.config.inactiveAfterDays * ONE_DAY) / 1000;
for (const user of Object.values(this.dataSet.users)) {
for (const user of this.dataSet.values()) {
if (!user.metadata.active) {
continue;
}
Expand All @@ -151,7 +152,7 @@ export class UserActivityTracker {
return {allUsers, privateUsers};
}

public getUserData(userId: string): UserActivity {
return this.dataSet.users[userId];
public getUserData(userId: string): UserActivity|undefined {
return this.dataSet.get(userId);
}
}