-
Notifications
You must be signed in to change notification settings - Fork 88
[PM-14360] Import Batching #703
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.
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
Changes from 9 commits
1249419
80b863d
c25d4be
e08ef3d
c7cde29
d0f260d
3b0d3c1
c26431b
79cf720
dfb54b7
6c0506e
87f4cb2
47f564b
53a0d56
c160e50
27583e3
73cdfb2
2bb7f2d
ac210f8
f3a682a
9349c12
e96581c
7d7623e
a48335c
9d8fbf0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import { DirectoryType } from "@/src/enums/directoryType"; | ||
import { IDirectoryService } from "@/src/services/directory.service"; | ||
|
||
export abstract class DirectoryFactoryService { | ||
abstract createService(type: DirectoryType): IDirectoryService; | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,13 @@ | ||||||
import { OrganizationImportRequest } from "@/jslib/common/src/models/request/organizationImportRequest"; | ||||||
|
||||||
import { GroupEntry } from "@/src/models/groupEntry"; | ||||||
import { UserEntry } from "@/src/models/userEntry"; | ||||||
|
||||||
export abstract class RequestBuilderAbstratction { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
(then your names should be good!) |
||||||
buildRequest: ( | ||||||
groups: GroupEntry[], | ||||||
users: UserEntry[], | ||||||
removeDisabled: boolean, | ||||||
overwriteExisting: boolean, | ||||||
) => OrganizationImportRequest[]; | ||||||
} |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You've duplicated this file instead of renaming it (there is now a |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
import { OrganizationImportRequest } from "@/jslib/common/src/models/request/organizationImportRequest"; | ||
|
||
import { GroupEntry } from "@/src/models/groupEntry"; | ||
import { UserEntry } from "@/src/models/userEntry"; | ||
|
||
import { RequestBuilderAbstratction } from "../abstractions/request-builder.service"; | ||
|
||
import { batchSize } from "./sync.service"; | ||
|
||
/** | ||
* This class is responsible for batching large sync requests (>=2k users) into multiple smaller | ||
* requests to the /import REST endpoint. This is done to ensure we are under the default | ||
* maximum packet size for NGINX web servers to avoid the request potentially timing out | ||
* */ | ||
export class DefaultBatchRequestBuilder implements RequestBuilderAbstratction { | ||
buildRequest( | ||
groups: GroupEntry[], | ||
users: UserEntry[], | ||
removeDisabled: boolean, | ||
overwriteExisting: boolean, | ||
): OrganizationImportRequest[] { | ||
const requests: OrganizationImportRequest[] = []; | ||
|
||
if (users.length > 0) { | ||
const usersRequest = users.map((u) => { | ||
return { | ||
email: u.email, | ||
externalId: u.externalId, | ||
deleted: u.deleted || (removeDisabled && u.disabled), | ||
}; | ||
}); | ||
|
||
// Partition users | ||
for (let i = 0; i < usersRequest.length; i += batchSize) { | ||
const u = usersRequest.slice(i, i + batchSize); | ||
const req = new OrganizationImportRequest({ | ||
groups: [], | ||
users: u, | ||
largeImport: true, | ||
overwriteExisting, | ||
}); | ||
requests.push(req); | ||
} | ||
} | ||
|
||
if (groups.length > 0) { | ||
const groupRequest = groups.map((g) => { | ||
return { | ||
name: g.name, | ||
externalId: g.externalId, | ||
memberExternalIds: Array.from(g.userMemberExternalIds), | ||
}; | ||
}); | ||
|
||
// Partition groups | ||
for (let i = 0; i < groupRequest.length; i += batchSize) { | ||
const g = groupRequest.slice(i, i + batchSize); | ||
const req = new OrganizationImportRequest({ | ||
groups: g, | ||
users: [], | ||
largeImport: true, | ||
overwriteExisting, | ||
}); | ||
requests.push(req); | ||
} | ||
} | ||
|
||
return requests; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
import { GroupEntry } from "@/src/models/groupEntry"; | ||
import { UserEntry } from "@/src/models/userEntry"; | ||
|
||
import { DefaultBatchRequestBuilder } from "./default-batch-request-builder"; | ||
import { DefaultSingleRequestBuilder } from "./default-single-request-builder"; | ||
|
||
describe("BatchingService", () => { | ||
eliykat marked this conversation as resolved.
Show resolved
Hide resolved
|
||
let batchRequestBuilder: DefaultBatchRequestBuilder; | ||
let singleRequestBuilder: DefaultSingleRequestBuilder; | ||
|
||
function userSimulator(userCount: number) { | ||
return Array(userCount).fill(new UserEntry()); | ||
} | ||
|
||
function groupSimulator(groupCount: number) { | ||
return Array(groupCount).fill(new GroupEntry()); | ||
} | ||
|
||
beforeEach(async () => { | ||
batchRequestBuilder = new DefaultBatchRequestBuilder(); | ||
singleRequestBuilder = new DefaultSingleRequestBuilder(); | ||
}); | ||
|
||
it("BatchRequestBuilder batches requests for > 2000 users", () => { | ||
const mockGroups = groupSimulator(11000); | ||
const mockUsers = userSimulator(11000); | ||
|
||
const requests = batchRequestBuilder.buildRequest(mockGroups, mockUsers, true, true); | ||
|
||
expect(requests.length).toEqual(12); | ||
}); | ||
|
||
it("SingleRequestBuilder returns single request for 200 users", () => { | ||
const mockGroups = groupSimulator(200); | ||
const mockUsers = userSimulator(200); | ||
|
||
const requests = singleRequestBuilder.buildRequest(mockGroups, mockUsers, true, true); | ||
|
||
expect(requests.length).toEqual(1); | ||
}); | ||
|
||
it("BatchRequestBuilder retuns an empty array when there are no users or groups", () => { | ||
const requests = batchRequestBuilder.buildRequest([], [], true, true); | ||
|
||
expect(requests).toEqual([]); | ||
}); | ||
}); |
eliykat marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
import { OrganizationImportRequest } from "@/jslib/common/src/models/request/organizationImportRequest"; | ||
|
||
import { GroupEntry } from "@/src/models/groupEntry"; | ||
import { UserEntry } from "@/src/models/userEntry"; | ||
|
||
import { RequestBuilderAbstratction } from "../abstractions/request-builder.service"; | ||
|
||
/** | ||
* This class is responsible for building small (<2k users) syncs as a single | ||
* request to the /import REST endpoint. This is done to be backwards compatiblle with | ||
* existing functionality for sync requests that are sufficiently small enough to not | ||
* exceed default maximum packet size limits on NGINX web servers. | ||
* */ | ||
export class DefaultSingleRequestBuilder implements RequestBuilderAbstratction { | ||
buildRequest( | ||
groups: GroupEntry[], | ||
users: UserEntry[], | ||
removeDisabled: boolean, | ||
overwriteExisting: boolean, | ||
): OrganizationImportRequest[] { | ||
return [ | ||
new OrganizationImportRequest({ | ||
groups: (groups ?? []).map((g) => { | ||
return { | ||
name: g.name, | ||
externalId: g.externalId, | ||
memberExternalIds: Array.from(g.userMemberExternalIds), | ||
}; | ||
}), | ||
users: (users ?? []).map((u) => { | ||
return { | ||
email: u.email, | ||
externalId: u.externalId, | ||
deleted: u.deleted || (removeDisabled && u.disabled), | ||
}; | ||
}), | ||
overwriteExisting: overwriteExisting, | ||
largeImport: false, | ||
}), | ||
]; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
import { I18nService } from "@/jslib/common/src/abstractions/i18n.service"; | ||
import { LogService } from "@/jslib/common/src/abstractions/log.service"; | ||
|
||
import { DirectoryFactoryService } from "../abstractions/directory-factory.service"; | ||
import { StateService } from "../abstractions/state.service"; | ||
import { DirectoryType } from "../enums/directoryType"; | ||
|
||
import { AzureDirectoryService } from "./azure-directory.service"; | ||
import { GSuiteDirectoryService } from "./gsuite-directory.service"; | ||
import { LdapDirectoryService } from "./ldap-directory.service"; | ||
import { OktaDirectoryService } from "./okta-directory.service"; | ||
import { OneLoginDirectoryService } from "./onelogin-directory.service"; | ||
|
||
export class DefaultDirectoryFactoryService implements DirectoryFactoryService { | ||
constructor( | ||
private logService: LogService, | ||
private i18nService: I18nService, | ||
private stateService: StateService, | ||
) {} | ||
|
||
createService(directoryType: DirectoryType) { | ||
switch (directoryType) { | ||
case DirectoryType.GSuite: | ||
return new GSuiteDirectoryService(this.logService, this.i18nService, this.stateService); | ||
case DirectoryType.AzureActiveDirectory: | ||
return new AzureDirectoryService(this.logService, this.i18nService, this.stateService); | ||
case DirectoryType.Ldap: | ||
return new LdapDirectoryService(this.logService, this.i18nService, this.stateService); | ||
case DirectoryType.Okta: | ||
return new OktaDirectoryService(this.logService, this.i18nService, this.stateService); | ||
case DirectoryType.OneLogin: | ||
return new OneLoginDirectoryService(this.logService, this.i18nService, this.stateService); | ||
default: | ||
throw new Error("Invalid Directory Type"); | ||
} | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.