[PM-20134] Fix overwriteExisting and largeImport causing users to be deleted

src/abstractions/request-builder.service.ts

@@ -3,13 +3,20 @@ import { OrganizationImportRequest } from "@/jslib/common/src/models/request/org
 import { GroupEntry } from "@/src/models/groupEntry";
 import { UserEntry } from "@/src/models/userEntry";
 
+export class RequestBuilderOptions {
+  constructor(options: { removeDisabled: boolean; overwriteExisting: boolean }) {
+    this.removeDisabled = options.removeDisabled;
+    this.overwriteExisting = options.overwriteExisting;
+  }
+
+  removeDisabled: boolean = false;
+  overwriteExisting: boolean = false;
+}
+
 export abstract class RequestBuilder {
   buildRequest: (
     groups: GroupEntry[],
     users: UserEntry[],
-    options: {
-      removeDisabled: boolean;
-      overwriteExisting: boolean;
-    },
+    options: RequestBuilderOptions,
   ) => OrganizationImportRequest[];
 }

src/services/batch-request-builder.ts

@@ -3,7 +3,7 @@ import { OrganizationImportRequest } from "@/jslib/common/src/models/request/org
 import { GroupEntry } from "@/src/models/groupEntry";
 import { UserEntry } from "@/src/models/userEntry";
 
-import { RequestBuilder } from "../abstractions/request-builder.service";
+import { RequestBuilder, RequestBuilderOptions } from "../abstractions/request-builder.service";
 
 import { batchSize } from "./sync.service";
 
