Skip to content
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.

Sign up for GitHub

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

feat!: replace createdBy with originalVersionId #191

Merged
merged 5 commits into from
Aug 24, 2024
Merged

feat!: replace createdBy with originalVersionId #191

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2cdd3a5
feat!: replace `createdBy` with `originalVersionId`
EvanHahn Jul 10, 2024
c980048
Merge branch 'main' into originalVersionId
EvanHahn Aug 15, 2024
e3024be
Encode originalVersionId as a `VersionId` message, not a string
EvanHahn Aug 15, 2024
d1b0975
OmitUnion -> SetOptional
EvanHahn Aug 15, 2024
144b9f2
stricter types for MapeoDoc encoding
gmaclennan Aug 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions proto/common/v1.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ message Common_1 {
repeated Link links = 2;
google.protobuf.Timestamp createdAt = 3 [(required) = true];
google.protobuf.Timestamp updatedAt = 4 [(required) = true];
// 32-byte hash of the discovery key of a core
bytes createdBy = 5 [(required) = true];
optional string originalVersionId = 5;
EvanHahn marked this conversation as resolved.
Show resolved Hide resolved
bool deleted = 6 [(required) = true];
}
/* ignored fields and differences from common.json jsonSchema
Expand Down
10 changes: 5 additions & 5 deletions schema/common/v1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@
"description": "core discovery id (hex-encoded 32-byte buffer) and core index number, separated by '/'",
"type": "string"
},
"originalVersionId": {
"description": "Version ID of the original version of this document. For the original version, matches `versionId`.",
"type": "string"
},
"schemaName": {
"description": "Name of Mapeo data type / schema",
"type": "string"
Expand All @@ -22,10 +26,6 @@
"type": "string",
"format": "date-time"
},
"createdBy": {
"description": "discovery id (hex-encoded 32-byte buffer) of the hypercore where the first version of this document is written",
"type": "string"
},
"updatedAt": {
"description": "RFC3339-formatted datetime of when this version of the element was created",
"type": "string",
Expand All @@ -51,7 +51,7 @@
"updatedAt",
"links",
"versionId",
"createdBy",
"originalVersionId",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does get confusing, because is this required or not? It seems from digidem/comapeo-core#715 that we are not storing this on creation, so reads will not include this/

Trying to fully represent the type in JSONSchema might be difficult, it might be easier to do in the TypeScript defs in this repo, maybe something like:

type CommonNew = (Omit<Common, 'links' | 'originalVersionId'> & { links: [] }) | SetRequired<Common, 'originalVersionId'>

e.g. you originalVersionId can be missing if link is an empty array, but otherwise it is required.

"deleted"
]
}
2 changes: 1 addition & 1 deletion src/encode.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { ExhaustivenessError } from './lib/utils.js'
* with the encoded data type ID and schema version, to send to an hypercore.
*/
export function encode(
mapeoDoc: OmitUnion<MapeoDocInternal, 'versionId'>
mapeoDoc: OmitUnion<MapeoDocInternal, 'versionId' | 'originalVersionId'>
EvanHahn marked this conversation as resolved.
Show resolved Hide resolved
): Buffer {
const { schemaName } = mapeoDoc
const schemaVersion = currentSchemaVersions[schemaName]
Expand Down
15 changes: 13 additions & 2 deletions src/lib/decode-conversions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,13 +346,24 @@ function convertCommon(
throw new Error('Missing required common properties')
}

const versionId = getVersionId(versionObj)

/** @type {string} */ let originalVersionId
if (common.originalVersionId) {
originalVersionId = common.originalVersionId
} else if (common.links.length === 0) {
originalVersionId = versionId
} else {
throw new Error('Cannot determine original version ID; data is malformed')
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see re. my comment above about it being required in JSON Schema is maybe ok given you are always appending it here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But maybe the encode function could benefit from the stricter typing and this same check when writing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I addressed this in d1b0975.

return {
docId: common.docId.toString('hex'),
versionId: getVersionId(versionObj),
versionId,
originalVersionId,
links: common.links.map((link) => getVersionId(link)),
createdAt: common.createdAt,
updatedAt: common.updatedAt,
createdBy: common.createdBy.toString('hex'),
deleted: common.deleted,
}
}
Expand Down
10 changes: 6 additions & 4 deletions src/lib/encode-conversions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { SetOptional } from 'type-fest'
import { CurrentProtoTypes } from '../proto/types.js'
import {
ProtoTypesWithSchemaInfo,
Expand Down Expand Up @@ -28,7 +29,7 @@ import {
* schema name, and returning the most recent JSONSchema type */
type ConvertFunction<TSchemaName extends SchemaName> = (
mapeoDoc: Extract<
OmitUnion<MapeoDocInternal, 'versionId'>,
OmitUnion<MapeoDocInternal, 'versionId' | 'originalVersionId'>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're loosing some type safety where you are doing this, but it's late so maybe I'm missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I did this in d1b0975, but maybe you were imagining something else.

{ schemaName: TSchemaName }
>
) => CurrentProtoTypes[TSchemaName]
Expand Down Expand Up @@ -127,7 +128,8 @@ export const convertDeviceInfo: ConvertFunction<'deviceInfo'> = (mapeoDoc) => {
}

export const convertCoreOwnership = (
mapeoDoc: Omit<CoreOwnership, 'versionId'> & CoreOwnershipSignatures
mapeoDoc: Omit<CoreOwnership, 'versionId' | 'originalVersionId'> &
CoreOwnershipSignatures
): CurrentProtoTypes['coreOwnership'] => {
return {
common: convertCommon(mapeoDoc),
Expand Down Expand Up @@ -218,13 +220,13 @@ export const convertTrack: ConvertFunction<'track'> = (mapeoDoc) => {
}

function convertCommon(
common: Omit<MapeoCommon, 'versionId'>
common: SetOptional<MapeoCommon, 'versionId' | 'originalVersionId'>
): ProtoTypesWithSchemaInfo['common'] {
return {
docId: Buffer.from(common.docId, 'hex'),
originalVersionId: common.originalVersionId,
createdAt: common.createdAt,
updatedAt: common.updatedAt,
createdBy: Buffer.from(common.createdBy, 'hex'),
links: common.links.map((link) => parseVersionId(link)),
deleted: common.deleted,
}
Expand Down
4 changes: 2 additions & 2 deletions src/lib/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export function parseVersionId(versionId: string): VersionIdObject {
/**
* @template {import('@mapeo/schema').MapeoDoc & { forks?: string[] }} T
* @param {T} doc
* @returns {Omit<T, 'docId' | 'versionId' | 'links' | 'forks' | 'createdAt' | 'updatedAt' | 'createdBy' | 'deleted'>}
* @returns {Omit<T, 'docId' | 'versionId' | 'originalVersionId' | 'links' | 'forks' | 'createdAt' | 'updatedAt' | 'deleted'>}
*/
export function valueOf<TDoc extends MapeoDoc>(
doc: TDoc & { forks?: string[] }
Expand All @@ -71,11 +71,11 @@ export function valueOf<TDoc extends MapeoDoc>(
const {
docId,
versionId,
originalVersionId,
links,
forks,
createdAt,
updatedAt,
createdBy,
deleted,
...rest
} = doc
Expand Down
30 changes: 26 additions & 4 deletions test/fixtures/bad-docs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ export const badDocs = [
doc: {
docId: cachedValues.docId,
versionId: cachedValues.versionId,
originalVersionId: cachedValues.versionId,
schemaName: 'observOtion',
createdAt: cachedValues.createdAt,
updatedAt: cachedValues.updatedAt,
createdBy: cachedValues.createdBy,
links: [],
refs: [],
attachments: [],
Expand All @@ -25,16 +25,38 @@ export const badDocs = [
deleted: false,
},
},
{
text: 'missing expected originalVersionId',
/** @type Omit<import('../../dist/index.js').Observation, 'originalVersionId'> */
doc: {
docId: cachedValues.docId,
versionId: cachedValues.versionId,
schemaName: 'observation',
createdAt: cachedValues.createdAt,
updatedAt: cachedValues.updatedAt,
links: [
JSON.stringify({
coreDiscoveryKey: Buffer.from('abc123'),
index: 123,
}),
],
refs: [],
attachments: [],
tags: {},
metadata: {},
deleted: false,
},
},
{
text: 'role doc with empty roleId',
/** @type {import('../../dist/index.js').Role} */
doc: {
docId: cachedValues.docId,
versionId: cachedValues.versionId,
originalVersionId: cachedValues.versionId,
schemaName: 'role',
createdAt: cachedValues.createdAt,
updatedAt: cachedValues.updatedAt,
createdBy: cachedValues.createdBy,
links: [],
roleId: '',
fromIndex: 4,
Expand All @@ -46,10 +68,10 @@ export const badDocs = [
doc: {
docId: cachedValues.docId,
versionId: cachedValues.versionId,
originalVersionId: cachedValues.versionId,
schemaName: 'icon',
createdAt: cachedValues.createdAt,
updatedAt: cachedValues.updatedAt,
createdBy: cachedValues.createdBy,
links: [],
deleted: false,
variants: [
Expand All @@ -67,10 +89,10 @@ export const badDocs = [
doc: {
docId: cachedValues.docId,
versionId: cachedValues.versionId,
originalVersionId: cachedValues.versionId,
schemaName: 'preset',
createdAt: cachedValues.createdAt,
updatedAt: cachedValues.updatedAt,
createdBy: cachedValues.createdBy,
links: [],
name: 'myPreset',
geometry: ['point'],
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/cached.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ const date = new Date().toJSON()
export const cachedValues = {
docId: randomBytes(32).toString('hex'),
versionId: `${randomBytes(32).toString('hex')}/0`,
originalVersionId: `${randomBytes(32).toString('hex')}/0`,
projectId: randomBytes(32).toString('hex'),
authorId: randomBytes(32).toString('hex'),
coreId: randomBytes(32).toString('hex'),
createdBy: randomBytes(32).toString('hex'),
createdAt: date,
updatedAt: date,
attachments: {
Expand Down
22 changes: 11 additions & 11 deletions test/fixtures/good-docs-completed.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ export const goodDocsCompleted = [
doc: {
docId: cachedValues.docId,
versionId: cachedValues.versionId,
originalVersionId: cachedValues.originalVersionId,
schemaName: 'observation',
createdAt: cachedValues.createdAt,
updatedAt: cachedValues.updatedAt,
createdBy: cachedValues.createdBy,
links: [],
lat: 24.0424,
lon: 21.0214,
Expand Down Expand Up @@ -75,10 +75,10 @@ export const goodDocsCompleted = [
doc: {
docId: cachedValues.docId,
versionId: cachedValues.versionId,
originalVersionId: cachedValues.originalVersionId,
schemaName: 'projectSettings',
createdAt: cachedValues.createdAt,
updatedAt: cachedValues.updatedAt,
createdBy: cachedValues.createdBy,
links: [],
defaultPresets: {
point: cachedValues.defaultPresets.point,
Expand All @@ -96,10 +96,10 @@ export const goodDocsCompleted = [
doc: {
docId: cachedValues.docId,
versionId: cachedValues.versionId,
originalVersionId: cachedValues.originalVersionId,
schemaName: 'field',
createdAt: cachedValues.createdAt,
updatedAt: cachedValues.updatedAt,
createdBy: cachedValues.createdBy,
links: [],
tagKey: 'otherTagKey',
type: 'number',
Expand All @@ -121,10 +121,10 @@ export const goodDocsCompleted = [
doc: {
docId: cachedValues.docId,
versionId: cachedValues.versionId,
originalVersionId: cachedValues.originalVersionId,
schemaName: 'preset',
createdAt: cachedValues.createdAt,
updatedAt: cachedValues.updatedAt,
createdBy: cachedValues.createdBy,
links: [],
name: 'myPreset',
geometry: ['point', 'vertex', 'line'],
Expand All @@ -150,10 +150,10 @@ export const goodDocsCompleted = [
doc: {
docId: cachedValues.docId,
versionId: cachedValues.versionId,
originalVersionId: cachedValues.originalVersionId,
schemaName: 'role',
createdAt: cachedValues.createdAt,
updatedAt: cachedValues.updatedAt,
createdBy: cachedValues.createdBy,
links: [],
roleId: '6fd029a78243',
fromIndex: 5,
Expand All @@ -165,10 +165,10 @@ export const goodDocsCompleted = [
doc: {
docId: cachedValues.docId,
versionId: cachedValues.versionId,
originalVersionId: cachedValues.originalVersionId,
schemaName: 'deviceInfo',
createdAt: cachedValues.createdAt,
updatedAt: cachedValues.updatedAt,
createdBy: cachedValues.createdBy,
links: [],
name: 'my device name',
deviceType: 'desktop',
Expand All @@ -180,10 +180,10 @@ export const goodDocsCompleted = [
doc: {
docId: cachedValues.docId,
versionId: cachedValues.versionId,
originalVersionId: cachedValues.originalVersionId,
schemaName: 'deviceInfo',
createdAt: cachedValues.createdAt,
updatedAt: cachedValues.updatedAt,
createdBy: cachedValues.createdBy,
links: [],
name: 'my device name',
// @ts-expect-error
Expand All @@ -198,10 +198,10 @@ export const goodDocsCompleted = [
doc: {
docId: cachedValues.docId,
versionId: cachedValues.versionId,
originalVersionId: cachedValues.originalVersionId,
schemaName: 'coreOwnership',
createdAt: cachedValues.createdAt,
updatedAt: cachedValues.updatedAt,
createdBy: cachedValues.createdBy,
links: [],
authCoreId: Buffer.from('authCoreId').toString('hex'),
configCoreId: Buffer.from('configCoreId').toString('hex'),
Expand All @@ -224,11 +224,11 @@ export const goodDocsCompleted = [
doc: {
docId: cachedValues.docId,
versionId: cachedValues.versionId,
originalVersionId: cachedValues.originalVersionId,
schemaName: 'icon',
name: 'tree',
createdAt: cachedValues.createdAt,
updatedAt: cachedValues.updatedAt,
createdBy: cachedValues.createdBy,
links: [],
deleted: false,
variants: [
Expand All @@ -251,10 +251,10 @@ export const goodDocsCompleted = [
doc: {
docId: cachedValues.docId,
versionId: cachedValues.versionId,
originalVersionId: cachedValues.originalVersionId,
schemaName: 'translation',
createdAt: cachedValues.createdAt,
updatedAt: cachedValues.updatedAt,
createdBy: cachedValues.createdBy,
links: [],
deleted: false,
schemaNameRef: 'preset',
Expand All @@ -270,10 +270,10 @@ export const goodDocsCompleted = [
doc: {
docId: cachedValues.docId,
versionId: cachedValues.versionId,
originalVersionId: cachedValues.originalVersionId,
schemaName: 'track',
createdAt: cachedValues.createdAt,
updatedAt: cachedValues.updatedAt,
createdBy: cachedValues.createdBy,
links: [],
deleted: false,
locations: [
Expand Down
Loading
Loading