Skip to content

Commit 96c7ff9

Browse files
gallaylgallayl
committed
review fixes, bulk remove fixes
1 parent 5555624 commit 96c7ff9

8 files changed

Lines changed: 116 additions & 79 deletions

File tree

‎common/src/models/service-state-history.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export type ServiceStateEvent =
1414
| 'build-completed'
1515
| 'build-failed'
1616
| 'pull-completed'
17+
| 'imported'
1718

1819
/**
1920
* How a state change was triggered.

‎frontend/src/pages/services/service-detail.tsx‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ const eventLabels: Record<string, string> = {
3838
'build-completed': 'Build completed',
3939
'build-failed': 'Build failed',
4040
'pull-completed': 'Pull completed',
41+
'imported': 'Imported',
4142
}
4243

4344
type ServiceDetailProps = {
@@ -86,8 +87,8 @@ export const ServiceDetail = Shade<ServiceDetailProps>({
8687
)
8788
}
8889

89-
const service = serviceState.data as ServiceView | undefined
90-
if (!service) {
90+
const serviceData = serviceState.data as (ServiceDefinition & Partial<ServiceView>) | undefined
91+
if (!serviceData) {
9192
return (
9293
<PageContainer>
9394
<PageHeader
@@ -106,13 +107,32 @@ export const ServiceDetail = Shade<ServiceDetailProps>({
106107
)
107108
}
108109

110+
const service: ServiceView = {
111+
...serviceData,
112+
serviceId: serviceData.serviceId ?? serviceData.id,
113+
autoFetchEnabled: serviceData.autoFetchEnabled ?? false,
114+
autoFetchIntervalMinutes: serviceData.autoFetchIntervalMinutes ?? 60,
115+
autoRestartOnFetch: serviceData.autoRestartOnFetch ?? false,
116+
installStatus: serviceData.installStatus ?? 'not-installed',
117+
buildStatus: serviceData.buildStatus ?? 'not-built',
118+
runStatus: serviceData.runStatus ?? 'stopped',
119+
updatedAt: serviceData.updatedAt,
120+
}
121+
109122
const stackState = useEntitySync(options, StackDefinition, service.stackName)
110123
const reposState = useCollectionSync(options, GitHubRepository, {
111124
filter: { stackName: { $eq: service.stackName } },
112125
})
113126
const repos = reposState.status === 'synced' || reposState.status === 'cached' ? reposState.data : []
114127
const linkedRepo = service.repositoryId ? repos.find((r) => r.id === service.repositoryId) : undefined
115-
const stack = stackState.status === 'synced' ? (stackState.data as StackView | undefined) : undefined
128+
const stackData = stackState.status === 'synced' ? stackState.data : undefined
129+
const stack = stackData
130+
? ({
131+
...stackData,
132+
stackName: (stackData as Partial<StackView>).stackName ?? stackData.name,
133+
mainDirectory: (stackData as Partial<StackView>).mainDirectory ?? '',
134+
} as StackView)
135+
: undefined
116136
const fullCwd = stack ? getServiceCwd(stack, service, linkedRepo ?? null) : null
117137

118138
const api = injector.getInstance(ServicesApiClient)

‎service/src/app-models/stacks/actions/import-stack-action.ts‎

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -100,33 +100,36 @@ export const ImportStackAction: RequestAction<ImportStackEndpoint> = async ({ in
100100
await historyDs.add(injector, {
101101
id: 0,
102102
serviceId: svcDef.id,
103-
event: 'install-completed',
103+
event: 'imported',
104104
newState: JSON.stringify({ installStatus: 'not-installed', buildStatus: 'not-built', runStatus: 'stopped' }),
105105
triggeredBy: 'system',
106-
triggerSource: 'api',
107-
metadata: JSON.stringify({ action: 'import' }),
106+
triggerSource: 'system',
107+
metadata: JSON.stringify({ action: 'import', stackName }),
108108
createdAt: now,
109109
})
110110
}
111111
} catch (error) {
112112
await logger.warning({ message: `Import failed for stack ${stackName}, rolling back`, data: { error } })
113113

114-
for (const svcDef of serviceDefinitions) {
115-
await historyDs
116-
.find(injector, { filter: { serviceId: { $eq: svcDef.id } } })
117-
.then(async (entries) => {
118-
for (const e of entries) await historyDs.remove(injector, e.id).catch(() => {})
119-
})
120-
.catch(() => {})
121-
await svcStatusDs.remove(injector, svcDef.id).catch(() => {})
122-
await svcConfigDs.remove(injector, svcDef.id).catch(() => {})
123-
await svcDefDs.remove(injector, svcDef.id).catch(() => {})
114+
const svcIds = serviceDefinitions.map((s) => s.id)
115+
for (const svcId of svcIds) {
116+
const historyEntries = await historyDs
117+
.find(injector, { filter: { serviceId: { $eq: svcId } } })
118+
.catch(() => [] as ServiceStateHistory[])
119+
if (historyEntries.length > 0) {
120+
await historyDs.remove(injector, ...historyEntries.map((e) => e.id)).catch(() => {})
121+
}
122+
}
123+
if (svcIds.length > 0) {
124+
await svcStatusDs.remove(injector, ...svcIds).catch(() => {})
125+
await svcConfigDs.remove(injector, ...svcIds).catch(() => {})
126+
await svcDefDs.remove(injector, ...svcIds).catch(() => {})
124127
}
125-
for (const dep of dependencies) {
126-
await depDs.remove(injector, dep.id).catch(() => {})
128+
if (dependencies.length > 0) {
129+
await depDs.remove(injector, ...dependencies.map((d) => d.id)).catch(() => {})
127130
}
128-
for (const repo of repositories) {
129-
await repoDs.remove(injector, repo.id).catch(() => {})
131+
if (repositories.length > 0) {
132+
await repoDs.remove(injector, ...repositories.map((r) => r.id)).catch(() => {})
130133
}
131134
await stackConfigDs.remove(injector, stackName).catch(() => {})
132135
await stackDefDs.remove(injector, stackName).catch(() => {})

‎service/src/app-models/stacks/setup-stacks-rest-api.ts‎

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@ export const setupStacksRestApi = async (injector: Injector) => {
4040
})
4141
const configs = await repo.getDataSetFor(StackConfig, 'stackName').find(i, {})
4242
const configMap = new Map(configs.map((c) => [c.stackName, c]))
43-
const entries = defs.map((def) => ({ ...def, ...configMap.get(def.name)! }) as StackView)
43+
const entries = defs
44+
.filter((def) => configMap.has(def.name))
45+
.map((def) => ({ ...def, ...configMap.get(def.name) }) as StackView)
4446
const count = await repo.getDataSetFor(StackDefinition, 'name').count(i, query.findOptions?.filter)
4547
return JsonResult({ count, entries })
4648
},
@@ -113,35 +115,28 @@ export const setupStacksRestApi = async (injector: Injector) => {
113115

114116
const svcDs = repo.getDataSetFor(ServiceDefinition, 'id')
115117
const svcs = await svcDs.find(i, { filter: { stackName: { $eq: id } }, select: ['id'] })
116-
for (const svc of svcs) {
117-
await repo
118-
.getDataSetFor(ServiceStatus, 'serviceId')
119-
.remove(i, svc.id)
120-
.catch(() => {})
121-
await repo
122-
.getDataSetFor(ServiceConfig, 'serviceId')
123-
.remove(i, svc.id)
124-
.catch(() => {})
125-
await svcDs.remove(i, svc.id).catch(() => {})
118+
const svcIds = svcs.map((svc) => svc.id)
119+
if (svcIds.length > 0) {
120+
await repo.getDataSetFor(ServiceStatus, 'serviceId').remove(i, ...svcIds).catch(() => {})
121+
await repo.getDataSetFor(ServiceConfig, 'serviceId').remove(i, ...svcIds).catch(() => {})
122+
await svcDs.remove(i, ...svcIds).catch(() => {})
126123
}
127124

128125
const repos = await repo
129126
.getDataSetFor(GitHubRepository, 'id')
130127
.find(i, { filter: { stackName: { $eq: id } }, select: ['id'] })
131-
for (const r of repos)
132-
await repo
133-
.getDataSetFor(GitHubRepository, 'id')
134-
.remove(i, r.id)
135-
.catch(() => {})
128+
const repoIds = repos.map((r) => r.id)
129+
if (repoIds.length > 0) {
130+
await repo.getDataSetFor(GitHubRepository, 'id').remove(i, ...repoIds).catch(() => {})
131+
}
136132

137133
const deps = await repo
138134
.getDataSetFor(Dependency, 'id')
139135
.find(i, { filter: { stackName: { $eq: id } }, select: ['id'] })
140-
for (const d of deps)
141-
await repo
142-
.getDataSetFor(Dependency, 'id')
143-
.remove(i, d.id)
144-
.catch(() => {})
136+
const depIds = deps.map((d) => d.id)
137+
if (depIds.length > 0) {
138+
await repo.getDataSetFor(Dependency, 'id').remove(i, ...depIds).catch(() => {})
139+
}
145140

146141
await repo
147142
.getDataSetFor(StackConfig, 'stackName')

‎service/src/config.spec.ts‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,23 @@
1+
import { Injector } from '@furystack/inject'
2+
import { getRepository } from '@furystack/repository'
3+
import { PublicApiToken } from 'common'
14
import { describe, expect, it } from 'vitest'
25

3-
describe('Config tests', () => {
4-
it('Should match the snapshot', () => {
5-
expect(true)
6+
import { authorizedOnly, injector } from './config.js'
7+
8+
describe('Config', () => {
9+
it('should create an injector instance', () => {
10+
expect(injector).toBeInstanceOf(Injector)
11+
})
12+
13+
it('should register a PublicApiToken dataset', () => {
14+
const ds = getRepository(injector).getDataSetFor(PublicApiToken, 'id')
15+
expect(ds).toBeDefined()
16+
})
17+
18+
it('authorizedOnly should reject unauthenticated requests', async () => {
19+
const testInjector = new Injector()
20+
const result = await authorizedOnly({ injector: testInjector })
21+
expect(result.isAllowed).toBe(false)
622
})
723
})

‎service/src/services/git-watcher.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ export class GitWatcher {
5555
if (!config?.autoFetchEnabled) return
5656

5757
const cwd = await resolveServiceCwd(getInjectorReference(this), svc, elevated)
58-
const intervalMs = (config.autoFetchIntervalMinutes || 60) * 60 * 1000
58+
const intervalMs = (config.autoFetchIntervalMinutes ?? 60) * 60 * 1000
5959

6060
const { remote } = await this.git.getBranches(cwd).catch(() => ({ remote: [] as string[] }))
6161

‎service/src/services/log-storage-service.ts‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,8 @@ export class LogStorageService {
8181
select: ['id'],
8282
})
8383

84-
for (const entry of entries) {
85-
await ds.remove(elevated, entry.id)
86-
}
87-
8884
if (entries.length > 0) {
85+
await ds.remove(elevated, ...entries.map((entry) => entry.id))
8986
await this.logger.information({ message: `Cleared ${entries.length} log entries for service ${serviceId}` })
9087
}
9188
}
@@ -105,11 +102,8 @@ export class LogStorageService {
105102
select: ['id'],
106103
})
107104

108-
for (const entry of oldEntries) {
109-
await ds.remove(elevated, entry.id)
110-
}
111-
112105
if (oldEntries.length > 0) {
106+
await ds.remove(elevated, ...oldEntries.map((entry) => entry.id))
113107
await this.logger.verbose({ message: `Pruned ${oldEntries.length} old log entries for service ${serviceId}` })
114108
}
115109
}

‎service/src/services/process-manager.ts‎

Lines changed: 34 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,11 @@ export class ProcessManager {
5757
event: ServiceStateEvent,
5858
trigger: TriggerContext,
5959
metadata?: Record<string, unknown>,
60+
options?: { skipHistory?: boolean },
6061
) {
6162
try {
6263
const elevated = this.getElevatedInjector()
6364
const statusDs = getRepository(elevated).getDataSetFor(ServiceStatus, 'serviceId')
64-
const historyDs = getRepository(elevated).getDataSetFor(ServiceStateHistory, 'id')
6565

6666
const statuses = await statusDs.find(elevated, { filter: { serviceId: { $eq: serviceId } }, top: 1 })
6767
const current = statuses[0]
@@ -74,31 +74,35 @@ export class ProcessManager {
7474
if (update.buildStatus === 'built') patchData.lastBuiltAt = now
7575
if (update.runStatus === 'running') patchData.lastStartedAt = now
7676

77-
const previousState = JSON.stringify({
78-
installStatus: current.installStatus,
79-
buildStatus: current.buildStatus,
80-
runStatus: current.runStatus,
81-
})
82-
8377
await statusDs.update(elevated, serviceId, patchData)
8478

85-
const newState = JSON.stringify({
86-
installStatus: update.installStatus ?? current.installStatus,
87-
buildStatus: update.buildStatus ?? current.buildStatus,
88-
runStatus: update.runStatus ?? current.runStatus,
89-
})
90-
91-
await historyDs.add(elevated, {
92-
id: 0,
93-
serviceId,
94-
event,
95-
previousState,
96-
newState,
97-
triggeredBy: trigger.triggeredBy,
98-
triggerSource: trigger.triggerSource,
99-
metadata: metadata ? JSON.stringify(metadata) : undefined,
100-
createdAt: now,
101-
})
79+
if (!options?.skipHistory) {
80+
const historyDs = getRepository(elevated).getDataSetFor(ServiceStateHistory, 'id')
81+
82+
const previousState = JSON.stringify({
83+
installStatus: current.installStatus,
84+
buildStatus: current.buildStatus,
85+
runStatus: current.runStatus,
86+
})
87+
88+
const newState = JSON.stringify({
89+
installStatus: update.installStatus ?? current.installStatus,
90+
buildStatus: update.buildStatus ?? current.buildStatus,
91+
runStatus: update.runStatus ?? current.runStatus,
92+
})
93+
94+
await historyDs.add(elevated, {
95+
id: 0,
96+
serviceId,
97+
event,
98+
previousState,
99+
newState,
100+
triggeredBy: trigger.triggeredBy,
101+
triggerSource: trigger.triggerSource,
102+
metadata: metadata ? JSON.stringify(metadata) : undefined,
103+
createdAt: now,
104+
})
105+
}
102106

103107
void this.ws.announce({
104108
type: 'service-status-changed',
@@ -127,7 +131,9 @@ export class ProcessManager {
127131
this.pendingOperations.add(serviceId)
128132
try {
129133
await this.logger.information({ message: `Starting service: ${svc.displayName}` })
130-
await this.updateServiceStatus(serviceId, { runStatus: 'starting' }, 'run-started', trigger)
134+
await this.updateServiceStatus(serviceId, { runStatus: 'starting' }, 'run-started', trigger, undefined, {
135+
skipHistory: true,
136+
})
131137

132138
const cwd = await resolveServiceCwd(getInjectorReference(this), svc, elevated)
133139
const child = this.spawnCommand(svc.runCommand, cwd)
@@ -180,7 +186,9 @@ export class ProcessManager {
180186
}
181187

182188
await this.logger.information({ message: `Stopping service: ${serviceId}` })
183-
await this.updateServiceStatus(serviceId, { runStatus: 'stopping' }, 'run-stopped', trigger)
189+
await this.updateServiceStatus(serviceId, { runStatus: 'stopping' }, 'run-stopped', trigger, undefined, {
190+
skipHistory: true,
191+
})
184192

185193
managed.process.kill('SIGTERM')
186194

0 commit comments

Comments
 (0)