Skip to content

Commit

Permalink
Apply code review suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
gaearon committed Nov 1, 2023
1 parent 404e3f5 commit 399a1fd
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 54 deletions.
5 changes: 2 additions & 3 deletions packages/lex-cli/src/util.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import fs from 'fs'
import { join } from 'path'
import { lexiconDoc, LexiconDoc } from '@atproto/lexicon'
import { parseLexiconDoc, LexiconDoc } from '@atproto/lexicon'
import { ZodError, ZodFormattedError } from 'zod'
import chalk from 'chalk'
import { GeneratedAPI, FileDiff } from './types'
Expand Down Expand Up @@ -41,8 +41,7 @@ export function readLexicon(path: string): LexiconDoc {
typeof (obj as LexiconDoc).lexicon === 'number'
) {
try {
lexiconDoc.parse(obj)
return obj as LexiconDoc
return parseLexiconDoc(obj)
} catch (e) {
console.error(`Invalid lexicon`, path)
if (e instanceof ZodError) {
Expand Down
33 changes: 6 additions & 27 deletions packages/lexicon/src/lexicons.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
import { ZodError } from 'zod'
import {
LexiconDoc,
lexiconDoc,
LexRecord,
LexXrpcProcedure,
LexXrpcQuery,
LexUserType,
LexiconDocMalformedError,
LexiconDefNotFoundError,
InvalidLexiconError,
ValidationResult,
Expand All @@ -32,7 +29,7 @@ export class Lexicons {
docs: Map<string, LexiconDoc> = new Map()
defs: Map<string, LexUserType> = new Map()

constructor(docs?: unknown[]) {
constructor(docs?: LexiconDoc[]) {
if (docs?.length) {
for (const doc of docs) {
this.add(doc)
Expand All @@ -43,37 +40,19 @@ export class Lexicons {
/**
* Add a lexicon doc.
*/
add(doc: unknown): void {
if (process.env.NODE_ENV !== 'production') {
try {
lexiconDoc.parse(doc)
} catch (e) {
if (e instanceof ZodError) {
throw new LexiconDocMalformedError(
`Failed to parse schema definition ${
(doc as Record<string, string>).id
}`,
doc,
e.issues,
)
} else {
throw e
}
}
}
const validatedDoc = doc as LexiconDoc
const uri = toLexUri(validatedDoc.id)
add(doc: LexiconDoc): void {
const uri = toLexUri(doc.id)
if (this.docs.has(uri)) {
throw new Error(`${uri} has already been registered`)
}

// WARNING
// mutates the object
// -prf
resolveRefUris(validatedDoc, uri)
resolveRefUris(doc, uri)

this.docs.set(uri, validatedDoc)
for (const [defUri, def] of iterDefs(validatedDoc)) {
this.docs.set(uri, doc)
for (const [defUri, def] of iterDefs(doc)) {
this.defs.set(defUri, def)
}
}
Expand Down
13 changes: 3 additions & 10 deletions packages/lexicon/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -415,16 +415,9 @@ export function isDiscriminatedObject(
return discriminatedObject.safeParse(value).success
}

export class LexiconDocMalformedError extends Error {
constructor(
message: string,
public schemaDef: unknown,
public issues?: z.ZodIssue[],
) {
super(message)
this.schemaDef = schemaDef
this.issues = issues
}
export function parseLexiconDoc(v: unknown): LexiconDoc {
lexiconDoc.parse(v)
return v as LexiconDoc
}

export type ValidationResult =
Expand Down
6 changes: 5 additions & 1 deletion packages/lexicon/tests/_scaffolds/lexicons.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
export default [
import { LexiconDoc } from '../../src/index'

const lexicons: LexiconDoc[] = [
{
lexicon: 1,
id: 'com.example.kitchenSink',
Expand Down Expand Up @@ -521,3 +523,5 @@ export default [
},
},
]

export default lexicons
12 changes: 6 additions & 6 deletions packages/lexicon/tests/general.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { CID } from 'multiformats/cid'
import { lexiconDoc, Lexicons } from '../src/index'
import { LexiconDoc, Lexicons, parseLexiconDoc } from '../src/index'
import LexiconDocs from './_scaffolds/lexicons'

describe('Lexicons collection', () => {
Expand Down Expand Up @@ -97,7 +97,7 @@ describe('General validation', () => {
},
}
expect(() => {
lexiconDoc.parse(schema)
parseLexiconDoc(schema)
}).toThrow('Required field \\"foo\\" not defined')
})
it('fails when unknown fields are present', () => {
Expand All @@ -113,11 +113,11 @@ describe('General validation', () => {
}

expect(() => {
lexiconDoc.parse(schema)
parseLexiconDoc(schema)
}).toThrow("Unrecognized key(s) in object: 'foo'")
})
it('fails lexicon parsing when uri is invalid', () => {
const schema = {
const schema: LexiconDoc = {
lexicon: 1,
id: 'com.example.invalidUri',
defs: {
Expand All @@ -135,7 +135,7 @@ describe('General validation', () => {
}).toThrow('Uri can only have one hash segment')
})
it('fails validation when ref uri has multiple hash segments', () => {
const schema = {
const schema: LexiconDoc = {
lexicon: 1,
id: 'com.example.invalidUri',
defs: {
Expand Down Expand Up @@ -168,7 +168,7 @@ describe('General validation', () => {
}).toThrow('Uri can only have one hash segment')
})
it('union handles both implicit and explicit #main', () => {
const schemas = [
const schemas: LexiconDoc[] = [
{
lexicon: 1,
id: 'com.example.implicitMain',
Expand Down
9 changes: 5 additions & 4 deletions packages/xrpc-server/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import express, {
RequestHandler,
} from 'express'
import {
LexiconDoc,
Lexicons,
lexToJson,
LexXrpcProcedure,
Expand Down Expand Up @@ -45,7 +46,7 @@ import {
import log from './logger'
import { consumeMany } from './rate-limiter'

export function createServer(lexicons?: unknown[], options?: Options) {
export function createServer(lexicons?: LexiconDoc[], options?: Options) {
return new Server(lexicons, options)
}

Expand All @@ -60,7 +61,7 @@ export class Server {
sharedRateLimiters: Record<string, RateLimiterI>
routeRateLimiterFns: Record<string, RateLimiterConsume[]>

constructor(lexicons?: unknown[], opts?: Options) {
constructor(lexicons?: LexiconDoc[], opts?: Options) {
if (lexicons) {
this.addLexicons(lexicons)
}
Expand Down Expand Up @@ -140,11 +141,11 @@ export class Server {
// schemas
// =

addLexicon(doc: unknown) {
addLexicon(doc: LexiconDoc) {
this.lex.add(doc)
}

addLexicons(docs: unknown[]) {
addLexicons(docs: LexiconDoc[]) {
for (const doc of docs) {
this.addLexicon(doc)
}
Expand Down
6 changes: 3 additions & 3 deletions packages/xrpc/src/client.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Lexicons, ValidationError } from '@atproto/lexicon'
import { LexiconDoc, Lexicons, ValidationError } from '@atproto/lexicon'
import {
getMethodSchemaHTTPMethod,
constructMethodCallUri,
Expand Down Expand Up @@ -46,11 +46,11 @@ export class Client {
// schemas
// =

addLexicon(doc: unknown) {
addLexicon(doc: LexiconDoc) {
this.lex.add(doc)
}

addLexicons(docs: unknown[]) {
addLexicons(docs: LexiconDoc[]) {
for (const doc of docs) {
this.addLexicon(doc)
}
Expand Down

0 comments on commit 399a1fd

Please sign in to comment.