[#39] Refactoring: improve error handling

- created DecoratorUtils.getSubjectName() and used it in DecoratorUsageError-s - throwing DecoratorUsageError when decorators used on wrong [class/method/property] , solving issue [#22] - changed @autowire() to @Autowired()
saskodh · Aug 11, 2016 · b886442 · b886442
1 parent 5b4edcc
commit b886442
Show file tree

Hide file tree

Showing 29 changed files with 300 additions and 77 deletions.
diff --git a/src/index.ts b/src/index.ts
@@ -7,7 +7,7 @@ export {ComponentScan} from "./lib/decorators/ComponentScanDecorator";
 export {Import} from "./lib/decorators/ImportDecorator";
 export {Configuration} from "./lib/decorators/ConfigurationDecorator";
 export {Controller} from "./lib/decorators/ControllerDecorator";
-export {Inject, Value, Autowire} from "./lib/decorators/InjectionDecorators";
+export {Inject, Value, Autowired} from "./lib/decorators/InjectionDecorators";
 export {PostConstruct, PreDestroy} from "./lib/decorators/LifeCycleHooksDecorators"
 export {Profile} from "./lib/decorators/ComponentDecorator";
 export {PropertySource} from "./lib/decorators/PropertySourceDecorator";

diff --git a/src/lib/decorators/ComponentDecorator.ts b/src/lib/decorators/ComponentDecorator.ts
@@ -2,6 +2,7 @@ import { InjectUtil, InjectionData } from "./InjectionDecorators";
 import { CONTROLLER_DECORATOR_TOKEN } from "./ControllerDecorator";
 import { INTERCEPTOR_DECORATOR_TOKEN } from "../interceptors/InterceptorDecorator";
 import { DecoratorUsageError } from "../errors/DecoratorUsageError";
+import { DecoratorUtil, DecoratorType } from "../helpers/DecoratorUtils";
 
 export class ComponentData {
     classToken: Symbol;
@@ -20,6 +21,11 @@ const COMPONENT_DECORATOR_TOKEN = Symbol('component_decorator_token');
 
 export function Component() {
     return function (target) {
+        let args = Array.prototype.slice.call(arguments);
+        if (!DecoratorUtil.isType(DecoratorType.CLASS, args)) {
+            let subjectName = DecoratorUtil.getSubjectName(args);
+            throw new DecoratorUsageError(`@Component can be set only on a class! (${subjectName})`);
+        }
         let componentData = new ComponentData();
         componentData.injectionData = InjectUtil.initIfDoesntExist(target.prototype);
         target[COMPONENT_DECORATOR_TOKEN] = componentData;
@@ -29,7 +35,8 @@ export function Component() {
 export function Profile(profile: string) {
     return function (target) {
         if (!ComponentUtil.isComponent(target)) {
-            throw new DecoratorUsageError(`@Profile can be set only on @Component! (${target.name})`);
+            let subjectName = DecoratorUtil.getSubjectName(Array.prototype.slice.call(arguments));
+            throw new DecoratorUsageError(`@Profile can be set only on @Component! (${subjectName})`);
         }
         ComponentUtil.getComponentData(target).profile = profile;
     };

diff --git a/src/lib/decorators/ComponentScanDecorator.ts b/src/lib/decorators/ComponentScanDecorator.ts
@@ -5,6 +5,7 @@ import { ConfigurationData, ConfigurationUtil } from "./ConfigurationDecorator";
 import { ComponentUtil } from "./ComponentDecorator";
 import { RequireUtils } from "../helpers/RequireUtils";
 import { DecoratorUsageError } from "../errors/DecoratorUsageError";
+import { DecoratorUtil } from "../helpers/DecoratorUtils";
 
 /**
  *A decorator for setting up project files to be component-scanned.
@@ -14,7 +15,8 @@ import { DecoratorUsageError } from "../errors/DecoratorUsageError";
 export function ComponentScan(path) {
     return function (target) {
         if (!ConfigurationUtil.isConfigurationClass(target)) {
-            throw new DecoratorUsageError(`@ComponentScan is allowed on @Configuration classes only! (${target.name})`);
+            let subjectName = DecoratorUtil.getSubjectName(Array.prototype.slice.call(arguments));
+            throw new DecoratorUsageError(`@ComponentScan is allowed on @Configuration classes only! (${subjectName})`);
         }
         ConfigurationUtil.addComponentScanPath(target, path);
     };

diff --git a/src/lib/decorators/ConfigurationDecorator.ts b/src/lib/decorators/ConfigurationDecorator.ts
@@ -2,6 +2,7 @@ import { ComponentFactory } from "../di/ComponentFactory";
 import { PropertySourceUtil } from "./PropertySourceDecorator";
 import { ComponentScanUtil } from "./ComponentScanDecorator";
 import { DecoratorUsageError } from "../errors/DecoratorUsageError";
+import { DecoratorUtil, DecoratorType } from "../helpers/DecoratorUtils";
 
 const CONFIGURATION_HOLDER_TOKEN = Symbol('configuration_holder_token');
 
@@ -38,6 +39,10 @@ export class ConfigurationData {
 
 export function Configuration() {
     return function (target) {
+        if (!DecoratorUtil.isType(DecoratorType.CLASS, Array.prototype.slice.call(arguments))) {
+            let subjectName = DecoratorUtil.getSubjectName(Array.prototype.slice.call(arguments));
+            throw new DecoratorUsageError(`@Configuration can be set only on classes! (${subjectName})`);
+        }
         if (target[CONFIGURATION_HOLDER_TOKEN]) {
             throw new DecoratorUsageError(`Duplicate @Configuration decorator' (${target.name})`);
         }
@@ -51,7 +56,8 @@ export class ConfigurationUtil {
 
     static getConfigurationData(target): ConfigurationData {
         if (!this.isConfigurationClass(target)) {
-            throw new Error(`${target.name} is not a @Configuration class`);
+            let subjectName = DecoratorUtil.getSubjectName(Array.prototype.slice.call(arguments));
+            throw new Error(`${subjectName} is not a @Configuration class`);
         }
         return target[CONFIGURATION_HOLDER_TOKEN];
     }

diff --git a/src/lib/decorators/ControllerDecorator.ts b/src/lib/decorators/ControllerDecorator.ts
@@ -1,8 +1,14 @@
 import { Component } from "./ComponentDecorator";
+import { DecoratorUsageError } from "../errors/DecoratorUsageError";
+import { DecoratorUtil, DecoratorType } from "../helpers/DecoratorUtils";
 export const CONTROLLER_DECORATOR_TOKEN = Symbol('controller_decorator_token');
 
 export function Controller() {
     return function (target) {
+        if (!DecoratorUtil.isType(DecoratorType.CLASS, Array.prototype.slice.call(arguments))) {
+            let subjectName = DecoratorUtil.getSubjectName(Array.prototype.slice.call(arguments));
+            throw new DecoratorUsageError(`@Configuration can be set only on classes! (${subjectName})`);
+        }
         Component()(target);
         target[CONTROLLER_DECORATOR_TOKEN] = true;
     };

diff --git a/src/lib/decorators/ImportDecorator.ts b/src/lib/decorators/ImportDecorator.ts
@@ -1,6 +1,7 @@
 import { ConfigurationUtil } from "./ConfigurationDecorator";
 import { BadArgumentError } from "../errors/BadArgumentError";
 import { DecoratorUsageError } from "../errors/DecoratorUsageError";
+import { DecoratorUtil } from "../helpers/DecoratorUtils";
 
 /**
  * Decorator used for composing configuration classes by importing other configuration classes.
@@ -11,8 +12,8 @@ import { DecoratorUsageError } from "../errors/DecoratorUsageError";
 export function Import(...configurationClasses) {
     return function (targetConfigurationClass) {
         if (!ConfigurationUtil.isConfigurationClass(targetConfigurationClass)) {
-            // tslint:disable-next-line
-            throw new DecoratorUsageError(`@Import is allowed on @Configuration classes only! (${targetConfigurationClass.name})`);
+            let subjectName = DecoratorUtil.getSubjectName(Array.prototype.slice.call(arguments));
+            throw new DecoratorUsageError(`@Import is allowed on @Configuration classes only! (${subjectName})`);
         }
         let targetConfigurationData = ConfigurationUtil.getConfigurationData(targetConfigurationClass);
         for (let configurationClass of configurationClasses) {

diff --git a/src/lib/decorators/InjectionDecorators.ts b/src/lib/decorators/InjectionDecorators.ts
@@ -1,6 +1,8 @@
 import { ComponentUtil } from "./ComponentDecorator";
 import { TypeUtils } from "../helpers/TypeUtils";
 import { InjectionError } from "../errors/InjectionError";
+import { DecoratorUsageError } from "../errors/DecoratorUsageError";
+import { DecoratorType, DecoratorUtil } from "../helpers/DecoratorUtils";
 
 const INJECT_DECORATOR_TOKEN = Symbol('injector_decorator_token');
 
@@ -24,15 +26,22 @@ export class InjectionData {
 }
 
 export function Inject(dependencyToken?: Symbol) {
-    return function (target: any, fieldName: string) {
+    return function (...args) {
+        if (!DecoratorUtil.isType(DecoratorType.PROPERTY, args)) {
+            let subject = DecoratorUtil.getSubjectName(args);
+            throw new DecoratorUsageError(`@Inject can be set only on properties of a @Component class! (${subject})`);
+        }
+        let target = args[0];
+        let fieldName = args[1];
         let token = dependencyToken;
         let type = (<any> Reflect).getMetadata('design:type', target, fieldName);
         if (!token) {
             // fallback to field type
             if (ComponentUtil.isComponent(type)) {
                 token = ComponentUtil.getClassToken(type);
             } else {
-                throw new InjectionError(`Cannot inject dependency which is not a @Component! (${type.name})`);
+                let sub = DecoratorUtil.getSubjectName(args);
+                throw new InjectionError(`Cannot inject dependency (${type.name}) which is not a @Component! (${sub})`);
             }
         }
         // NOTE assumption: if type not declared or any then type is Object and isArray is false
@@ -41,12 +50,23 @@ export function Inject(dependencyToken?: Symbol) {
     };
 }
 
-export function Autowire() {
-    return Inject();
+export function Autowired() {
+    return function (...args) {
+        if (!DecoratorUtil.isType(DecoratorType.PROPERTY, args)) {
+            let subj = DecoratorUtil.getSubjectName(args);
+            throw new DecoratorUsageError(`@Autowired can be set only on properties of a @Component class! (${subj})`);
+        }
+        return Inject()(...args);
+    };
 }
 
 export function Value(preopertyKey) {
     return function (target: any, fieldName: string) {
+        let args = Array.prototype.slice.call(arguments);
+        if (!DecoratorUtil.isType(DecoratorType.PROPERTY, args)) {
+            let subject = DecoratorUtil.getSubjectName(args);
+            throw new DecoratorUsageError(`@Value can be set only on properties of a @Component class! (${subject})`);
+        }
         InjectUtil.initIfDoesntExist(target).properties.set(fieldName, preopertyKey);
     };
 }

diff --git a/src/lib/decorators/LifeCycleHooksDecorators.ts b/src/lib/decorators/LifeCycleHooksDecorators.ts
@@ -1,4 +1,5 @@
 import { DecoratorUsageError } from "../errors/DecoratorUsageError";
+import { DecoratorUtil, DecoratorType } from "../helpers/DecoratorUtils";
 
 const LIFE_CYCLE_HOOKS_TOKEN = Symbol('life_cycle_hooks_token');
 
@@ -12,11 +13,17 @@ export class LifeCycleHooksConfig {
  */
 export function PostConstruct() {
     return function (target, methodName, descriptor: PropertyDescriptor) {
+        let args = Array.prototype.slice.call(arguments);
+        if (!DecoratorUtil.isType(DecoratorType.METHOD, args)) {
+            let sub = DecoratorUtil.getSubjectName(args);
+            throw new DecoratorUsageError(`@PostConstruct can be set only on methods of a @Component class! (${sub})`);
+        }
         let conf = LifeCycleHooksUtil.initIfDoesntExist(target);
         if (conf.postConstructMethod) {
             let errorParams = [conf.postConstructMethod, methodName].join(', ');
+            let subjectName = DecoratorUtil.getSubjectName(args);
             // tslint:disable-next-line
-            throw new DecoratorUsageError(`@PostConstruct used on multiple methods (${errorParams}) within a component`);
+            throw new DecoratorUsageError(`@PostConstruct used on multiple methods (${errorParams}) within a @Component (${subjectName})`);
         }
         conf.postConstructMethod = methodName;
     };
@@ -27,10 +34,17 @@ export function PostConstruct() {
  */
 export function PreDestroy() {
     return function (target, methodName, descriptor: PropertyDescriptor) {
+        let args = Array.prototype.slice.call(arguments);
+        if (!DecoratorUtil.isType(DecoratorType.METHOD, args)) {
+            let subject = DecoratorUtil.getSubjectName(args);
+            throw new DecoratorUsageError(`@PreDestroy can be set only on methods of a @Component class! (${subject})`);
+        }
         let conf = LifeCycleHooksUtil.initIfDoesntExist(target);
         if (conf.preDestroyMethod) {
             let errorParams = [conf.preDestroyMethod, methodName].join(', ');
-            throw new DecoratorUsageError(`@PreDestroy used on multiple methods within a component (${errorParams})`);
+            let subjectName = DecoratorUtil.getSubjectName(args);
+            // tslint:disable-next-line
+            throw new DecoratorUsageError(`@PreDestroy used on multiple methods (${errorParams}) within a @Component (${subjectName})`);
         }
         conf.preDestroyMethod = methodName;
     };

diff --git a/src/lib/decorators/PropertySourceDecorator.ts b/src/lib/decorators/PropertySourceDecorator.ts
@@ -3,6 +3,7 @@ import { ConfigurationUtil } from "./ConfigurationDecorator";
 import { GeneralUtils } from "../helpers/GeneralUtils";
 import {RequireUtils} from "../helpers/RequireUtils";
 import { DecoratorUsageError } from "../errors/DecoratorUsageError";
+import { DecoratorUtil } from "../helpers/DecoratorUtils";
 
 /**
  * A decorator for defining a JSON property source for the configuration properties.
@@ -12,8 +13,8 @@ import { DecoratorUsageError } from "../errors/DecoratorUsageError";
 export function PropertySource(path: string) {
     return function (target) {
         if (!ConfigurationUtil.isConfigurationClass(target)) {
-            // tslint:disable-next-line
-            throw new DecoratorUsageError(`@PropertySource can be used only on @Configuration classes! (${target.name})`);
+            let subject = DecoratorUtil.getSubjectName(Array.prototype.slice.call(arguments));
+            throw new DecoratorUsageError(`@PropertySource is allowed on @Configuration classes only! (${subject})`);
         }
         ConfigurationUtil.addPropertySourcePath(target, path);
     };

diff --git a/src/lib/decorators/QualifierDecorator.ts b/src/lib/decorators/QualifierDecorator.ts
@@ -1,10 +1,12 @@
 import { ComponentUtil } from "./ComponentDecorator";
 import { DecoratorUsageError } from "../errors/DecoratorUsageError";
+import { DecoratorUtil } from "../helpers/DecoratorUtils";
 
 export function Qualifier(token: Symbol) {
     return function (target) {
         if (!ComponentUtil.isComponent(target)) {
-            throw new DecoratorUsageError(`@Qualifier can be used only on @Component classes! (${target.name})`);
+            let subjectName = DecoratorUtil.getSubjectName(Array.prototype.slice.call(arguments));
+            throw new DecoratorUsageError(`@Qualifier can be used only on @Component classes! (${subjectName})`);
         }
         ComponentUtil.getAliasTokens(target).push(token);
     };

diff --git a/src/lib/decorators/RequestMappingDecorator.ts b/src/lib/decorators/RequestMappingDecorator.ts
@@ -63,7 +63,8 @@ export function RequestMapping(config: RequestMappingConfig) {
             // TODO: refactor when new options are added on @RequestMapping for classes
             target[CLASS_ROUTER_CONFIG] = config.path;
         } else {
-            throw new DecoratorUsageError(`@RequestMapping can only be used on classes and methods! (${target.name})`);
+            let subjectName = DecoratorUtil.getSubjectName(Array.prototype.slice.call(arguments));
+            throw new DecoratorUsageError(`@RequestMapping can only be used on classes and methods! (${subjectName})`);
         }
     };
 }

diff --git a/src/lib/decorators/ViewDecorator.ts b/src/lib/decorators/ViewDecorator.ts
@@ -1,8 +1,15 @@
 import * as _ from "lodash";
 import { RequestMappingUtil, RouterConfigItem } from "../decorators/RequestMappingDecorator";
+import { DecoratorUsageError } from "../errors/DecoratorUsageError";
+import { DecoratorUtil, DecoratorType } from "../helpers/DecoratorUtils";
 
 export function View(name?: string) {
     return function (target, methodName) {
+        let args = Array.prototype.slice.call(arguments);
+        if (!DecoratorUtil.isType(DecoratorType.METHOD, args)) {
+            let subject = DecoratorUtil.getSubjectName(args);
+            throw new DecoratorUsageError(`@View can be set only on methods of a @Component class! (${subject})`);
+        }
         let viewName = name || methodName;
         let routerConfig = RequestMappingUtil.initRouterConfigIfDoesntExist(target);
         let routeConfig = _.find(routerConfig.routes, {methodHandler: methodName});

diff --git a/src/lib/helpers/DecoratorUtils.ts b/src/lib/helpers/DecoratorUtils.ts
@@ -12,7 +12,7 @@ export class DecoratorType {
 
 export class DecoratorUtil {
 
-    static getType(args): string {
+    static getType(args: Array<any>): string {
         if (args.length === 1) {
             return DecoratorType.CLASS;
         } else if (args.length === 2) {
@@ -29,7 +29,17 @@ export class DecoratorUtil {
         }
     }
 
-    static isType(decoratorType, args): boolean {
+    static isType(decoratorType: DecoratorType, args: Array<any>): boolean {
         return this.getType(args) === decoratorType;
     }
+
+    static getSubjectName (args: Array<any>) {
+        if (this.isType(DecoratorType.CLASS, args)) {
+            return args[0].name;
+        }
+        if (this.isType(DecoratorType.METHOD, args)) {
+            return `${args[0].constructor.name}.${args[1]}()`;
+        }
+        return [args[0].constructor.name, args[1]].join('.');
+    }
 }
diff --git a/src/lib/helpers/ProcessHandler.ts b/src/lib/helpers/ProcessHandler.ts
@@ -1,4 +1,5 @@
 import * as _ from "lodash";
+import { BadArgumentError } from "../errors/BadArgumentError";
 
 export class ProcessHandler {
 
@@ -23,7 +24,7 @@ export class ProcessHandler {
 
     registerOnExitListener(callback: Function) {
         if (!_.isFunction(callback)) {
-            throw new Error('Passed callback must be a function!');
+            throw new BadArgumentError('Passed callback must be a function!');
         }
 
         this.onExitListeners.push(callback);

diff --git a/test/lib/decorators/ComponentDecorator.spec.ts b/test/lib/decorators/ComponentDecorator.spec.ts
@@ -8,6 +8,11 @@ import {Controller} from "../../../src/lib/decorators/ControllerDecorator";
 import {Interceptor} from "../../../src/lib/interceptors/InterceptorDecorator";
 import { DecoratorUsageError } from "../../../src/lib/errors/DecoratorUsageError";
 
+class MyClass {
+    myProperty: string;
+    myFunction() {} // tslint:disable-line
+}
+
 describe('ComponentDecorator', function () {
 
     it('should add metadata', function () {
@@ -25,6 +30,13 @@ describe('ComponentDecorator', function () {
         expect(componentData.injectionData).to.be.instanceOf(InjectionData);
         expect(componentData.profile).to.be.undefined;
     });
+
+    it('should throw when not on a class', function () {
+        // given / when / then
+        expect(Component().bind(undefined, MyClass, 'myFunction', MyClass.prototype.myFunction))
+            .to.throw(DecoratorUsageError);
+        expect(Component().bind(undefined, MyClass, 'myProperty')).to.throw(DecoratorUsageError);
+    });
 });
 
 describe('ProfileDecorator', function () {
@@ -43,11 +55,11 @@ describe('ProfileDecorator', function () {
     });
 
     it('should throw error when @Profile is used on non Component', function () {
-        // given
-        class B {}
-
-        // when / then
-        expect(Profile('dev').bind(this, B)).to.throw(DecoratorUsageError);
+        // given / when / then
+        expect(Profile('dev').bind(undefined, MyClass)).to.throw(DecoratorUsageError);
+        expect(Profile('dev').bind(undefined, MyClass, 'myFunction', MyClass.prototype.myFunction))
+            .to.throw(DecoratorUsageError);
+        expect(Profile('dev').bind(undefined, MyClass, 'myProperty')).to.throw(DecoratorUsageError);
     });
 });
 

diff --git a/test/lib/decorators/ComponentScanDecorator.spec.ts b/test/lib/decorators/ComponentScanDecorator.spec.ts
@@ -29,10 +29,16 @@ describe('ComponentScanDecorator', function () {
 
     it('should throw when not on @Configuration', function () {
         // given
-        class A {}
+        class MyClass {
+            myProperty: string;
+            myFunction() {} // tslint:disable-line
+        }
 
         // when / then
-        expect(ComponentScan('somePath').bind(null, A)).to.throw(DecoratorUsageError);
+        expect(ComponentScan('somePath').bind(undefined, MyClass)).to.throw(DecoratorUsageError);
+        expect(ComponentScan('somePath').bind(undefined, MyClass, 'myFunction', MyClass.prototype.myFunction))
+            .to.throw(DecoratorUsageError);
+        expect(ComponentScan('somePath').bind(undefined, MyClass, 'myProperty')).to.throw(DecoratorUsageError);
     });
 });