Skip to content

Commit 16e4699

Browse files
committed
improvement(release): reuse shared helpers and name the People sync checkpoint lock
1 parent 76c37b2 commit 16e4699

4 files changed

Lines changed: 32 additions & 30 deletions

File tree

‎apps/sim/app/o/[organizationId]/people/directory-sync-form.tsx‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
type PeopleSyncStatus,
1717
peopleSyncConfigSchema,
1818
} from '@/lib/api/contracts/people-sync'
19+
import { flattenFieldErrors } from '@/lib/api/contracts/primitives'
1920
import { organizationRoutes } from '@/lib/navigation/paths'
2021
import { useOrganizationContext } from '@/app/o/[organizationId]/providers/organization-provider'
2122
import { useScimConnection } from '@/ee/scim/hooks/scim'
@@ -45,7 +46,9 @@ export function DirectorySyncForm({
4546
const [tenantId, setTenantId] = useState(current?.tenantId ?? '')
4647
const [clientId, setClientId] = useState(current?.clientId ?? '')
4748
const [secret, setSecret] = useState('')
48-
const [fieldErrors, setFieldErrors] = useState<Record<string, string>>({})
49+
const [fieldErrors, setFieldErrors] = useState<
50+
Partial<Record<'domain' | 'token' | 'tenantId' | 'clientId' | 'clientSecret', string>>
51+
>({})
4952
const { settingsFeatures } = useOrganizationContext()
5053
const provisioning = useScimConnection(organizationId, source === 'scim')
5154
const scimReady = provisioning.isSuccess && provisioning.data.connection?.status === 'active'
@@ -66,11 +69,7 @@ export function DirectorySyncForm({
6669
: { source }
6770
)
6871
if (!config.success) {
69-
setFieldErrors(
70-
Object.fromEntries(
71-
config.error.issues.map((issue) => [String(issue.path[0]), issue.message])
72-
)
73-
)
72+
setFieldErrors(flattenFieldErrors(config.error))
7473
return
7574
}
7675
setFieldErrors({})

‎apps/sim/lib/people/sync/runner.ts‎

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { db } from '@sim/db'
22
import { peopleDirectoryScimChange as change, peopleDirectorySync as sync } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
44
import { getPostgresErrorCode } from '@sim/utils/errors'
5+
import { chunkArray } from '@sim/utils/helpers'
56
import { generateId } from '@sim/utils/id'
67
import { backoffWithJitter } from '@sim/utils/retry'
78
import { and, eq, isNotNull, isNull, lte, or, sql } from 'drizzle-orm'
@@ -40,7 +41,7 @@ export const CLEARED_RUN = {
4041
cursor: null,
4142
usersRead: 0,
4243
} as const
43-
const RELEASED = { leaseToken: null, leaseExpiresAt: null } as const
44+
export const RELEASED = { leaseToken: null, leaseExpiresAt: null } as const
4445
const available = (at: Date) => or(isNull(sync.leaseToken), lte(sync.leaseExpiresAt, at))
4546
const due = (at: Date) => or(lte(sync.nextSyncAt, at), isNotNull(pendingScimChange))
4647
type Connection = typeof sync.$inferSelect
@@ -78,11 +79,11 @@ const ownsLease = (worker: Worker) =>
7879
/** Every page and its next cursor commit together, after checking that the same run still owns the source. */
7980
async function checkpoint(
8081
worker: Worker,
81-
step: 'page' | 'publish',
82-
write: (tx: DbTransaction, row: Checkpoint) => Promise<void>
82+
write: (tx: DbTransaction, row: Checkpoint) => Promise<void>,
83+
options: { lockProfiles?: boolean } = {}
8384
) {
8485
return db.transaction(async (tx) => {
85-
if (step === 'publish') await lockProfiles(tx, worker.organizationId)
86+
if (options.lockProfiles) await lockProfiles(tx, worker.organizationId)
8687
const [row] = await tx
8788
.select(checkpointColumns)
8889
.from(sync)
@@ -127,7 +128,7 @@ function retryUpdate(row: Checkpoint, providerDelay: number, reason: string) {
127128

128129
async function checkpointUsers(worker: Worker, reader: DirectoryReader, cursor: string | null) {
129130
const page = await reader.users(cursor)
130-
return checkpoint(worker, 'page', async (tx) => {
131+
return checkpoint(worker, async (tx) => {
131132
await stageUsers(tx, worker, cursor, page.people)
132133
await tx
133134
.update(sync)
@@ -147,7 +148,7 @@ async function checkpointReports(worker: Worker, reader: DirectoryReader) {
147148
throw new OrchestrationError('validation', 'The directory cannot read reporting lines.')
148149
const items = await pendingManagers(db, worker)
149150
if (!items.length) {
150-
return checkpoint(worker, 'page', async (tx) => {
151+
return checkpoint(worker, async (tx) => {
151152
await tx.update(sync).set({ phase: 'publish' }).where(ownsLease(worker))
152153
})
153154
}
@@ -168,7 +169,7 @@ async function checkpointReports(worker: Worker, reader: DirectoryReader) {
168169
const delays = results.flatMap((result) =>
169170
'retryAfterMs' in result ? [result.retryAfterMs] : []
170171
)
171-
const applied = await checkpoint(worker, 'page', async (tx, row) => {
172+
const applied = await checkpoint(worker, async (tx, row) => {
172173
await stageRelationships(tx, worker, pages)
173174
const update = delays.length
174175
? retryUpdate(
@@ -184,12 +185,16 @@ async function checkpointReports(worker: Worker, reader: DirectoryReader) {
184185

185186
/** A local SCIM generation captures, validates, and publishes atomically. New provisioning changes have their own marker. */
186187
async function runScim(worker: Worker) {
187-
await checkpoint(worker, 'publish', async (tx, row) => {
188-
await discardStage(tx, row.organizationId)
189-
const count = await stageScimDirectory(tx, row.organizationId, worker.runId, row.sourceTarget)
190-
await tx.update(sync).set({ phase: 'publish', usersRead: count }).where(ownsLease(worker))
191-
await publish(tx, worker, 'scim')
192-
})
188+
await checkpoint(
189+
worker,
190+
async (tx, row) => {
191+
await discardStage(tx, row.organizationId)
192+
const count = await stageScimDirectory(tx, row.organizationId, worker.runId, row.sourceTarget)
193+
await tx.update(sync).set({ phase: 'publish', usersRead: count }).where(ownsLease(worker))
194+
await publish(tx, worker, 'scim')
195+
},
196+
{ lockProfiles: true }
197+
)
193198
}
194199

195200
async function handleFailure(worker: Worker, error: unknown) {
@@ -203,7 +208,7 @@ async function handleFailure(worker: Worker, error: unknown) {
203208
retry || (error instanceof OrchestrationError && error.code !== 'internal')
204209
? error.message
205210
: 'Directory sync was interrupted. Sim will retry automatically.'
206-
const applied = await checkpoint(worker, 'page', async (tx, row) => {
211+
const applied = await checkpoint(worker, async (tx, row) => {
207212
if (temporary) {
208213
await tx
209214
.update(sync)
@@ -297,7 +302,7 @@ export async function runDirectorySync(organizationId: string, trigger: keyof ty
297302
if (!current) return true
298303
await requirePeopleEnabled(organizationId)
299304
if (current.phase === 'publish') {
300-
await checkpoint(worker, 'publish', (tx) => publish(tx, worker, row.source))
305+
await checkpoint(worker, (tx) => publish(tx, worker, row.source), { lockProfiles: true })
301306
return true
302307
}
303308
reader ??= await connectReader(row)
@@ -340,8 +345,8 @@ export async function syncDueDirectories() {
340345
SELECT organization_id AS "organizationId" FROM candidates
341346
GROUP BY organization_id ORDER BY min(due_at), organization_id LIMIT 50
342347
`)
343-
for (let i = 0; i < connections.length && Date.now() < deadline; i += 2) {
344-
const slice = connections.slice(i, i + 2)
348+
for (const slice of chunkArray(connections, 2)) {
349+
if (Date.now() >= deadline) break
345350
const results = await Promise.allSettled(
346351
slice.map(({ organizationId }) => runDirectorySync(organizationId, 'cron'))
347352
)

‎apps/sim/lib/people/sync/service.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { OrchestrationError } from '@/lib/core/orchestration/types'
1010
import { encryptSecret } from '@/lib/core/security/encryption'
1111
import { oktaDirectoryHost } from '@/lib/people/provider-import'
1212
import { lockProfiles } from '@/lib/people/repository'
13-
import { CLEARED_RUN, runDirectorySync } from '@/lib/people/sync/runner'
13+
import { CLEARED_RUN, RELEASED, runDirectorySync } from '@/lib/people/sync/runner'
1414
import { pendingScimChange } from '@/lib/people/sync/scim-changes'
1515
import { discardStage } from '@/lib/people/sync/stage'
1616
import { isScimEntitledForOrganization } from '@/ee/scim/lib/entitlement'
@@ -132,10 +132,9 @@ export async function configureDirectorySync(
132132
existing && existing.retryCount > 0 && existing.nextSyncAt.getTime() > Date.now()
133133
const values = {
134134
...CLEARED_RUN,
135+
...RELEASED,
135136
...stored,
136137
configuredBy: actorUserId,
137-
leaseToken: null,
138-
leaseExpiresAt: null,
139138
nextSyncAt: coolingDown ? existing.nextSyncAt : new Date(),
140139
retryCount: coolingDown ? existing.retryCount : 0,
141140
lastError: coolingDown ? existing.lastError : null,

‎packages/sim-cli/src/runtime/request.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { closeSync, existsSync, fstatSync, openSync, readSync } from 'node:fs'
2+
import { isRecordLike } from '@sim/utils/object'
23
import { CLI_CONTRACT } from '../contract/commands'
34
import type { BodyVariantSpec, CommandSpec, FlagSpec } from '../contract/types'
45
import { embedStore } from '../embed-context'
@@ -28,10 +29,8 @@ function bodyVariantShapeError(
2829
const flag = `--${variant.name}`
2930
const example = variant.kind === 'array' ? `'[{"column":"value"}]'` : `'{"column":"value"}'`
3031
const base = `${flag} must be a JSON ${variant.kind}, e.g. ${flag} ${example}; got ${jsonKind(parsed)}.`
31-
if (variant.kind === 'array' && parsed && typeof parsed === 'object' && !Array.isArray(parsed)) {
32-
if (Array.isArray((parsed as Record<string, unknown>)[variant.property])) {
33-
return `${base} Pass the array under its "${variant.property}" key itself, not the object wrapping it.`
34-
}
32+
if (variant.kind === 'array' && isRecordLike(parsed) && Array.isArray(parsed[variant.property])) {
33+
return `${base} Pass the array under its "${variant.property}" key itself, not the object wrapping it.`
3534
}
3635
const other = variants.find(
3736
(candidate) =>

0 commit comments

Comments
 (0)