Add lexicon LexXrpcParameters, Object required properties check (#1040)

* Add lexicon LexXrpcParameters, Object required properties check

Now schemas like the one below won't validate
```
...
{

  "type": "object",
  "required": ["foo"],
  "properties": {}
}
...
````

* Fix lexicons with missing required fields

* Improve lexicon user type error messages and document why it needs to be `z.custom`

lexicons/app/bsky/graph/defs.json

@@ -4,7 +4,7 @@
   "defs": {
     "listViewBasic": {
       "type": "object",
-      "required": ["uri", "creator", "name", "purpose"],
+      "required": ["uri", "name", "purpose"],
       "properties": {
         "uri": {"type": "string", "format": "at-uri"},
         "name": {"type": "string", "maxLength": 64, "minLength": 1},

lexicons/com/atproto/repo/applyWrites.json

@@ -39,7 +39,7 @@
     "create": {
       "type": "object",
       "description": "Create a new record.",
-      "required": ["action", "collection", "value"],
+      "required": ["collection", "value"],
       "properties": {
         "collection": {"type": "string", "format": "nsid"},
         "rkey": {"type": "string", "maxLength": 15},
@@ -49,7 +49,7 @@
     "update": {
       "type": "object",
       "description": "Update an existing record.",
-      "required": ["action", "collection", "rkey", "value"],
+      "required": ["collection", "rkey", "value"],
       "properties": {
         "collection": {"type": "string", "format": "nsid"},
         "rkey": {"type": "string"},
@@ -59,7 +59,7 @@
     "delete": {
       "type": "object",
       "description": "Delete an existing record.",
-      "required": ["action", "collection", "rkey"],
+      "required": ["collection", "rkey"],
       "properties": {
         "collection": {"type": "string", "format": "nsid"},
         "rkey": {"type": "string"}

packages/api/src/client/lexicons.ts

@@ -1611,7 +1611,7 @@ export const schemaDict = {
       create: {
         type: 'object',
         description: 'Create a new record.',
-        required: ['action', 'collection', 'value'],
+        required: ['collection', 'value'],
         properties: {
           collection: {
             type: 'string',
@@ -1629,7 +1629,7 @@ export const schemaDict = {
       update: {
         type: 'object',
         description: 'Update an existing record.',
-        required: ['action', 'collection', 'rkey', 'value'],
+        required: ['collection', 'rkey', 'value'],
         properties: {
           collection: {
             type: 'string',
@@ -1646,7 +1646,7 @@ export const schemaDict = {
       delete: {
         type: 'object',
         description: 'Delete an existing record.',
-        required: ['action', 'collection', 'rkey'],
+        required: ['collection', 'rkey'],
         properties: {
           collection: {
             type: 'string',
@@ -5277,7 +5277,7 @@ export const schemaDict = {
     defs: {
       listViewBasic: {
         type: 'object',
-        required: ['uri', 'creator', 'name', 'purpose'],
+        required: ['uri', 'name', 'purpose'],
         properties: {
           uri: {
             type: 'string',

packages/bsky/src/lexicon/lexicons.ts

@@ -1611,7 +1611,7 @@ export const schemaDict = {
       create: {
         type: 'object',
         description: 'Create a new record.',
-        required: ['action', 'collection', 'value'],
+        required: ['collection', 'value'],
         properties: {
           collection: {
             type: 'string',
@@ -1628,7 +1628,7 @@ export const schemaDict = {
       update: {
         type: 'object',
         description: 'Update an existing record.',
-        required: ['action', 'collection', 'rkey', 'value'],
+        required: ['collection', 'rkey', 'value'],
         properties: {
           collection: {
             type: 'string',
@@ -1645,7 +1645,7 @@ export const schemaDict = {
       delete: {
         type: 'object',
         description: 'Delete an existing record.',
-        required: ['action', 'collection', 'rkey'],
+        required: ['collection', 'rkey'],
         properties: {
           collection: {
             type: 'string',

packages/lexicon/src/types.ts

@@ -1,5 +1,6 @@
 import { z } from 'zod'
 import { NSID } from '@atproto/nsid'
+import { requiredPropertiesRefinement } from './util'
 
 // primitives
 // =
@@ -141,28 +142,32 @@ export const lexToken = z.object({
 })
 export type LexToken = z.infer<typeof lexToken>
 
-export const lexObject = z.object({
-  type: z.literal('object'),
-  description: z.string().optional(),
-  required: z.string().array().optional(),
-  nullable: z.string().array().optional(),
-  properties: z
-    .record(
-      z.union([lexRefVariant, lexIpldType, lexArray, lexBlob, lexPrimitive]),
-    )
-    .optional(),
-})
+export const lexObject = z
+  .object({
+    type: z.literal('object'),
+    description: z.string().optional(),
+    required: z.string().array().optional(),
+    nullable: z.string().array().optional(),
+    properties: z
+      .record(
+        z.union([lexRefVariant, lexIpldType, lexArray, lexBlob, lexPrimitive]),
+      )
+      .optional(),
+  })
+  .superRefine(requiredPropertiesRefinement)
 export type LexObject = z.infer<typeof lexObject>
 
 // xrpc
 // =
 
-export const lexXrpcParameters = z.object({
-  type: z.literal('params'),
-  description: z.string().optional(),
-  required: z.string().array().optional(),
-  properties: z.record(z.union([lexPrimitive, lexPrimitiveArray])),
-})
+export const lexXrpcParameters = z
+  .object({
+    type: z.literal('params'),
+    description: z.string().optional(),
+    required: z.string().array().optional(),
+    properties: z.record(z.union([lexPrimitive, lexPrimitiveArray])),
+  })
+  .superRefine(requiredPropertiesRefinement)
 export type LexXrpcParameters = z.infer<typeof lexXrpcParameters>
 
 export const lexXrpcBody = z.object({
@@ -229,26 +234,91 @@ export type LexRecord = z.infer<typeof lexRecord>
 // core
 // =
 
-export const lexUserType = z.discriminatedUnion('type', [
-  lexRecord,
+// We need to use `z.custom` here because
+// lexXrpcProperty and lexObject are refined
+// `z.union` would work, but it's too slow
+// see #915 for details
+export const lexUserType = z.custom<
+  | LexRecord
+  | LexXrpcQuery
+  | LexXrpcProcedure
+  | LexXrpcSubscription
+  | LexBlob
+  | LexArray
+  | LexToken
+  | LexObject
+  | LexBoolean
+  | LexInteger
+  | LexString
+  | LexBytes
+  | LexCidLink
+  | LexUnknown
+>(
+  (val) => {
+    if (!val || typeof val !== 'object') {
+      return
+    }
 
-  lexXrpcQuery,
-  lexXrpcProcedure,
-  lexXrpcSubscription,
+    if (val['type'] === undefined) {
+      return
+    }
 
-  lexBlob,
+    switch (val['type']) {
+      case 'record':
+        return lexRecord.parse(val)
 
-  lexArray,
-  lexToken,
-  lexObject,
+      case 'query':
+        return lexXrpcQuery.parse(val)
+      case 'procedure':
+        return lexXrpcProcedure.parse(val)
+      case 'subscription':
+        return lexXrpcSubscription.parse(val)
 
-  lexBoolean,
-  lexInteger,
-  lexString,
-  lexBytes,
-  lexCidLink,
-  lexUnknown,
-])
+      case 'blob':
+        return lexBlob.parse(val)
+
+      case 'array':
+        return lexArray.parse(val)
+      case 'token':
+        return lexToken.parse(val)
+      case 'object':
+        return lexObject.parse(val)
+
+      case 'boolean':
+        return lexBoolean.parse(val)
+      case 'integer':
+        return lexInteger.parse(val)
+      case 'string':
+        return lexString.parse(val)
+      case 'bytes':
+        return lexBytes.parse(val)
+      case 'cid-link':
+        return lexCidLink.parse(val)
+      case 'unknown':
+        return lexUnknown.parse(val)
+    }
+  },
+  (val) => {
+    if (!val || typeof val !== 'object') {
+      return {
+        message: 'Must be an object',
+        fatal: true,
+      }
+    }
+
+    if (val['type'] === undefined) {
+      return {
+        message: 'Must have a type',
+        fatal: true,
+      }
+    }
+
+    return {
+      message: `Invalid type: ${val['type']} must be one of: record, query, procedure, subscription, blob, array, token, object, boolean, integer, string, bytes, cid-link, unknown`,
+      fatal: true,
+    }
+  },
+)
 export type LexUserType = z.infer<typeof lexUserType>
 
 export const lexiconDoc = z

packages/lexicon/src/util.ts

@@ -7,6 +7,7 @@ import {
   ValidationResult,
   isDiscriminatedObject,
 } from './types'
+import { z } from 'zod'
 
 export function toLexUri(str: string, baseUri?: string): string {
   if (str.startsWith('lex:')) {
@@ -104,3 +105,43 @@ export function toConcreteTypes(
     return [def]
   }
 }
+
+export function requiredPropertiesRefinement<
+  ObjectType extends {
+    required?: string[]
+    properties?: Record<string, unknown>
+  },
+>(object: ObjectType, ctx: z.RefinementCtx) {
+  // Required fields check
+  if (object.required === undefined) {
+    return
+  }
+
+  if (!Array.isArray(object.required)) {
+    ctx.addIssue({
+      code: z.ZodIssueCode.invalid_type,
+      received: typeof object.required,
+      expected: 'array',
+    })
+    return
+  }
+
+  if (object.properties === undefined) {
+    if (object.required.length > 0) {
+      ctx.addIssue({
+        code: z.ZodIssueCode.custom,
+        message: `Required fields defined but no properties defined`,
+      })
+    }
+    return
+  }
+
+  for (const field of object.required) {
+    if (object.properties[field] === undefined) {
+      ctx.addIssue({
+        code: z.ZodIssueCode.custom,
+        message: `Required field "${field}" not defined`,
+      })
+    }
+  }
+}

packages/lexicon/tests/general.test.ts

@@ -1,5 +1,6 @@
 import { CID } from 'multiformats/cid'
-import { Lexicons } from '../src/index'
+import { lexiconDoc, Lexicons } from '../src/index'
+import { object } from '../src/validators/complex'
 import LexiconDocs from './_scaffolds/lexicons'
 
 describe('Lexicons collection', () => {
@@ -84,6 +85,22 @@ describe('General validation', () => {
       expect(res.error?.message).toBe('Object must have the property "object"')
     }
   })
+  it('fails when a required property is missing', () => {
+    const schema = {
+      lexicon: 1,
+      id: 'com.example.kitchenSink',
+      defs: {
+        test: {
+          type: 'object',
+          required: ['foo'],
+          properties: {},
+        },
+      },
+    }
+    expect(() => {
+      lexiconDoc.parse(schema)
+    }).toThrow('Required field \\"foo\\" not defined')
+  })
 })
 
 describe('Record validation', () => {

packages/pds/src/lexicon/lexicons.ts

@@ -1611,7 +1611,7 @@ export const schemaDict = {
       create: {
         type: 'object',
         description: 'Create a new record.',
-        required: ['action', 'collection', 'value'],
+        required: ['collection', 'value'],
         properties: {
           collection: {
             type: 'string',
@@ -1629,7 +1629,7 @@ export const schemaDict = {
       update: {
         type: 'object',
         description: 'Update an existing record.',
-        required: ['action', 'collection', 'rkey', 'value'],
+        required: ['collection', 'rkey', 'value'],
         properties: {
           collection: {
             type: 'string',
@@ -1646,7 +1646,7 @@ export const schemaDict = {
       delete: {
         type: 'object',
         description: 'Delete an existing record.',
-        required: ['action', 'collection', 'rkey'],
+        required: ['collection', 'rkey'],
         properties: {
           collection: {
             type: 'string',
@@ -5277,7 +5277,7 @@ export const schemaDict = {
     defs: {
       listViewBasic: {
         type: 'object',
-        required: ['uri', 'creator', 'name', 'purpose'],
+        required: ['uri', 'name', 'purpose'],
         properties: {
           uri: {
             type: 'string',