Skip to content

Commit

Permalink
Merge pull request #96 from kamilmysliwiec/bugfixes
Browse files Browse the repository at this point in the history
Bugfixes
  • Loading branch information
kamilmysliwiec committed Jun 24, 2017
2 parents 41f069d + c22d848 commit a753c50
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 31 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
@@ -1,3 +1,11 @@
## 3.0.1 (24.06.2017)
**@nestjs/core**
- Hierarchical injector bugfix,
- Middlewares `@UseFilters()` bugfix (#95).

**@nestjs/microservices**
- TCP server / client bugfix (#91)

## 3.0.0 (03.06.2017)
**@nestjs/common - BREAKING CHANGE**
- You should now pass objects into `@UseFilters()` decorator instead of metatypes,
Expand Down
11 changes: 7 additions & 4 deletions src/core/injector/injector.ts
Expand Up @@ -139,7 +139,7 @@ export class Injector {
return components.has(name) ? components.get(name) : scanInExports();
}

public scanForComponentInExports(components: Map<string, any>, name: any, module: Module, { metatype }, context: Module[] = []) {
public scanForComponentInExports(components: Map<string, any>, name: any, module: Module, metatype, context: Module[] = []) {
const instanceWrapper = this.scanForComponentInRelatedModules(module, name, context);
if (!isNil(instanceWrapper)) {
return instanceWrapper;
Expand Down Expand Up @@ -185,11 +185,14 @@ export class Injector {

(relatedModules as any[]).forEach((relatedModule) => {
const { components, exports } = relatedModule;
if (!exports.has(name) || !components.has(name)) return;

const isInScope = context === null;
if ((!exports.has(name) && !isInScope) || !components.has(name)) {
return;
}
component = components.get(name);
if (!component.isResolved) {
this.loadInstanceOfComponent(component, relatedModule, [].concat(context, module));
const ctx = isInScope ? [module] : [].concat(context, module);
this.loadInstanceOfComponent(component, relatedModule, ctx);
}
});
return component;
Expand Down
26 changes: 15 additions & 11 deletions src/core/middlewares/middlewares-module.ts
Expand Up @@ -17,26 +17,29 @@ import { NestMiddleware } from '@nestjs/common/interfaces/middlewares/nest-middl
import { Metatype } from '@nestjs/common/interfaces/metatype.interface';
import { RuntimeException } from '../errors/exceptions/runtime.exception';
import { isUndefined } from '@nestjs/common/utils/shared.utils';
import { ApplicationConfig } from './../application-config';
import { RouterExceptionFilters } from './../router/router-exception-filters';

export class MiddlewaresModule {
private static routesMapper = new RoutesMapper();
private static container = new MiddlewaresContainer();
private static readonly routesMapper = new RoutesMapper();
private static readonly container = new MiddlewaresContainer();
private static readonly routerProxy = new RouterProxy();
private static readonly routerMethodFactory = new RouterMethodFactory();
private static routerExceptionFilter: RouterExceptionFilters;
private static resolver: MiddlewaresResolver;
private static exceptionHandler = new ExceptionsHandler();
private static routerProxy = new RouterProxy();
private static routerMethodFactory = new RouterMethodFactory();

public static getContainer(): MiddlewaresContainer {
return this.container;
}

public static setup(container: NestContainer) {
public static setup(container: NestContainer, config: ApplicationConfig) {
this.routerExceptionFilter = new RouterExceptionFilters(config);
this.resolver = new MiddlewaresResolver(this.container);

const modules = container.getModules();
this.resolveMiddlewares(modules);
}

public static getContainer(): MiddlewaresContainer {
return this.container;
}

public static resolveMiddlewares(modules: Map<string, Module>) {
modules.forEach((module, name) => {
const instance = module.instance;
Expand Down Expand Up @@ -105,8 +108,9 @@ export class MiddlewaresModule {
if (isUndefined(instance.resolve)) {
throw new InvalidMiddlewareException(metatype.name);
}
const exceptionsHandler = this.routerExceptionFilter.create(instance, instance.resolve);
const router = this.routerMethodFactory.get(app, method).bind(app);
const proxy = this.routerProxy.createProxy(instance.resolve(), this.exceptionHandler);
const proxy = this.routerProxy.createProxy(instance.resolve(), exceptionsHandler);

router(path, proxy);
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/nest-application.ts
Expand Up @@ -35,7 +35,7 @@ export class NestApplication implements INestApplication {

public setupModules() {
SocketModule.setup(this.container, this.config);
MiddlewaresModule.setup(this.container);
MiddlewaresModule.setup(this.container, this.config);
MicroservicesModule.setupClients(this.container);
}

Expand Down
10 changes: 9 additions & 1 deletion src/core/test/middlewares/middlewares-module.spec.ts
Expand Up @@ -9,7 +9,9 @@ import { RequestMethod } from '../../../common/enums/request-method.enum';
import { Controller } from '../../../common/utils/decorators/controller.decorator';
import { RequestMapping } from '../../../common/utils/decorators/request-mapping.decorator';
import { RuntimeException } from '../../errors/exceptions/runtime.exception';
import { RoutesMapper } from "../../middlewares/routes-mapper";
import { RoutesMapper } from '../../middlewares/routes-mapper';
import { RouterExceptionFilters } from '../../router/router-exception-filters';
import { ApplicationConfig } from '../../application-config';

describe('MiddlewaresModule', () => {
@Controller({ path: 'test' })
Expand All @@ -32,6 +34,12 @@ describe('MiddlewaresModule', () => {
}
}

beforeEach(() => {
(MiddlewaresModule as any).routerExceptionFilter = new RouterExceptionFilters(
new ApplicationConfig(),
);
});

describe('loadConfiguration', () => {

it('should call "configure" method if method is implemented', () => {
Expand Down
11 changes: 5 additions & 6 deletions src/microservices/client/client-tcp.ts
Expand Up @@ -53,7 +53,7 @@ export class ClientTCP extends ClientProxy {
const { err, response, disposed } = buffer;
if (disposed) {
callback(null, null, true);
socket.close();
socket.end();
return;
}
callback(err, response);
Expand All @@ -64,12 +64,11 @@ export class ClientTCP extends ClientProxy {
}

public close() {
if (!this.socket) {
return;
if (this.socket) {
this.socket.end();
this.isConnected = false;
this.socket = null;
}
this.socket.close();
this.isConnected = false;
this.socket = null;
}

public bindEvents(socket) {
Expand Down
16 changes: 8 additions & 8 deletions src/microservices/test/client/client-tcp.spec.ts
Expand Up @@ -8,7 +8,7 @@ describe('ClientTCP', () => {
connect: sinon.SinonSpy,
sendMessage: sinon.SinonSpy,
on: sinon.SinonStub,
close: sinon.SinonSpy,
end: sinon.SinonSpy,
};
let createSocketStub: sinon.SinonStub;

Expand All @@ -17,7 +17,7 @@ describe('ClientTCP', () => {
connect: sinon.spy(),
sendMessage: sinon.spy(),
on: sinon.stub().callsFake((event, callback) => event !== 'error' && event !== 'close' && callback({})),
close: sinon.spy(),
end: sinon.spy(),
};
createSocketStub = sinon.stub(client, 'createSocket').callsFake(() => socket);
});
Expand Down Expand Up @@ -65,8 +65,8 @@ describe('ClientTCP', () => {
callback = sinon.spy();
client.handleResponse(socket, callback, { disposed: true });
});
it('should close server', () => {
expect(socket.close.called).to.be.true;
it('should end server', () => {
expect(socket.end.called).to.be.true;
});
it('should emit disposed callback', () => {
expect(callback.called).to.be.true;
Expand All @@ -80,8 +80,8 @@ describe('ClientTCP', () => {
callback = sinon.spy();
client.handleResponse(socket, callback, buffer);
});
it('should not close server', () => {
expect(socket.close.called).to.be.false;
it('should not end server', () => {
expect(socket.end.called).to.be.false;
});
it('should call callback with error and response data', () => {
expect(callback.called).to.be.true;
Expand All @@ -95,8 +95,8 @@ describe('ClientTCP', () => {
(client as any).isConnected = true;
client.close();
});
it('should close() socket', () => {
expect(socket.close.called).to.be.true;
it('should end() socket', () => {
expect(socket.end.called).to.be.true;
});
it('should set "isConnected" to false', () => {
expect((client as any).isConnected).to.be.false;
Expand Down

0 comments on commit a753c50

Please sign in to comment.