Merge pull request #4 from fabrix-app/v1.rc-1

V1.rc 1

.circleci/config.yml

@@ -21,6 +21,9 @@ jobs:
       - run:
           name: test
           command: npm test
+      - run:
+          name: test performance
+          command: npm run test-performance
       - run:
           name: code-coverage
           command: './node_modules/.bin/nyc report --reporter=text-lcov'

README.md

@@ -97,6 +97,11 @@ See https://github.com/fabrix-app/fabrix/wiki/FAQ
 We love contributions! Please check out our [Contributor's Guide](https://github.com/fabrix-app/fabrix/blob/master/.github/CONTRIBUTING.md) for more
 information on how our projects are organized and how to get started.
 
+## Development
+Fabrix uses a continuous integration process and all tests must pass for Fabrix to release a new version.  CircleCI releases a new version when a PR is merged into master.  For local development, you can download [CircleCI's local development tools](https://circleci.com/docs/2.0/local-cli/#installing-the-circleci-local-cli-on-macos-and-linux-distros) and run local tests before submitting a Pull Request.
+
+Fabrix maintains a high score of coverage tests, any Pull Request should have well written Integration and Unit tests that increase the overall coverage score. 
+
 ## License
 [MIT](https://github.com/fabrix-app/fabrix/blob/master/LICENSE)
 

lib/Configuration.ts

@@ -1,4 +1,4 @@
-import { merge, defaultsDeep, isArray } from 'lodash'
+import { merge, isArray, defaults, union } from 'lodash'
 import { resolve, dirname } from 'path'
 import { IllegalAccessError, ConfigValueError } from './errors'
 import { requireMainFilename } from './utils'
@@ -25,31 +25,54 @@ export class Configuration extends Map<any, any> {
 
   /**
    * Flattens configuration tree
+   * Recursive
    */
   static flattenTree (tree = { }) {
-    const toReturn: {[key: string]: any} = { }
-
-    Object.entries(tree).forEach(([ k, v ]) => {
-      if (typeof v === 'object' && v !== null) {
-        const flatObject = Configuration.flattenTree(v)
-        Object.keys(flatObject).forEach(flatKey => {
-          toReturn[`${k}.${flatKey}`] = flatObject[flatKey]
-        })
-      }
-      toReturn[k] = v
-    })
-    return toReturn
+    // Try to flatten and fail if circular
+    try {
+      const toReturn: { [key: string]: any } = {}
+      Object.entries(tree).forEach(([k, v]) => {
+        // if (typeof v === 'object' && v !== null) {
+        if (
+          v instanceof Object
+          && typeof v !== 'function'
+        ) {
+          // If value is an array, flatten by index and don't try to flatten further
+          if (Array.isArray(v)) {
+            v.forEach((val, i) => {
+              toReturn[`${k}.${i}`] = val
+            })
+          }
+          // If the value is a normal object, keep flattening
+          else {
+            const flatObject = Configuration.flattenTree(v)
+            Object.keys(flatObject).forEach(flatKey => {
+              toReturn[`${k}.${flatKey}`] = flatObject[flatKey]
+            })
+          }
+        }
+        // Other wise, the value is a function, string, or number etc and should stop flattening
+        toReturn[k] = v
+      })
+      return toReturn
+    }
+    catch (err) {
+      throw new RangeError('Tree is circular, check that there are no circular references in the config')
+    }
   }
 
-  static initialResources (tree) {
+  /**
+   * Defines the initial api resources
+   */
+  static initialResources (tree, resources = []) {
     if (tree.hasOwnProperty('main') && tree.main.hasOwnProperty('resources')) {
       if (!isArray(tree.main['resources'])) {
         throw new ConfigValueError('if set, main.resources must be an array')
       }
       return tree.main['resources']
     }
     else {
-      return ['controllers', 'policies', 'services', 'models', 'resolvers']
+      return resources
     }
   }
 
@@ -65,6 +88,7 @@ export class Configuration extends Map<any, any> {
     const configTemplate = {
       main: {
         resources: Configuration.initialResources(initialConfig),
+        lockResources: false,
         maxListeners: 128,
         spools: [ ],
         paths: {
@@ -120,7 +144,7 @@ export class Configuration extends Map<any, any> {
   }
 
   /**
-    * Merge tree into this configuration. Return overwritten keys
+    * Merge tree into this configuration if allowed. Return overwritten keys
    */
   merge (configTree: {[key: string]: any}, configAction = 'hold'): { hasKey: boolean, key: any }[] {
     const configEntries = Object.entries(Configuration.flattenTree(configTree))
@@ -131,11 +155,12 @@ export class Configuration extends Map<any, any> {
       if (!hasKey || configAction === 'hold') {
         this.set(key, value)
       }
-      // If configAction is set to merge, it will merge values over the initial config
+      // If configAction is set to merge, it will default values over the initial config
       else if (hasKey && configAction === 'merge') {
-        this.set(key, defaultsDeep(this.get(key), value))
+        this.set(key, defaults(this.get(key), value))
       }
       // If configAction is replaceable, and the key already exists, it's ignored completely
+      // This is because it was set by a higher level app config
       else if (hasKey && configAction === 'replaceable') {
         // Do Nothing
       }

lib/Core.ts

@@ -1,3 +1,4 @@
+import { union } from 'lodash'
 import { FabrixApp } from './'
 import * as mkdirp from 'mkdirp'
 import { Templates } from './'
@@ -95,12 +96,12 @@ export const Core = {
    * Bind the context of API resource methods.
    */
   bindMethods (app: FabrixApp, resource: string): any {
-    return Object.entries(app.api[resource])
+    return Object.entries(app.api[resource] || { })
       .map(([ resourceName, Resource ]: [string, any]) => {
         const objContext = <typeof Resource> Resource
         const obj = new objContext(app)
 
-        obj.methods = Core.getClassMethods(obj) || []
+        obj.methods = Core.getClassMethods(obj) || [ ]
         Object.entries(obj.methods).forEach(([ _, method]: [any, string])  => {
           obj[method] = obj[method].bind(obj)
         })
@@ -111,6 +112,20 @@ export const Core = {
       }), { })
   },
 
+  /**
+   * Instantiate resource classes and bind resource methods
+   */
+  bindResourceMethods(app: FabrixApp, defaults: string[]): void {
+    defaults.forEach(resource => {
+      try {
+        app[resource] = Core.bindMethods(app, resource)
+      }
+      catch (err) {
+        app.log.error(err)
+      }
+    })
+  },
+
   /**
    * Traverse prototype chain and aggregate all class method names
    */
@@ -135,16 +150,38 @@ export const Core = {
 
   /**
    * Merge the app api resources with the ones provided by the spools
+   * Given that they are allowed by app.config.main.resources
    */
-  mergeApi (
-    app: FabrixApp,
-    spool: Spool,
-    defaults = [ ]
-  ) {
-    defaults.forEach(resource => Object.assign(
-      app.api[resource] || {},
-      spool.api[resource] || {})
-    )
+  mergeApi (app: FabrixApp, spool: Spool) {
+    // Use the setter to see if any new api resources from the spool can be applied
+    app.resources = union(app.resources, Object.keys(app.api), Object.keys(spool.api))
+    // Foreach resource, bind the spool.api into the app.api
+    app.resources.forEach( resource => {
+      // If there is a conflict, resolve it at the resource level
+      if (app.api.hasOwnProperty(resource) && spool.api.hasOwnProperty(resource)) {
+        Core.mergeApiResource(app, spool, resource)
+      }
+      // Else define the resource in the app.api and merge the spool.api resource into it
+      else if (!app.api.hasOwnProperty(resource) && spool.api.hasOwnProperty(resource)) {
+        app.api[resource] = {}
+        Object.assign(
+          (app.api[resource] || {}),
+          (spool.api[resource] || {})
+        )
+      }
+    })
+  },
+
+  /**
+   * Merge the Spool Api Resource if not already defined by App Api
+   */
+  mergeApiResource (app: FabrixApp, spool: Spool, resource: string) {
+    const spoolApiResources = Object.keys(spool.api[resource])
+    spoolApiResources.forEach(k => {
+      if (!app.api[resource].hasOwnProperty(k)) {
+        app.api[resource][k] = spool.api[resource][k]
+      }
+    })
   },
 
   /**
@@ -159,6 +196,9 @@ export const Core = {
       if (!extensions.hasOwnProperty(ext)) {
         continue
       }
+      if (app.hasOwnProperty(ext)) {
+        app.log.warn(`Spool Extension ${spool.name}.${ext} overriding app.${ext}`)
+      }
       Object.defineProperty(app, ext, spool.extensions[ext])
     }
   },

lib/Fabrix.ts

@@ -1,4 +1,5 @@
 import { EventEmitter } from 'events'
+import { union } from 'lodash'
 import { Core } from './Core'
 import { Configuration } from './Configuration'
 import { LoggerProxy } from './LoggerProxy'
@@ -46,15 +47,12 @@ export class FabrixApp extends EventEmitter {
   public models: {[key: string]: any } // FabrixModel }
   public resolvers: {[key: string]: any } // FabrixResolver }
 
-  public routes: any[] = [ ]
-
-
   /**
    * @param app.pkg The application package.json
    * @param app.api The application api (api/ folder)
    * @param app.config The application configuration (config/ folder)
    *
-   * Initialize the Fabrix Application and its EventEmitter parentclass. Set
+   * Initialize the Fabrix Application and its EventEmitter parent class. Set
    * some necessary default configuration.
    */
   constructor (app: {
@@ -93,23 +91,32 @@ export class FabrixApp extends EventEmitter {
     // Set the max listeners from the config
     this.setMaxListeners(this.config.get('main.maxListeners'))
 
-    // Set the resources from the configuration
-    this.resources = this.config.get('main.resources')
+    // Set the resources from the configuration (this bypasses the setter with the initial config
+    // in case the resourceLock is configured)
+    this._resources = this.config.get('main.resources')
+    // See if additional resources can be set
+    this.resources = union(Object.keys(app.api), this.config.get('main.resources'))
 
-    // Set each api resource to make sure it's provided as an object in app
+    // Set each api resource to make sure it's provided as an object in the app
     this.resources.forEach(resource => {
       app.api[resource] = app.api[resource] || (app.api[resource] = { })
     })
 
     // instantiate spools TOTO type of Spool
     this.config.get('main.spools').forEach((NewSpool: any) => {
       try {
+        // Create new Instance of the Spool
         const spoolContext = <typeof NewSpool> NewSpool
         const spool = new spoolContext(this, {})
+        // Add the spool instance to the app.spools namespace
         this.spools[spool.name] = spool
+        // Reconcile the spool.config with the app.config
         this.config.merge(spool.config, spoolContext.configAction)
+        // Merge extensions into app.<ext>
         Core.mergeExtensions(this, spool)
-        Core.mergeApi(this, spool, this.resources)
+        // Merge the spool.api with app.api
+        Core.mergeApi(this, spool)
+        // Bind the Spool Listeners to app.emit
         Core.bindSpoolMethodListeners(this, spool)
       }
       catch (e) {
@@ -118,8 +125,8 @@ export class FabrixApp extends EventEmitter {
       }
     })
 
-    // instantiate resource classes and bind resource methods
-    this.bindResourceMethods(this.resources)
+    // Instantiate resource classes and bind resource methods
+    Core.bindResourceMethods(this, this.resources)
     // Bind Application Listeners
     Core.bindApplicationListeners(this)
     // Bind the Phase listeners for the Spool lifecycle
@@ -128,58 +135,43 @@ export class FabrixApp extends EventEmitter {
     this.emit('fabrix:constructed')
   }
 
-  /**
-   * Instantiate resource classes and bind resource methods
-   */
-  bindResourceMethods(defaults: string[]): void {
-    defaults.forEach(resource => {
-      try {
-        this[resource] = Core.bindMethods(this, resource)
-      }
-      catch (err) {
-        this.log.error(err)
-      }
-    })
-  }
-
-  // @enumerable(false)
-  // @writable(false)
   get logger () {
     return this._logger
   }
 
-  // @enumerable(false)
   get env () {
     return this._env
   }
 
-  // @enumerable(false)
   get pkg () {
     return this._pkg
   }
 
-  // @enumerable(false)
-  // @writable(false)
-  // @configurable(false)
   get versions () {
     return this._versions
   }
 
-  // @writable(false)
-  // @configurable(true)
   get config () {
     return this._config
   }
 
-  // @enumerable(false)
+  /**
+   * Gets the package.json of the Fabrix module
+   */
   get fabrix () {
     return this._fabrix
   }
 
+  /**
+   * Gets the Spools that have been installed
+   */
   get spools () {
     return this._spools
   }
 
+  /**
+   *   Gets the api
+   */
   get api () {
     return this._api
   }
@@ -192,10 +184,19 @@ export class FabrixApp extends EventEmitter {
     return this.logger
   }
 
+  /**
+   * Sets available/allowed resources from Api and Spool Apis
+   */
   set resources (values) {
-    this._resources = Object.assign(this._resources, values)
+    if (!this.config.get('main.lockResources')) {
+      this._resources = Object.assign([], Configuration.initialResources(this.config, values))
+      this.config.set('main.resources', this._resources)
+    }
   }
 
+  /**
+   * Gets the Api resources that have been set
+   */
   get resources() {
     return this._resources
   }
@@ -226,6 +227,10 @@ export class FabrixApp extends EventEmitter {
         this.log.debug('All spools unloaded. Done.')
         this.removeAllListeners()
       })
+      .catch(err => {
+        this.log.error(err, 'while handling stop.')
+        throw err
+      })
 
     return this
   }