Skip to content

Commit

Permalink
API: Remove 'immer' dependency
Browse files Browse the repository at this point in the history
'immer' didn't really work for us and led to all kinds of hard-to-debug
problems. Instead of relying on 'immer' to handle changes during event
sourcing, we now carefully deep-copy relevant data in the
`*_eventsourcing.ts` files and have the event-specific modules `mutate`
the aggregate in-place. This makes the latter very simple; at the same
time we ensure that the aggregates stored in the cache aren't changed
outside the cache.

Affected aggregates:

- project
- subproject
- workflowitem
  • Loading branch information
kevinbader committed Apr 10, 2019
1 parent 5e99704 commit b26909d
Show file tree
Hide file tree
Showing 55 changed files with 1,023 additions and 668 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

## [Unreleased]

### Fixed

- [API] Increased the stability of the event sourcing code by replacing the "immer" dependency with a custom implementation.

## [1.0.0-beta.7] - 2019-04-03

### Added
Expand Down
46 changes: 30 additions & 16 deletions api/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@
"fastify-jwt": "^0.3.0",
"fastify-metrics": "^2.0.2",
"fastify-swagger": "^0.15.0",
"immer": "^2.1.3",
"joi": "^14.3.1",
"jsonwebtoken": "^8.5.0",
"lodash.clonedeep": "^4.5.0",
Expand Down
16 changes: 16 additions & 0 deletions api/src/http_errors/bad_request.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import Joi = require("joi");
import { VError } from "verror";

import { Ctx } from "../lib/ctx";

interface Info {
ctx: Ctx;
requestPath: string;
validationResult: Joi.ValidationError;
}

