Skip to content

Commit ce85064

Browse files
authored
fix: better source validation and refine required source fields (#1895)
## Summary Large refactor changing the TSource type to a true discriminated union. This means that the expected fields for `kind: 'log'` will differ from those for `'trace', 'session', 'metrics'`. This avoids the current laissez faire source type that currently exists, and required extensive changes across the api and app packages. Also includes a nice addition to `useSource` - you can now specify a `kind` field, which will properly infer the type of the returned source. This also makes use of discriminators in mongoose. This does change a bit of the way that we create and update sources. Obvious changes to sources have also been made, namely making `timeValueExpression` required on sources. Care has been taken to avoid requiring a migration. ### How to test locally or on Vercel 1. `yarn dev` 2. Play around with the app, especially around source creation, source edits, and loading existing sources from a previous version ### References - Linear Issue: References HDX-3352 - Related PRs: Ref: HDX-3352
1 parent a36b350 commit ce85064

File tree

89 files changed

+1883
-800
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

89 files changed

+1883
-800
lines changed

.changeset/six-ways-sell.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@hyperdx/common-utils": patch
3+
"@hyperdx/api": patch
4+
"@hyperdx/app": patch
5+
---
6+
7+
fix: change sources to discriminated union

packages/api/src/controllers/sources.ts

Lines changed: 72 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,31 @@
1-
import { SourceKind } from '@hyperdx/common-utils/dist/types';
2-
3-
import { ISource, Source } from '@/models/source';
4-
5-
/**
6-
* Clean up metricTables property when changing source type away from Metric.
7-
* This prevents metric-specific configuration from persisting when switching
8-
* to Log, Trace, or Session sources.
9-
*/
10-
function cleanSourceData(source: Omit<ISource, 'id'>): Omit<ISource, 'id'> {
11-
// Only clean metricTables if the source is not a Metric type
12-
if (source.kind !== SourceKind.Metric) {
13-
// explicitly setting to null for mongoose to clear column
14-
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
15-
source.metricTables = null as any;
16-
}
1+
import { SourceKind, SourceSchema } from '@hyperdx/common-utils/dist/types';
2+
3+
import {
4+
ISourceInput,
5+
LogSource,
6+
MetricSource,
7+
SessionSource,
8+
Source,
9+
TraceSource,
10+
} from '@/models/source';
1711

18-
return source;
12+
// Returns the discriminator model for the given source kind.
13+
// Updates must go through the correct discriminator model so Mongoose
14+
// recognises kind-specific fields (e.g. metricTables on MetricSource).
15+
function getModelForKind(kind: SourceKind) {
16+
switch (kind) {
17+
case SourceKind.Log:
18+
return LogSource;
19+
case SourceKind.Trace:
20+
return TraceSource;
21+
case SourceKind.Session:
22+
return SessionSource;
23+
case SourceKind.Metric:
24+
return MetricSource;
25+
default:
26+
kind satisfies never;
27+
throw new Error(`${kind} is not a valid SourceKind`);
28+
}
1929
}
2030

2131
export function getSources(team: string) {
@@ -26,19 +36,56 @@ export function getSource(team: string, sourceId: string) {
2636
return Source.findOne({ _id: sourceId, team });
2737
}
2838

29-
export function createSource(team: string, source: Omit<ISource, 'id'>) {
30-
return Source.create({ ...source, team });
39+
type DistributiveOmit<T, K extends PropertyKey> = T extends T
40+
? Omit<T, K>
41+
: never;
42+
43+
export function createSource(
44+
team: string,
45+
source: DistributiveOmit<ISourceInput, 'id'>,
46+
) {
47+
// @ts-expect-error The create method has incompatible type signatures but is actually safe
48+
return getModelForKind(source.kind)?.create({ ...source, team });
3149
}
3250

33-
export function updateSource(
51+
export async function updateSource(
3452
team: string,
3553
sourceId: string,
36-
source: Omit<ISource, 'id'>,
54+
source: DistributiveOmit<ISourceInput, 'id'>,
3755
) {
38-
const cleanedSource = cleanSourceData(source);
39-
return Source.findOneAndUpdate({ _id: sourceId, team }, cleanedSource, {
40-
new: true,
41-
});
56+
const existing = await Source.findOne({ _id: sourceId, team });
57+
if (!existing) return null;
58+
59+
// Same kind: simple update through the discriminator model
60+
if (existing.kind === source.kind) {
61+
// @ts-expect-error The findOneAndUpdate method has incompatible type signatures but is actually safe
62+
return getModelForKind(source.kind)?.findOneAndUpdate(
63+
{ _id: sourceId, team },
64+
source,
65+
{ new: true },
66+
);
67+
}
68+
69+
// Kind changed: validate through Zod before writing since the raw
70+
// collection bypass skips Mongoose's discriminator validation.
71+
const parseResult = SourceSchema.safeParse(source);
72+
if (!parseResult.success) {
73+
throw new Error(
74+
`Invalid source data: ${parseResult.error.errors.map(e => e.message).join(', ')}`,
75+
);
76+
}
77+
78+
// Use replaceOne on the raw collection to swap the entire document
79+
// in place (including the discriminator key). This is a single atomic
80+
// write — the document is never absent from the collection.
81+
const replacement = {
82+
...parseResult.data,
83+
_id: existing._id,
84+
team: existing.team,
85+
updatedAt: new Date(),
86+
};
87+
await Source.collection.replaceOne({ _id: existing._id }, replacement);
88+
return getModelForKind(replacement.kind)?.hydrate(replacement);
4289
}
4390

4491
export function deleteSource(team: string, sourceId: string) {

0 commit comments

Comments
 (0)