Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Session y struct #1

Merged
merged 4 commits into from Jul 23, 2021
Merged

Conversation

mdanelutti
Copy link

Se agregaron dos puntos:

  • En el consumer tener session y por otro lado que el handler valide la estructura del record y en caso de tener la propiedad del cliente se cree la session.
  • El handler valida si el consumer tiene definido un struct en caso de tenerlo los body de los records pasan por el struct.

@coveralls
Copy link

coveralls commented Jul 14, 2021

Pull Request Test Coverage Report for Build 1057049128

  • 52 of 52 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 603513573: 0.0%
Covered Lines: 82
Relevant Lines: 82

💛 - Coveralls

Copy link
Member

@jormaechea jormaechea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Te deje algunos comentarios (más que nada typos y ajustes en el JSDoc).

Lo que sí falta, es documentar en el README. Hay que documentar tanto el struct, como el messageAttributes con janis-client que se debe mandar y que si se manda, vas a tener el this.session disponible en tu handler..

lib/sqs-handler.js Outdated Show resolved Hide resolved
* @param {import('./sqs-consumer')} Consumer
* @param {SQSEvent} event
* @param {boolean} isBatch
* @returns {Promise}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solo para dejar en claro que el valor al que resuelve va a ser ignorado...

Suggested change
* @returns {Promise}
* @returns {Promise<void>}

lib/sqs-handler.js Outdated Show resolved Hide resolved
lib/sqs-handler.js Outdated Show resolved Hide resolved
lib/sqs-handler.js Outdated Show resolved Hide resolved
sinon.assert.calledOnceWithExactly(ConditionalConsumer.prototype.setSession, { clientCode: 'fizzmodarg' });
});

it('Should pass the event by the consumer with the sessions are setted', async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it('Should pass the event by the consumer with the sessions are setted', async () => {
it('Should pass the event to the consumer with the session set for each of them', async () => {

sinon.assert.calledWithExactly(ConditionalConsumer.prototype.setSession.getCall(1), { clientCode: 'test' });
});

it('Should pass the event by the consumer with the sessions are setted and omit the session for the record without client', async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it('Should pass the event by the consumer with the sessions are setted and omit the session for the record without client', async () => {
it('Should pass the event to the consumer with the sessions set only for the records with janis-client and omitted for the records without it', async () => {

await assert.doesNotReject(SQSHandler.handle(ConditionalConsumerWithArrayStruct, eventWithoutClient));
});

it('Should throw error if the structure body in the records are not correct for a batch', async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it('Should throw error if the structure body in the records are not correct for a batch', async () => {
it('Should reject if the body structure of the records are invalid when processing a batch', async () => {

sinon.assert.calledWithExactly(ConditionalConsumer.prototype.setSession.getCall(1), { clientCode: 'test' });
});

it('Should process if the structure body in the records are correct', async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it('Should process if the structure body in the records are correct', async () => {
it('Should process if the body structure in the records are valid', async () => {

sinon.assert.notCalled(ConditionalConsumerWithStruct.prototype.processBatch);
});

it('Should throw error if the structure body in the records are not correct at processing one by one', async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it('Should throw error if the structure body in the records are not correct at processing one by one', async () => {
it('Should reject if the body structure of the records are invalid when processing one by one', async () => {

@jormaechea jormaechea merged commit b7651aa into master Jul 23, 2021
@jormaechea jormaechea deleted the OMS-108-consumer-with-struct-and-session branch July 23, 2021 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants