-
Notifications
You must be signed in to change notification settings - Fork 63
Send message when transfer finished #244
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
base: master
Are you sure you want to change the base?
Changes from 5 commits
bc15d25
e2c4ad8
df6ad48
f834f3b
24d3618
6f67cc8
2ec6e2d
74006d4
cd36e79
ecf53e9
9bdbe22
0c5cabf
e491cd3
aa7eb8d
d9468ed
ffbb64f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ const knex = require("../database/knex"); | |
| const tracker = mockKnex.getTracker(); | ||
| const Session = require("../models/Session"); | ||
| const uuid = require('uuid'); | ||
| const sinon = require('sinon'); | ||
|
|
||
| describe("TransferRepository", () => { | ||
| let transferRepository; | ||
|
|
@@ -74,5 +75,24 @@ describe("TransferRepository", () => { | |
| expect(result).lengthOf(1); | ||
| }); | ||
|
|
||
| it("getTokensById", async () => { | ||
| const data = [{ | ||
| capture_id: "c", | ||
| token_id: "t", | ||
| }]; | ||
| tracker.uninstall(); | ||
| tracker.install(); | ||
| tracker.on('query', function sendResult(query, step) { | ||
| [ | ||
| function firstQuery() { | ||
| expect(query.sql).match(/capture_id/); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is match here a regex kind? I feel either we should do a robust match by verifying the exact query if possible.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, maybe you are right, I don't realize that I added some tests maybe look not so useful, yes, it might be unnessarary, I will think it over, and another reason lead to this result is that I wrote the test first before I wrote the implementation, so when I link this model to the real system, I don't need to worry too much about that it's some problematic model, it makes integrating the code easier. |
||
| query.response(data); | ||
| }, | ||
| ][step - 1](); | ||
| }); | ||
| const result = await transferRepository.getTokensById(1); | ||
| sinon.assert.match(result, data); | ||
| }); | ||
|
|
||
| }); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| module.exports = { | ||
| config: { | ||
| "vhosts": { | ||
| "test": { | ||
| "connection": { | ||
| "url": process.env.RABBIT_MQ_URL, | ||
| "socketOptions": { | ||
| "timeout": 1000 | ||
| } | ||
| }, | ||
| "exchanges": ["field-data"], | ||
|
||
| "queues": ["field-data-events", "field-data:verifications"], | ||
| "publications": { | ||
| "raw-capture-created": { | ||
| "exchange": "field-data" | ||
| } | ||
| }, | ||
| "subscriptions": { | ||
| "admin-verification": { | ||
| "queue": "field-data:verifications" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| const Broker = require('rascal').BrokerAsPromised; | ||
| const config = require("./MQConfig").config; | ||
| const HttpError = require("../utils/HttpError"); | ||
| const log = require("loglevel"); | ||
|
|
||
| class MQService{ | ||
|
|
||
| constructor(session){ | ||
| this._settsion = session; | ||
| } | ||
|
|
||
| sendMessage(payload){ | ||
| return new Promise((resolve, reject) => { | ||
| const broker = Broker.create(config); | ||
| // TODO | ||
| Promise.resolve(broker) | ||
| .then(broker => { | ||
| broker.publish("raw-capture-created", payload, "field-data.capture.creation") | ||
| .then(publication => { | ||
| publication | ||
| .on("success", () => resolve(true)) | ||
| .on("error", (err, messageId)=> { | ||
| const error = `Error with id ${messageId} ${err.message}`; | ||
| log.error(error); | ||
| reject(new HttpError(500, error)); | ||
| }); | ||
| }) | ||
| .catch(err => { | ||
| log.error(err); | ||
| reject(new HttpError(500, `Error publishing message ${err}`)); | ||
| }) | ||
| }); | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| module.exports = MQService; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| const MQService = require("./MQService"); | ||
| const Broker = require('rascal').BrokerAsPromised; | ||
| const sinon = require("sinon"); | ||
| const {expect} = require("chai"); | ||
| const jestExpect = require("expect"); | ||
|
|
||
| describe("MQService", () => { | ||
|
|
||
| afterEach(() => { | ||
| sinon.restore(); | ||
| }); | ||
|
|
||
| it("send message successfully", async () => { | ||
| const broker = { | ||
| publish: async () => { | ||
| console.log("publish"); | ||
| return { | ||
| on(event, handler){ | ||
| // mock the success event | ||
| if(event === "success"){ | ||
| setImmediate(handler); | ||
| } | ||
| return this; | ||
| } | ||
| } | ||
| } | ||
| }; | ||
| sinon.spy(broker, "publish"); | ||
| sinon.stub(Broker, "create").resolves(broker); | ||
| const mqService = new MQService(); | ||
|
|
||
| const payload = {a:1}; | ||
| const result = await mqService.sendMessage(payload); | ||
| expect(result).eq(true); | ||
| sinon.assert.calledWith(broker.publish, "raw-capture-created", payload, "field-data.capture.creation"); | ||
|
|
||
| }); | ||
|
|
||
| it("send message with problem", async () => { | ||
| sinon.stub(Broker, "create").returns({ | ||
| publish: async () => { | ||
| console.log("publish"); | ||
| return { | ||
| on(event, handler){ | ||
| // mock the error event | ||
| if(event === "error"){ | ||
| setImmediate(() => handler(new Error("Message sending wrong"), "No.1")); | ||
| } | ||
| return this; | ||
| } | ||
| } | ||
| } | ||
| }); | ||
| const mqService = new MQService(); | ||
|
|
||
| await jestExpect(async () => { | ||
| await mqService.sendMessage({a:1}); | ||
| }).rejects.toThrow(/Message sending wrong/); | ||
|
|
||
| }); | ||
|
|
||
| }); |

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renaming this to
getTokenAndCaptureIds(transactionId)would be verbose but more clear. Right now, it is not clear the parameteridis referring the transaction/transfer id or token id.Another lesser preference would be to rename it to
getTokens(transactionId)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good, I agree, I will change it