Skip to content

Commit

Permalink
fix: compiler does not properly format class names for nested namespa…
Browse files Browse the repository at this point in the history
…ces (#449)

* fix: compiler does not properly format class names for nested namespaces

* fix: additional errors with nested namespaces

* test: add unit tests for namespace sorting in builder
  • Loading branch information
zacharygolba authored Oct 11, 2016
1 parent adc7230 commit a2bee41
Show file tree
Hide file tree
Showing 12 changed files with 116 additions and 34 deletions.
8 changes: 5 additions & 3 deletions src/packages/compiler/utils/format-name.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
// @flow
import { classify } from 'inflection';
import { camelize } from 'inflection';

import chain from '../../../utils/chain';
import underscore from '../../../utils/underscore';

import stripExt from './strip-ext';
import normalizePath from './normalize-path';

const DOUBLE_COLON = /::/g;

/**
* @private
*/
function applyNamespace(source: string) {
return source.replace('::', '$');
return source.replace(DOUBLE_COLON, '$');
}

/**
Expand All @@ -22,7 +24,7 @@ export default function formatName(source: string) {
.pipe(normalizePath)
.pipe(stripExt)
.pipe(underscore)
.pipe(classify)
.pipe(camelize)
.pipe(applyNamespace)
.value();
}
37 changes: 37 additions & 0 deletions src/packages/loader/builder/test/sort-by-namespace.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// @flow
import { expect } from 'chai';
import { describe, it } from 'mocha';

import sortByNamespace from '../utils/sort-by-namespace';

describe('module "loader/builder"', () => {
describe('util sortByNamespace()', () => {
it('returns -1 if "root" is the first argument', () => {
//$FlowIgnore
const result = sortByNamespace(['root'], ['api']);

expect(result).to.equal(-1);
});

it('returns 1 if "root" is the second argument', () => {
//$FlowIgnore
const result = sortByNamespace(['api'], ['root']);

expect(result).to.equal(1);
});

it('returns -1 if the first argument is shorter than the second', () => {
//$FlowIgnore
const result = sortByNamespace(['api'], ['admin']);

expect(result).to.equal(-1);
});

it('returns 1 if the first argument is longer than the second', () => {
//$FlowIgnore
const result = sortByNamespace(['admin'], ['api']);

expect(result).to.equal(1);
});
});
});
4 changes: 3 additions & 1 deletion src/packages/loader/builder/utils/create-parent-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@
import { getParentKey } from '../../resolver';
import type { Builder$Construct, Builder$ParentBuilder } from '../interfaces';

import sortByNamespace from './sort-by-namespace';

export default function createParentBuilder<T>(
construct: Builder$Construct<T>
): Builder$ParentBuilder<T> {
return target => Array
.from(target)
.sort(([a], [b]) => a.length - b.length)
.sort(sortByNamespace)
.reduce((result, [key, value]) => {
let parent = value.get('application') || null;

Expand Down
18 changes: 18 additions & 0 deletions src/packages/loader/builder/utils/sort-by-namespace.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// @flow
import type { Bundle$Namespace } from '../../index';

/**
* @private
*/
export default function sortByNamespace<T>(
[a]: [string, Bundle$Namespace<T>],
[b]: [string, Bundle$Namespace<T>]
): number {
if (a === 'root') {
return -1;
} else if (b === 'root') {
return 1;
}

return Math.min(Math.max(a.length - b.length, -1), 1);
}
2 changes: 1 addition & 1 deletion src/packages/loader/utils/format-key.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { dasherize } from 'inflection';
import chain from '../../../utils/chain';
import underscore from '../../../utils/underscore';

const NAMESPACE_DELIMITER = /\$\-/;
const NAMESPACE_DELIMITER = /\$\-/g;

/**
* @private
Expand Down
32 changes: 20 additions & 12 deletions src/packages/router/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,46 +2,54 @@
import { FreezeableMap } from '../freezeable';
import type { Request } from '../server';

import { ID_PATTERN } from './route';
import Namespace from './namespace';
import { build, define } from './definitions';
import createReplacer from './utils/create-replacer';
import type { Router$opts } from './interfaces';
import type Route from './route'; // eslint-disable-line no-duplicate-imports

/**
* @private
*/
class Router extends FreezeableMap<string, Route> {
constructor({ routes, controller, controllers }: Router$opts) {
super();
replacer: RegExp;

constructor({ routes, controller, controllers }: Router$opts) {
const definitions = build(routes, new Namespace({
controller,
controllers,
path: '/',
name: 'root'
}));

super();
define(this, definitions);

Reflect.defineProperty(this, 'replacer', {
value: createReplacer(controllers),
writable: false,
enumerable: false,
configurable: false
});

this.freeze();
}

match({ method, url: { pathname } }: Request) {
const staticPath = pathname.replace(ID_PATTERN, ':dynamic');
match({ method, url }: Request): void | Route {
const params = [];
const staticPath = url.pathname.replace(this.replacer, (str, g1, g2) => {
params.push(g2);
return `${g1}/:dynamic`;
});

Reflect.set(url, 'params', params);

return this.get(`${method}:${staticPath}`);
}
}

export default Router;

export {
ID_PATTERN,
DYNAMIC_PATTERN,
RESOURCE_PATTERN,
default as Route
} from './route';
export { DYNAMIC_PATTERN, default as Route } from './route';

export type { Router$Namespace } from './interfaces';
export type { Resource$opts } from './resource';
Expand Down
2 changes: 0 additions & 2 deletions src/packages/router/route/constants.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,2 @@
// @flow
export const ID_PATTERN = /(?!=)(\d+)/;
export const DYNAMIC_PATTERN = /(:\w+)/g;
export const RESOURCE_PATTERN = /^((?!\/)[a-z\-]+)/ig;
19 changes: 8 additions & 11 deletions src/packages/router/route/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { FreezeableSet, freezeProps, deepFreezeProps } from '../../freezeable';
import type Controller from '../../controller';
import type { Request, Response, Request$method } from '../../server';

import { ID_PATTERN } from './constants';
import { createAction } from './action';
import { paramsFor, defaultParamsFor, validateResourceId } from './params';
import getStaticPath from './utils/get-static-path';
Expand Down Expand Up @@ -112,20 +111,18 @@ class Route extends FreezeableSet<Action<any>> {
this.freeze();
}

parseParams(pathname: string) {
const parts = pathname.match(ID_PATTERN) || [];

return parts.reduce((params, val, index) => {
const key = this.dynamicSegments[index];
parseParams(params: Array<string>): Object {
return params.reduce((result, value, idx) => {
const key = this.dynamicSegments[idx];

if (key) {
return {
...params,
[key]: parseInt(val, 10)
...result,
[key]: Number.parseInt(value, 10)
};
}

return params;
return result;
}, {});
}

Expand All @@ -148,7 +145,7 @@ class Route extends FreezeableSet<Action<any>> {
defaultParams,
params: this.params.validate({
...req.params,
...this.parseParams(req.url.pathname)
...this.parseParams(req.url.params)
})
});

Expand All @@ -161,7 +158,7 @@ class Route extends FreezeableSet<Action<any>> {
}

export default Route;
export { ID_PATTERN, DYNAMIC_PATTERN, RESOURCE_PATTERN } from './constants';
export { DYNAMIC_PATTERN } from './constants';

export type { Action } from './action';
export type { Route$opts, Route$type } from './interfaces';
6 changes: 3 additions & 3 deletions src/packages/router/route/test/route.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,15 @@ describe('module "router/route"', () => {

describe('#parseParams()', () => {
it('is empty for static paths', () => {
expect(staticRoute.parseParams('/posts/1')).to.be.empty;
expect(staticRoute.parseParams(['1'])).to.be.empty;
});

it('contains params matching dynamic segments', () => {
expect(dynamicRoute.parseParams('/posts/1')).to.deep.equal({ id: 1 });
expect(dynamicRoute.parseParams(['1'])).to.deep.equal({ id: 1 });
});

it('does not contain params for unmatched dynamic segments', () => {
expect(dynamicRoute.parseParams('/posts/1/2')).to.deep.equal({ id: 1 });
expect(dynamicRoute.parseParams(['1', '2'])).to.deep.equal({ id: 1 });
});
});
});
Expand Down
19 changes: 19 additions & 0 deletions src/packages/router/utils/create-replacer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// @flow
import type Controller from '../../controller';

/**
* @private
*/
export default function createReplacer(
controllers: Map<string, Controller>
): RegExp {
const names = Array
.from(controllers)
.map(([, { model }]) => model)
.filter(Boolean)
.map(({ resourceName }) => resourceName)
.filter((str, idx, arr) => idx === arr.lastIndexOf(str))
.join('|');

return new RegExp(`(${names})/(\\d+)`, 'ig');
}
2 changes: 1 addition & 1 deletion src/packages/server/request/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export function createRequest(req: any, {
logger,
router
}: Request$opts): Request {
const url = parseURL(req.url, true);
const url = { ...parseURL(req.url, true), params: [] };
const headers: Map<string, string> = new Map(entries(req.headers));

Object.assign(req, {
Expand Down
1 change: 1 addition & 0 deletions src/packages/server/request/interfaces.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ type Request$url = {
hostname?: string;
hash?: string;
search?: string;
params: Array<string>;
query: Object;
pathname: string;
path: string;
Expand Down

0 comments on commit a2bee41

Please sign in to comment.