export class BadRequest extends VError {
constructor(info: Info) {
super({ name: "BadRequest", info }, `invalid request to ${info.requestPath}`);
}
}
6 changes: 3 additions & 3 deletions api/src/service/cache2.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,15 +182,15 @@ describe("The cache updates", () => {

// Apply events to existing cache
const testAssignee = "shiba";
const wfAssginedEvent = WorkflowitemAssigned.createEvent(
const wfAssignedEvent = WorkflowitemAssigned.createEvent(
"http",
"test",
projectId,
subprojectId,
workflowitemId,
testAssignee,
);
if (Result.isErr(wfAssginedEvent)) {
if (Result.isErr(wfAssignedEvent)) {
return assert.fail(undefined, undefined, "Workflowitem assigned event failed");
}
const wfCloseEvent = WorkflowitemClosed.createEvent(
Expand All @@ -203,7 +203,7 @@ describe("The cache updates", () => {
if (Result.isErr(wfCloseEvent)) {
return assert.fail(undefined, undefined, "Workflowitem closed event failed");
}
updateAggregates(defaultCtx, cache, [wfAssginedEvent, wfCloseEvent]);
updateAggregates(defaultCtx, cache, [wfAssignedEvent, wfCloseEvent]);

// Test if events have been reflected on the aggregate
const wfUnderTest = cache.cachedWorkflowItems.get(workflowitemId);
Expand Down
23 changes: 11 additions & 12 deletions api/src/service/cache2.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { Ctx } from "../lib/ctx";
import deepcopy from "../lib/deepcopy";
import { isEmpty } from "../lib/emptyChecks";
import logger from "../lib/logger";
import * as Result from "../result";
Expand Down Expand Up @@ -31,7 +30,6 @@ import * as SubprojectAssigned from "./domain/workflow/subproject_assigned";
import * as SubprojectClosed from "./domain/workflow/subproject_closed";
import * as SubprojectCreated from "./domain/workflow/subproject_created";
import { sourceSubprojects } from "./domain/workflow/subproject_eventsourcing";
import * as WorkflowitemsReordered from "./domain/workflow/workflowitems_reordered";
import * as SubprojectPermissionsGranted from "./domain/workflow/subproject_permission_granted";
import * as SubprojectPermissionsRevoked from "./domain/workflow/subproject_permission_revoked";
import * as SubprojectProjectedBudgetDeleted from "./domain/workflow/subproject_projected_budget_deleted";
Expand All @@ -45,6 +43,7 @@ import { sourceWorkflowitems } from "./domain/workflow/workflowitem_eventsourcin
import * as WorkflowitemPermissionsGranted from "./domain/workflow/workflowitem_permission_granted";
import * as WorkflowitemPermissionsRevoked from "./domain/workflow/workflowitem_permission_revoked";
import * as WorkflowitemUpdated from "./domain/workflow/workflowitem_updated";
import * as WorkflowitemsReordered from "./domain/workflow/workflowitems_reordered";
import { Item } from "./liststreamitems";

const STREAM_BLACKLIST = [
Expand Down Expand Up @@ -117,17 +116,17 @@ export type TransactionFn<T> = (cache: CacheInstance) => Promise<T>;
export function getCacheInstance(ctx: Ctx, cache: Cache2): CacheInstance {
return {
getGlobalEvents: (): BusinessEvent[] => {
return deepcopy(cache.eventsByStream.get("global")) || [];
return cache.eventsByStream.get("global") || [];
},

getUserEvents: (_userId?: string): BusinessEvent[] => {
// userId currently not leveraged
return deepcopy(cache.eventsByStream.get("users")) || [];
return cache.eventsByStream.get("users") || [];
},

getGroupEvents: (_groupId?: string): BusinessEvent[] => {
// groupId currently not leveraged
return deepcopy(cache.eventsByStream.get("groups")) || [];
return cache.eventsByStream.get("groups") || [];
},

getNotificationEvents: (userId: string): BusinessEvent[] => {
Expand All @@ -147,11 +146,11 @@ export function getCacheInstance(ctx: Ctx, cache: Cache2): CacheInstance {
}
};

return (deepcopy(cache.eventsByStream.get("notifications")) || []).filter(userFilter);
return (cache.eventsByStream.get("notifications") || []).filter(userFilter);
},

getProjects: async (): Promise<Project.Project[]> => {
return deepcopy([...cache.cachedProjects.values()]);
return [...cache.cachedProjects.values()];
},

getProject: async (projectId: string): Promise<Result.Type<Project.Project>> => {
Expand All @@ -160,7 +159,7 @@ export function getCacheInstance(ctx: Ctx, cache: Cache2): CacheInstance {
if (project === undefined) {
return new NotFound(ctx, "project", projectId);
}
return deepcopy(project);
return project;
},

getSubprojects: async (projectId: string): Promise<Result.Type<Subproject.Subproject[]>> => {
Expand All @@ -180,7 +179,7 @@ export function getCacheInstance(ctx: Ctx, cache: Cache2): CacheInstance {
}
subprojects.push(sp);
}
return deepcopy(subprojects);
return subprojects;
},

getSubproject: (
Expand All @@ -191,7 +190,7 @@ export function getCacheInstance(ctx: Ctx, cache: Cache2): CacheInstance {
if (subproject === undefined) {
return new NotFound(ctx, "subproject", subprojectId);
}
return deepcopy(subproject);
return subproject;
},

getWorkflowitems: async (
Expand All @@ -213,7 +212,7 @@ export function getCacheInstance(ctx: Ctx, cache: Cache2): CacheInstance {
}
workflowitems.push(wf);
}
return deepcopy(workflowitems);
return workflowitems;
},

getWorkflowitem: async (
Expand All @@ -225,7 +224,7 @@ export function getCacheInstance(ctx: Ctx, cache: Cache2): CacheInstance {
if (workflowitem === undefined) {
return new NotFound(ctx, "workflowitem", workflowitemId);
}
return deepcopy(workflowitem);
return workflowitem;
},
};
}
Expand Down
5 changes: 3 additions & 2 deletions api/src/service/domain/workflow/project_assign.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { produce } from "immer";
import { VError } from "verror";

import { Ctx } from "../../../lib/ctx";
import * as Result from "../../../result";
import { BusinessEvent } from "../business_event";
Expand All @@ -12,6 +12,7 @@ import * as UserRecord from "../organization/user_record";
import * as NotificationCreated from "./notification_created";
import * as Project from "./project";
import * as ProjectAssigned from "./project_assigned";
import * as ProjectEventSourcing from "./project_eventsourcing";

interface Repository {
getProject(): Promise<Result.Type<Project.Project>>;
Expand Down Expand Up @@ -48,7 +49,7 @@ export async function assignProject(
}

// Check that the new event is indeed valid:
const result = produce(project, draft => ProjectAssigned.apply(ctx, projectAssigned, draft));
const result = ProjectEventSourcing.newProjectFromEvent(ctx, project, projectAssigned);
if (Result.isErr(result)) {
return new InvalidCommand(ctx, projectAssigned, [result]);
}
Expand Down
Loading

0 comments on commit b26909d

Please sign in to comment.