diff --git a/buf.gen.yaml b/buf.gen.yaml index 7bc6513..a3c0343 100644 --- a/buf.gen.yaml +++ b/buf.gen.yaml @@ -7,6 +7,8 @@ plugins: opt: - esModuleInterop=true - importSuffix=.js + - outputJsonMethods=false + - outputPartialMethods=false - env=node - snakeToCamel=false - useDate=string diff --git a/proto/observation/v1.proto b/proto/observation/v1.proto index d2d4bc0..447d891 100644 --- a/proto/observation/v1.proto +++ b/proto/observation/v1.proto @@ -38,7 +38,7 @@ message Observation_1 { // METADATA message Metadata { - bool manualLocation = 1; + optional bool manualLocation = 1; message Position { google.protobuf.Timestamp timestamp = 1; @@ -57,10 +57,10 @@ message Observation_1 { } message PositionProvider { - bool gpsAvailable = 1; - bool passiveAvailable = 2; + optional bool gpsAvailable = 1; + optional bool passiveAvailable = 2; bool locationServicesEnabled = 3; - bool networkAvailable = 4; + optional bool networkAvailable = 4; } optional Position position = 3; diff --git a/schema/observation/v1.json b/schema/observation/v1.json index a1fc651..fe29cb3 100644 --- a/schema/observation/v1.json +++ b/schema/observation/v1.json @@ -7,6 +7,7 @@ "position": { "description": "Position details", "type": "object", + "required": ["timestamp", "coords"], "properties": { "timestamp": { "description": "Timestamp of when the current position was obtained", @@ -162,6 +163,7 @@ "positionProvider": { "description": "Details of the location providers that were available on the device when the observation was recorded", "type": "object", + "required": ["locationServicesEnabled"], "properties": { "gpsAvailable": { "description": "Whether the user has enabled GPS for device location (this is not the same as whether location is turned on or off, this is a device setting whether to use just wifi and bluetooth or use GPS for location)", diff --git a/src/lib/decode-conversions.ts b/src/lib/decode-conversions.ts index 9e3d6a6..242db2f 100644 --- a/src/lib/decode-conversions.ts +++ b/src/lib/decode-conversions.ts @@ -22,10 +22,15 @@ import { } from '../types.js' import { ExhaustivenessError, VersionIdObject, getVersionId } from './utils.js' import type { Observation, Track } from '../index.js' -import type { Observation_1_Attachment } from '../proto/observation/v1.js' +import type { + Observation_1_Attachment, + Observation_1_Metadata, + Observation_1_Metadata_Position, +} from '../proto/observation/v1.js' import type { Track_1_Position } from '../proto/track/v1.js' import { ProjectSettings_1_ConfigMetadata } from '../proto/projectSettings/v1.js' import { ProjectSettings } from '../schema/projectSettings.js' +import type { Position } from '../schema/observation.js' /** Function type for converting a protobuf type of any version for a particular * schema name, and returning the most recent JSONSchema type */ @@ -76,7 +81,7 @@ export const convertObservation: ConvertFunction<'observation'> = ( message, versionObj ) => { - const { common, schemaVersion, ...rest } = message + const { common, metadata, schemaVersion, ...rest } = message const jsonSchemaCommon = convertCommon(common, versionObj) let presetRef @@ -95,7 +100,7 @@ export const convertObservation: ConvertFunction<'observation'> = ( ...rest, attachments: message.attachments.map(convertAttachment), tags: convertTags(message.tags), - metadata: message.metadata, + metadata: metadata ? removeInvalidPositionMetadata(metadata) : {}, presetRef, } return obs @@ -453,3 +458,32 @@ function convertTrackPosition( timestamp: position.timestamp, } } + +/** + * Because of the way protobuf works, it's possible that a protobuf message is + * missing required fields. In this case `timestamp` and the `latitude` and + * `longitude` fields on `coords`. We shouldn't have any observations with these + * fields missing, but if we do, rather than throwing (and not indexing the + * observation at all), we remove the position metadata, since it is not useful + * without this metadata. + */ +function removeInvalidPositionMetadata( + metadata: Observation_1_Metadata +): Observation['metadata'] { + const { position, lastSavedPosition, ...rest } = metadata + return { + ...rest, + position: position && removeInvalidPosition(position), + lastSavedPosition: + lastSavedPosition && removeInvalidPosition(lastSavedPosition), + } +} + +function removeInvalidPosition( + position: Observation_1_Metadata_Position +): Position | undefined { + if (position.coords === undefined || position.timestamp === undefined) { + return undefined + } + return position as Position +} diff --git a/src/lib/encode-conversions.ts b/src/lib/encode-conversions.ts index 74afc8f..a2cd415 100644 --- a/src/lib/encode-conversions.ts +++ b/src/lib/encode-conversions.ts @@ -95,9 +95,6 @@ export const convertObservation: ConvertFunction<'observation'> = ( mapeoDoc ) => { const attachments = mapeoDoc.attachments.map(convertAttachment) - const metadata: Observation_1_Metadata | undefined = mapeoDoc.metadata && { - ...Observation_1_Metadata.fromPartial(mapeoDoc.metadata), - } let presetRef if (mapeoDoc.presetRef) { presetRef = { @@ -111,7 +108,6 @@ export const convertObservation: ConvertFunction<'observation'> = ( ...mapeoDoc, attachments, tags: convertTags(mapeoDoc.tags), - metadata, presetRef, } } diff --git a/test/fixtures/bad-docs.js b/test/fixtures/bad-docs.js index 4e37aaf..88e6190 100644 --- a/test/fixtures/bad-docs.js +++ b/test/fixtures/bad-docs.js @@ -39,7 +39,6 @@ export const badDocs = [ index: 123, }), ], - refs: [], attachments: [], tags: {}, metadata: {}, diff --git a/test/index.test.js b/test/index.test.js index 0ea13aa..7418bac 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -19,6 +19,7 @@ import { goodDocsCompleted, badDocs, } from './fixtures/index.js' +import { cachedValues } from './fixtures/cached.js' test('Bad docs throw when encoding', () => { for (const { text, doc } of badDocs) { @@ -225,6 +226,75 @@ test(`test encoding of wrongly formatted header`, async () => { }) }) +/** @type {import('../dist/index.js').Observation} */ +const minimalObservation = { + docId: cachedValues.docId, + versionId: cachedValues.versionId, + originalVersionId: cachedValues.originalVersionId, + schemaName: 'observation', + createdAt: cachedValues.createdAt, + updatedAt: cachedValues.updatedAt, + links: [], + lat: 24.0424, + lon: 21.0214, + attachments: [], + tags: {}, + metadata: {}, + deleted: false, +} + +test(`encoding observation with missing position metadata`, async () => { + /** @type {import('../dist/index.js').Observation} */ + const doc = { + ...minimalObservation, + metadata: { + position: /** @type {any} */ ({ coords: {} }), + }, + } + const buf = encode(doc) + const decodedDoc = decode(buf, parseVersionId(doc.versionId)) + assert.equal(decodedDoc.schemaName, 'observation') + // a previous bug meant that protobuf defaults of 0 were being set for lat/lon + assert.equal( + typeof decodedDoc.metadata?.position?.coords?.longitude, + 'undefined' + ) + assert.equal( + typeof decodedDoc.metadata?.position?.coords?.latitude, + 'undefined' + ) +}) + +test(`decoding observation with missing position provider props`, async () => { + /** @type {import('../dist/index.js').Observation} */ + const doc = { + ...minimalObservation, + metadata: { + positionProvider: { + locationServicesEnabled: true, + }, + }, + } + const buf = encode(doc) + const decodedDoc = decode(buf, parseVersionId(doc.versionId)) + assert.equal(decodedDoc.schemaName, 'observation') + assert.equal( + typeof decodedDoc.metadata?.positionProvider?.gpsAvailable, + 'undefined', + 'optional gpsAvailable prop should be undefined' + ) + assert.equal( + typeof decodedDoc.metadata?.positionProvider?.networkAvailable, + 'undefined', + 'optional networkAvailable prop should be undefined' + ) + assert.equal( + typeof decodedDoc.metadata?.positionProvider?.passiveAvailable, + 'undefined', + 'optional passiveAvailable prop should be undefined' + ) +}) + /** * Remove undefined properties (deeply) from an object, by round-tripping to * JSON. Also handles Buffers via JSON.parse reviver