From e49524a2ba1c37b72635eeff08fcfd6be47cf518 Mon Sep 17 00:00:00 2001 From: Daniel Shmuglin Date: Thu, 29 Oct 2020 10:24:31 +0200 Subject: [PATCH 1/3] Server#addService - lift the limitation of adding a new service to started server --- packages/grpc-js/src/server.ts | 6 +----- packages/grpc-js/test/test-server.ts | 6 +++--- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/grpc-js/src/server.ts b/packages/grpc-js/src/server.ts index 5ac25c7c..c0ced6f7 100644 --- a/packages/grpc-js/src/server.ts +++ b/packages/grpc-js/src/server.ts @@ -148,10 +148,6 @@ export class Server { service: ServiceDefinition, implementation: UntypedServiceImplementation ): void { - if (this.started === true) { - throw new Error("Can't add a service to a started server."); - } - if ( service === null || typeof service !== 'object' || @@ -637,7 +633,7 @@ async function handleUnary( if (request === undefined || call.cancelled) { return; } - + const emitter = new ServerUnaryCallImpl( call, metadata, diff --git a/packages/grpc-js/test/test-server.ts b/packages/grpc-js/test/test-server.ts index 434efbbc..eb531c96 100644 --- a/packages/grpc-js/test/test-server.ts +++ b/packages/grpc-js/test/test-server.ts @@ -202,7 +202,7 @@ describe('Server', () => { }); }); - it('fails if the server has been started', done => { + it('succeeds after server has been started', done => { const server = new Server(); server.bindAsync( @@ -211,9 +211,9 @@ describe('Server', () => { (err, port) => { assert.ifError(err); server.start(); - assert.throws(() => { + assert.doesNotThrow(() => { server.addService(mathServiceAttrs, dummyImpls); - }, /Can't add a service to a started server\./); + }); server.tryShutdown(done); } ); From 7c3ccda8fff80fe2677ca27ebf4ed7499bd58731 Mon Sep 17 00:00:00 2001 From: Daniel Shmuglin Date: Thu, 29 Oct 2020 10:54:33 +0200 Subject: [PATCH 2/3] implement Server#unregister(handlerName) --- packages/grpc-js/src/server.ts | 4 ++ packages/grpc-js/test/test-server.ts | 55 ++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/packages/grpc-js/src/server.ts b/packages/grpc-js/src/server.ts index c0ced6f7..341b28bd 100644 --- a/packages/grpc-js/src/server.ts +++ b/packages/grpc-js/src/server.ts @@ -458,6 +458,10 @@ export class Server { return true; } + unregister(name: string): boolean { + return this.handlers.delete(name); + } + start(): void { if ( this.http2ServerList.length === 0 || diff --git a/packages/grpc-js/test/test-server.ts b/packages/grpc-js/test/test-server.ts index eb531c96..6a95b5cf 100644 --- a/packages/grpc-js/test/test-server.ts +++ b/packages/grpc-js/test/test-server.ts @@ -220,6 +220,61 @@ describe('Server', () => { }); }); + describe('unregister', () => { + + let server: Server; + let client: ServiceClient; + + const mathProtoFile = path.join(__dirname, 'fixtures', 'math.proto'); + const mathClient = (loadProtoFile(mathProtoFile).math as any).Math; + const mathServiceAttrs = mathClient.service; + + beforeEach(done => { + server = new Server(); + server.addService(mathServiceAttrs, { + div(call: ServerUnaryCall, callback: sendUnaryData) { + callback(null, {quotient: '42'}); + }, + }); + server.bindAsync( + 'localhost:0', + ServerCredentials.createInsecure(), + (err, port) => { + assert.ifError(err); + client = new mathClient( + `localhost:${port}`, + grpc.credentials.createInsecure() + ); + server.start(); + done(); + } + ); + }); + + afterEach(done => { + client.close(); + server.tryShutdown(done); + }); + + it('removes handler by name and returns true', done => { + const name = mathServiceAttrs['Div'].path; + assert.strictEqual(server.unregister(name), true, 'Server#unregister should return true on success'); + + client.div( + { divisor: 4, dividend: 3 }, + (error: ServiceError, response: any) => { + assert(error); + assert.strictEqual(error.code, grpc.status.UNIMPLEMENTED); + done(); + } + ); + }); + + it('returns false for unknown handler', () => { + assert.strictEqual(server.unregister('noOneHere'), false, 'Server#unregister should return false on failure'); + }); + }); + it('throws when unimplemented methods are called', () => { const server = new Server(); From 51ca00298e95f67727c9e10549e0500629f17987 Mon Sep 17 00:00:00 2001 From: Daniel Shmuglin Date: Thu, 29 Oct 2020 11:33:29 +0200 Subject: [PATCH 3/3] implement Server#unregisterService(serviceDefinition) --- packages/grpc-js/src/server.ts | 15 +++++++ packages/grpc-js/test/test-server.ts | 61 ++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/packages/grpc-js/src/server.ts b/packages/grpc-js/src/server.ts index 341b28bd..28599160 100644 --- a/packages/grpc-js/src/server.ts +++ b/packages/grpc-js/src/server.ts @@ -208,6 +208,21 @@ export class Server { }); } + removeService(service: ServiceDefinition): void { + if ( + service === null || + typeof service !== 'object' + ) { + throw new Error('removeService() requires object as argument'); + } + + const serviceKeys = Object.keys(service); + serviceKeys.forEach((name) => { + const attrs = service[name]; + this.unregister(attrs.path); + }); + } + bind(port: string, creds: ServerCredentials): void { throw new Error('Not implemented. Use bindAsync() instead'); } diff --git a/packages/grpc-js/test/test-server.ts b/packages/grpc-js/test/test-server.ts index 6a95b5cf..58b10288 100644 --- a/packages/grpc-js/test/test-server.ts +++ b/packages/grpc-js/test/test-server.ts @@ -220,6 +220,67 @@ describe('Server', () => { }); }); + describe('removeService', () => { + let server: Server; + let client: ServiceClient; + + const mathProtoFile = path.join(__dirname, 'fixtures', 'math.proto'); + const mathClient = (loadProtoFile(mathProtoFile).math as any).Math; + const mathServiceAttrs = mathClient.service; + const dummyImpls = { div() {}, divMany() {}, fib() {}, sum() {} }; + + beforeEach(done => { + server = new Server(); + server.addService(mathServiceAttrs, dummyImpls); + server.bindAsync( + 'localhost:0', + ServerCredentials.createInsecure(), + (err, port) => { + assert.ifError(err); + client = new mathClient( + `localhost:${port}`, + grpc.credentials.createInsecure() + ); + server.start(); + done(); + } + ); + }); + + afterEach(done => { + client.close(); + server.tryShutdown(done); + }); + + it('succeeds with a single service by removing all method handlers', done => { + server.removeService(mathServiceAttrs); + + let methodsVerifiedCount = 0; + const methodsToVerify = Object.keys(mathServiceAttrs); + + const assertFailsWithUnimplementedError = (error: ServiceError) => { + assert(error); + assert.strictEqual(error.code, grpc.status.UNIMPLEMENTED); + methodsVerifiedCount++; + if (methodsVerifiedCount === methodsToVerify.length) { + done(); + } + }; + + methodsToVerify.forEach((method) => { + const call = client[method]({}, assertFailsWithUnimplementedError); // for unary + call.on('error', assertFailsWithUnimplementedError); // for streamed + }); + }); + + it('fails for non-object service definition argument', () => { + assert.throws(() => { + server.removeService('upsie' as any) + }, /removeService.*requires object as argument/ + ); + }); + }); + describe('unregister', () => { let server: Server;