fix(dandi/mvc): make @RequestParam params optional by default

- clean up duplicative decorator utility functions

closes #42

packages/dandi/core/src/injectable-metadata.ts

@@ -12,15 +12,15 @@ export function methodTarget<T>(target: Constructor<T>): MethodTarget<T> {
 }
 
 export interface ParamMetadata<T> {
-  name: string;
-  token?: InjectionToken<T>;
-  providers?: Array<Provider<any>>;
-  optional?: boolean;
+  name: string
+  token?: InjectionToken<T>
+  providers?: Array<Provider<any>>
+  optional?: boolean
 }
 
 export interface InjectableMetadata {
-  paramNames?: string[];
-  params: Array<ParamMetadata<any>>;
+  paramNames?: string[]
+  params: Array<ParamMetadata<any>>
 }
 
 export const getInjectableMetadata: MetadataAccessor<InjectableMetadata> = getMetadata.bind(null, META_KEY, () => ({

packages/dandi/mvc/src/default.route.handler.spec.ts

@@ -1,11 +1,10 @@
 import { Uuid } from '@dandi/common'
-import { Repository, InjectorContext } from '@dandi/core'
 import { stubHarness } from '@dandi/core/testing'
 import { ModelBuilderModule } from '@dandi/model-builder'
 import {
   DefaultRouteHandler,
   HttpMethod, MimeTypes,
-  MissingPathParamError,
+  MissingParamError,
   MvcRequest,
   MvcResponse,
   MvcResponseRenderer,
@@ -195,7 +194,7 @@ describe('DefaultRouteHandler', function() {
       this.registerController(new TestController())
 
       await expect(this.invokeHandler())
-        .to.be.rejectedWith(MissingPathParamError)
+        .to.be.rejectedWith(MissingParamError)
 
     })
   })

packages/dandi/mvc/src/errors.ts

@@ -43,8 +43,8 @@ export class ModelBindingError extends RequestError {
   }
 }
 
-export class MissingPathParamError extends RequestError {
+export class MissingParamError extends RequestError {
   constructor(paramName: string, innerError?: Error) {
-    super(HttpStatusCode.badRequest, null, `Missing required path param ${paramName}`, innerError)
+    super(HttpStatusCode.badRequest, null, `Missing required param ${paramName}`, innerError)
   }
 }

packages/dandi/mvc/src/query.param.decorator.ts

@@ -1,23 +1,10 @@
-import { InjectionToken, Provider } from '@dandi/core'
-import { MemberMetadata } from '@dandi/model'
 import { ConvertedType } from '@dandi/model-builder'
 
-import { requestParamDecorator, requestParamProvider, requestParamToken } from './request.param.decorator'
+import {
+  makeRequestParamDecorator,
+} from './request.param.decorator'
 import { RequestQueryParamMap } from './tokens'
 
-export function QueryParam<T>(type?: ConvertedType, name?: string): ParameterDecorator {
-  return requestParamDecorator.bind(null, RequestQueryParamMap, type || String, name)
-}
-
-export function queryParamToken<T>(paramName: string, requestParamName: string): InjectionToken<T> {
-  return requestParamToken(RequestQueryParamMap, paramName, requestParamName)
-}
-
-export function queryParamProvider<T>(
-  paramToken: InjectionToken<T>,
-  type: ConvertedType,
-  paramName: string,
-  memberMetadata: MemberMetadata,
-): Provider<T> {
-  return requestParamProvider(RequestQueryParamMap, paramToken, type, paramName, memberMetadata)
+export function QueryParam<T>(type?: ConvertedType, name?: string, required: boolean = false): ParameterDecorator {
+  return makeRequestParamDecorator(RequestQueryParamMap, type || String, name, !required)
 }

packages/dandi/mvc/src/request-decorator.int-spec.ts

@@ -0,0 +1,98 @@
+import { Url } from '@dandi/common'
+import { testHarness } from '@dandi/core/testing'
+import { Required, UrlProperty } from '@dandi/model'
+import { ModelBuilderModule } from '@dandi/model-builder'
+import {
+  MissingParamError,
+  MvcRequest,
+  PathParam,
+  QueryParam,
+  RequestBody,
+  RequestPathParamMap,
+  RequestQueryParamMap,
+} from '@dandi/mvc'
+
+import { expect } from 'chai'
+
+describe('Request Decorators', () => {
+  class TestModel {
+    @UrlProperty()
+    @Required()
+    public url: Url;
+  }
+  class TestController {
+    public testBody(@RequestBody(TestModel) body: TestModel): TestModel {
+      return body
+    }
+
+    public testPathParam(@PathParam(String) id: string): string {
+      return id
+    }
+
+    public testQueryParam(@QueryParam(String) search?: string): string {
+      return search
+    }
+  }
+
+  const harness = testHarness(ModelBuilderModule,
+    {
+      provide: MvcRequest,
+      useFactory: () => ({
+        body: {
+          url: 'http://localhost',
+        },
+      }),
+    },
+    {
+      provide: RequestPathParamMap,
+      useFactory: () => ({}),
+    },
+    {
+      provide: RequestQueryParamMap,
+      useFactory: () => ({}),
+    },
+  )
+
+  beforeEach(function() {
+    this.controller = new TestController()
+  })
+
+  describe('@RequestBody', function() {
+
+    it('constructs and validates the body', async function() {
+      const result: TestModel = await harness.invoke(this.controller, 'testBody')
+      expect(result).to.be.instanceof(TestModel)
+      expect(result.url).to.be.instanceof(Url)
+    })
+
+  })
+
+  describe('@PathParam', function() {
+
+    it('throws an error if a path param is missing', async function() {
+      await expect(harness.invoke(this.controller, 'testPathParam')).to.be.rejectedWith(MissingParamError)
+    })
+
+    it('passes the path param value if present', async function() {
+      const paramMap = await harness.inject(RequestPathParamMap)
+      paramMap.id = 'foo'
+
+      expect(await harness.invoke(this.controller, 'testPathParam')).to.equal('foo')
+    })
+  })
+
+  describe('@QueryParam', function() {
+
+    it('does not throw an error if a query param is optional', async function() {
+      expect(await harness.invoke(this.controller, 'testQueryParam')).to.equal(undefined)
+    })
+
+    it('passes the query param value if present', async function() {
+      const queryMap = await harness.inject(RequestQueryParamMap)
+      queryMap.search = 'foo'
+
+      expect(await harness.invoke(this.controller, 'testQueryParam')).to.equal('foo')
+    })
+
+  })
+})

packages/dandi/mvc/src/request.param.decorator.ts

@@ -43,11 +43,12 @@ export function requestParamProvider(
   token: InjectionToken<any>,
   type: ConvertedType,
   paramName: string,
+  paramMeta: ParamMetadata<any>,
   memberMetadata: MemberMetadata,
 ): Provider<any> {
   return {
     provide: token,
-    useFactory: requestParamValidatorFactory.bind(undefined, type, paramName, memberMetadata),
+    useFactory: requestParamValidatorFactory.bind(undefined, type, paramName, paramMeta, memberMetadata),
     deps: [mapToken, ModelBuilder, RequestParamModelBuilderOptions],
     providers: [RequestParamModelBuilderOptionsProvider],
   }
@@ -72,7 +73,7 @@ export function makeRequestParamDecorator<T>(
     }
     meta.token = token
     meta.optional = optional
-    meta.providers = [requestParamProvider(mapToken, token, type, name || meta.name, memberMetadata)]
+    meta.providers = [requestParamProvider(mapToken, token, type, name || meta.name, meta, memberMetadata)]
 
     return {
       meta,
@@ -82,21 +83,3 @@ export function makeRequestParamDecorator<T>(
   apply.within = conditionWithinByKeyDecorator.bind(null, apply)
   return apply
 }
-
-export function requestParamDecorator<T>(
-  mapToken: InjectionToken<ParamMap>,
-  type: ConvertedType,
-  name: string,
-  target: MethodTarget<T>,
-  memberName: string,
-  paramIndex: number,
-): void {
-  const meta = getInjectableParamMetadata(target, memberName, paramIndex)
-  const memberMetadata = getMemberMetadata(target.constructor, memberName, paramIndex)
-  const token = requestParamToken<T>(mapToken, memberName, name || meta.name)
-  if (isConstructor(type)) {
-    memberMetadata.type = type
-  }
-  meta.token = token
-  meta.providers = [requestParamProvider(mapToken, token, type, name || meta.name, memberMetadata)]
-}

packages/dandi/mvc/src/request.param.validator.spec.ts

@@ -1,3 +1,4 @@
+import { ParamMetadata } from '@dandi/core'
 import { testHarnessSingle } from '@dandi/core/testing'
 import { MemberMetadata } from '@dandi/model'
 import { MetadataModelBuilder, ModelBuilder, PrimitiveTypeConverter, TypeConverter } from '@dandi/model-builder'
@@ -10,11 +11,13 @@ import { requestParamValidatorFactory } from './request.param.validator'
 
 describe('requestParamValidatorFactory', () => {
   let paramMap: { [key: string]: string }
+  let paramMeta: ParamMetadata<any>
   let builder: SinonStubbedInstance<ModelBuilder>
   let memberMetadata: MemberMetadata
 
   beforeEach(() => {
     paramMap = { foo: 'bar' }
+    paramMeta = {} as any
     builder = createStubInstance(MetadataModelBuilder)
     memberMetadata = {
       type: String,
@@ -29,6 +32,7 @@ describe('requestParamValidatorFactory', () => {
     requestParamValidatorFactory(
       String,
       'foo',
+      paramMeta,
       memberMetadata,
       paramMap,
       builder,

packages/dandi/mvc/src/request.param.validator.ts

@@ -1,20 +1,25 @@
+import { ParamMetadata } from '@dandi/core'
 import { MemberMetadata } from '@dandi/model'
 import { MemberBuilderOptions, ModelBuilder } from '@dandi/model-builder'
 
-import { MissingPathParamError } from './errors'
+import { MissingParamError } from './errors'
 import { ParamMap } from './tokens'
 
 export function requestParamValidatorFactory(
   type: any,
   paramName: string,
+  paramMeta: ParamMetadata<any>,
   memberMetadata: MemberMetadata,
   paramMap: ParamMap,
   builder: ModelBuilder,
   options: MemberBuilderOptions,
 ): any {
   const value = paramMap[paramName]
   if (typeof value === 'undefined') {
-    throw new MissingPathParamError(paramName)
+    if (paramMeta.optional) {
+      return undefined
+    }
+    throw new MissingParamError(paramName)
   }
   return builder.constructMember(memberMetadata, paramName, value, options)
 }