@@ -16,10 +16,7 @@ export class BatchRequestBuilder implements RequestBuilder {
   buildRequest(
     groups: GroupEntry[],
     users: UserEntry[],
-    options: {
-      removeDisabled: boolean;
-      overwriteExisting: boolean;
-    },
+    options: RequestBuilderOptions,
   ): OrganizationImportRequest[] {
     if (options.overwriteExisting) {
       throw new Error(

src/services/batch-requests-builder.spec.ts

@@ -1,12 +1,12 @@
 import { GroupEntry } from "@/src/models/groupEntry";
 import { UserEntry } from "@/src/models/userEntry";
 
+import { RequestBuilderOptions } from "../abstractions/request-builder.service";
+
 import { BatchRequestBuilder } from "./batch-request-builder";
-import { SingleRequestBuilder } from "./single-request-builder";
 
 describe("BatchRequestBuilder", () => {
   let batchRequestBuilder: BatchRequestBuilder;
-  let singleRequestBuilder: SingleRequestBuilder;
 
   function userSimulator(userCount: number) {
     return Array(userCount).fill(new UserEntry());
@@ -18,10 +18,12 @@ describe("BatchRequestBuilder", () => {
 
   beforeEach(async () => {
     batchRequestBuilder = new BatchRequestBuilder();
-    singleRequestBuilder = new SingleRequestBuilder();
   });
 
-  const defaultOptions = { overwriteExisting: false, removeDisabled: false };
+  const defaultOptions = new RequestBuilderOptions({
+    overwriteExisting: false,
+    removeDisabled: false,
+  });
 
   it("BatchRequestBuilder batches requests for > 2000 users", () => {
     const mockGroups = groupSimulator(11000);
@@ -35,7 +37,7 @@ describe("BatchRequestBuilder", () => {
   it("BatchRequestBuilder throws error when overwriteExisting is true", () => {
     const mockGroups = groupSimulator(11000);
     const mockUsers = userSimulator(11000);
-    const options = { ...defaultOptions, overwriteExisting: true };
+    const options = new RequestBuilderOptions({ ...defaultOptions, overwriteExisting: true });
 
     const r = () => batchRequestBuilder.buildRequest(mockGroups, mockUsers, options);
 
@@ -44,13 +46,22 @@ describe("BatchRequestBuilder", () => {
     );
   });
 
-  it("SingleRequestBuilder returns single request for 200 users", () => {
-    const mockGroups = groupSimulator(200);
-    const mockUsers = userSimulator(200);
-
-    const requests = singleRequestBuilder.buildRequest(mockGroups, mockUsers, defaultOptions);
-
-    expect(requests.length).toEqual(1);
+  it("BatchRequestBuilder returns requests with deleted users when removeDisabled is true", () => {
+    const mockGroups = groupSimulator(11000);
+    const disabledUser = new UserEntry();
+    const mockUsers = userSimulator(11000);
+    const options = new RequestBuilderOptions({ ...defaultOptions, removeDisabled: true });
+
+    disabledUser.disabled = true;
+    mockUsers[0] = disabledUser;
+    mockUsers.push(disabledUser);
+
+    const requests = batchRequestBuilder.buildRequest(mockGroups, mockUsers, options);
+    expect(requests[0].members.find((m) => m.deleted)).toBeTruthy();
+    expect(requests[1].members.find((m) => m.deleted)).toBeUndefined();
+    expect(requests[3].members.find((m) => m.deleted)).toBeUndefined();
+    expect(requests[4].members.find((m) => m.deleted)).toBeUndefined();
+    expect(requests[5].members.find((m) => m.deleted)).toBeTruthy();
   });
 
   it("BatchRequestBuilder retuns an empty array when there are no users or groups", () => {

src/services/ldap-directory.service.integration.spec.ts

@@ -1,14 +1,24 @@
 import { mock, MockProxy } from "jest-mock-extended";
 
+import { ApiService } from "@/jslib/common/src/abstractions/api.service";
+import { CryptoFunctionService } from "@/jslib/common/src/abstractions/cryptoFunction.service";
+import { MessagingService } from "@/jslib/common/src/abstractions/messaging.service";
+import { EnvironmentService } from "@/jslib/common/src/services/environment.service";
+
 import { I18nService } from "../../jslib/common/src/abstractions/i18n.service";
 import { LogService } from "../../jslib/common/src/abstractions/log.service";
 import { groupFixtures } from "../../openldap/group-fixtures";
 import { userFixtures } from "../../openldap/user-fixtures";
+import { DirectoryFactoryService } from "../abstractions/directory-factory.service";
 import { DirectoryType } from "../enums/directoryType";
 import { getLdapConfiguration, getSyncConfiguration } from "../utils/test-fixtures";
 
+import { BatchRequestBuilder } from "./batch-request-builder";
 import { LdapDirectoryService } from "./ldap-directory.service";
+import { SingleRequestBuilder } from "./single-request-builder";
 import { StateService } from "./state.service";
+import { SyncService } from "./sync.service";
+import * as constants from "./sync.service";
 
 // These tests integrate with the OpenLDAP docker image and seed data located in the openldap folder.
 // To run theses tests:
@@ -20,19 +30,49 @@
   let logService: MockProxy<LogService>;
   let i18nService: MockProxy<I18nService>;
   let stateService: MockProxy<StateService>;
-
+  let cryptoFunctionService: MockProxy<CryptoFunctionService>;
+  let apiService: MockProxy<ApiService>;
+  let messagingService: MockProxy<MessagingService>;
+  let environmentService: MockProxy<EnvironmentService>;
+  let directoryFactory: MockProxy<DirectoryFactoryService>;
+
+  let batchRequestBuilder: BatchRequestBuilder;
+  let singleRequestBuilder: SingleRequestBuilder;
+  let syncService: SyncService;
   let directoryService: LdapDirectoryService;
 
   beforeEach(() => {
     logService = mock();
     i18nService = mock();
     stateService = mock();
+    cryptoFunctionService = mock();
+    apiService = mock();
+    messagingService = mock();
+    environmentService = mock();
+    directoryFactory = mock();
 
     stateService.getDirectoryType.mockResolvedValue(DirectoryType.Ldap);
+    stateService.getOrganizationId.mockResolvedValue("fakeId");
     stateService.getLastUserSync.mockResolvedValue(null); // do not filter results by last modified date
     i18nService.t.mockImplementation((id) => id); // passthrough implementation for any error  messages
 
     directoryService = new LdapDirectoryService(logService, i18nService, stateService);
+    directoryFactory.createService.mockReturnValue(directoryService);
+
+    batchRequestBuilder = new BatchRequestBuilder();
+    singleRequestBuilder = new SingleRequestBuilder();
+
+    syncService = new SyncService(
+      cryptoFunctionService,
+      apiService,
+      messagingService,
+      i18nService,
+      environmentService,
+      stateService,
+      batchRequestBuilder,
+      singleRequestBuilder,
+      directoryFactory,
+    );
   });
 
   describe("basic sync fetching users and groups", () => {
@@ -75,6 +115,60 @@
       const result = await directoryService.getEntries(true, true);
       expect(result).toEqual([groupFixtures, userFixtures]);
     });
+
+    it("with largeImport disabled matches directory fixture data", async () => {
+      stateService.getDirectory
+        .calledWith(DirectoryType.Ldap)
+        .mockResolvedValue(getLdapConfiguration());
+      stateService.getSync.mockResolvedValue(
+        getSyncConfiguration({
+          users: true,
+          groups: true,
+        }),
+      );
+
+      cryptoFunctionService.hash.mockResolvedValue(new ArrayBuffer(1));
+      // This arranges the last hash to be differet from the ArrayBuffer after it is converted to b64
+      stateService.getLastSyncHash.mockResolvedValue("unique hash");
+
+      const syncResult = await syncService.sync(false, false);
+      const result = await directoryService.getEntries(true, true);
+
+      expect(syncResult).toEqual([groupFixtures, userFixtures]);
+      expect(result).toEqual([groupFixtures, userFixtures]);
+      expect(result).toEqual(syncResult);
+
+      expect(apiService.postPublicImportDirectory).toHaveBeenCalledTimes(1);
+    });
+
+    it("with largeImport enabled matches directory fixture data", async () => {
+      stateService.getDirectory
+        .calledWith(DirectoryType.Ldap)
+        .mockResolvedValue(getLdapConfiguration());
+      stateService.getSync.mockResolvedValue(
+        getSyncConfiguration({
+          users: true,
+          groups: true,
+          largeImport: true,
+          overwriteExisting: false,
+        }),
+      );
+
+      cryptoFunctionService.hash.mockResolvedValue(new ArrayBuffer(1));
+      // This arranges the last hash to be differet from the ArrayBuffer after it is converted to b64
+      stateService.getLastSyncHash.mockResolvedValue("unique hash");
+
+      // @ts-expect-error This is a workaround to make the batchsize smaller to trigger the batching logic since its a const.
+      constants.batchSize = 4;
+
+      const result = await directoryService.getEntries(true, true);
+      const syncResult = await syncService.sync(false, false);
+
+      expect(syncResult).toEqual([groupFixtures, userFixtures]);
+      expect(result).toEqual([groupFixtures, userFixtures]);
+      expect(result).toEqual(syncResult);
+      expect(apiService.postPublicImportDirectory).toHaveBeenCalledTimes(6);
+    });
   });
 
   describe("users", () => {

src/services/single-request-builder.spec.ts

@@ -0,0 +1,73 @@
+import { GroupEntry } from "@/src/models/groupEntry";
+import { UserEntry } from "@/src/models/userEntry";
+
+import { RequestBuilderOptions } from "../abstractions/request-builder.service";
+
+import { SingleRequestBuilder } from "./single-request-builder";
+
+describe("SingleRequestBuilder", () => {
+  let singleRequestBuilder: SingleRequestBuilder;
+
+  function userSimulator(userCount: number) {
+    return Array(userCount).fill(new UserEntry());
+  }
+
+  function groupSimulator(groupCount: number) {
+    return Array(groupCount).fill(new GroupEntry());
+  }
+
+  beforeEach(async () => {
+    singleRequestBuilder = new SingleRequestBuilder();
+  });
+
+  const defaultOptions = new RequestBuilderOptions({
+    overwriteExisting: false,
+    removeDisabled: false,
+  });
+
+  it("SingleRequestBuilder returns single request for 200 users", () => {
+    const mockGroups = groupSimulator(200);
+    const mockUsers = userSimulator(200);
+
+    const requests = singleRequestBuilder.buildRequest(mockGroups, mockUsers, defaultOptions);
+
+    expect(requests.length).toEqual(1);
+  });
+
+  it("SingleRequestBuilder returns request with overwriteExisting enabled", () => {
+    const mockGroups = groupSimulator(200);
+    const mockUsers = userSimulator(200);
+    const options = new RequestBuilderOptions({ ...defaultOptions, overwriteExisting: true });
+
+    const request = singleRequestBuilder.buildRequest(mockGroups, mockUsers, options)[0];
+    expect(request.overwriteExisting).toBe(true);
+  });
+
+  it("SingleRequestBuilder returns request with deleted user when removeDisabled is true", () => {
+    const mockGroups = groupSimulator(200);
+    const mockUsers = userSimulator(200);
+    const disabledUser = new UserEntry();
+    const options = new RequestBuilderOptions({ ...defaultOptions, removeDisabled: true });
+
+    disabledUser.disabled = true;
+    mockUsers.push(disabledUser);
+
+    const request = singleRequestBuilder.buildRequest(mockGroups, mockUsers, options)[0];
+    expect(request.members.length).toEqual(201);
+    expect(request.members.pop().deleted).toBe(true);
+  });
+
+  it("SingleRequestBuilder returns request with deleted user and overwriteExisting enabled when overwriteExisting and removeDisabled are true", () => {
+    const mockGroups = groupSimulator(200);
+    const mockUsers = userSimulator(200);
+    const disabledUser = new UserEntry();
+    const options = new RequestBuilderOptions({ overwriteExisting: true, removeDisabled: true });
+
+    disabledUser.disabled = true;
+    mockUsers.push(disabledUser);
+
+    const request = singleRequestBuilder.buildRequest(mockGroups, mockUsers, options)[0];
+    expect(request.members.pop().deleted).toBe(true);
+    expect(request.overwriteExisting).toBe(true);
+  });
+});

src/services/single-request-builder.ts

@@ -3,7 +3,7 @@ import { OrganizationImportRequest } from "@/jslib/common/src/models/request/org
 import { GroupEntry } from "@/src/models/groupEntry";
 import { UserEntry } from "@/src/models/userEntry";
 
-import { RequestBuilder } from "../abstractions/request-builder.service";
+import { RequestBuilder, RequestBuilderOptions } from "../abstractions/request-builder.service";
 
 /**
  * This class is responsible for building small (<2k users) syncs as a single
@@ -15,10 +15,7 @@ export class SingleRequestBuilder implements RequestBuilder {
   buildRequest(
     groups: GroupEntry[],
     users: UserEntry[],
-    options: {
-      removeDisabled: boolean;
-      overwriteExisting: boolean;
-    },
+    options: RequestBuilderOptions,
   ): OrganizationImportRequest[] {
     return [
       new OrganizationImportRequest({