moscajs / mosca

MQTT broker as a module
mosca.io
3.2k stars 509 forks source link

add message to publish callback #527

Closed spali closed 8 years ago

spali commented 8 years ago

allows the callback to get the generated message id or any other content. i.e. usefully for filtering own published message from the published hook.

against the contribution guidelines, could not run tests for this commit. Currently have some problems with other dependencies. Hope this small change didn't break anything. At least could find code that should break with this.

mcollina commented 8 years ago

This is missing a unit test.

spali commented 8 years ago

Could get test to run on my client. I'm really new to this, would be my first test :)

I would put it in tests/server.js after the test "should support subscribing via server.subscribe": Hope this makes any sence as test

  it("should provide packet in callback", function(done) {
    var messageId;

    this.instance.once("published", function(packet) {
      messageId = packet.messageId;
    });

    this.instance.publish({
      topic: "hello",
      payload: "some data"
    }, function(error, packet) {
      expect(packet.topic).to.be.equal("hello");
      expect(packet.payload.toString().toString()).to.be.equal("some data");
      expect(packet.messageId.toString()).to.equal(messageId);
      done();
    });
  });

please let me know if I should push this to the PR.

mcollina commented 8 years ago

I would cal it "should provide packet in publish callback". Anyway, yes, push!

Il giorno sab 23 lug 2016 alle 20:06 Thomas Spalinger < notifications@github.com> ha scritto:

Could get test to run on my client. I'm really new to this, would be my first test :)

I would put it in tests/server.js after the test "should support subscribing via server.subscribe": Hope this makes any sence as test

it("should provide packet in callback", function(done) { var messageId;

this.instance.once("published", function(packet) {
  messageId = packet.messageId;
});

this.instance.publish({
  topic: "hello",
  payload: "some data"
}, function(error, packet) {
  expect(packet.topic).to.be.equal("hello");
  expect(packet.payload.toString().toString()).to.be.equal("some data");
  expect(packet.messageId.toString()).to.equal(messageId);
  done();
});

});

please let me know if I should push this to the PR.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/mcollina/mosca/pull/527#issuecomment-234731901, or mute the thread https://github.com/notifications/unsubscribe-auth/AADL4zaGHJisKonaLZ6a8fn-YLJACx3lks5qYlgngaJpZM4JTWSO .

spali commented 8 years ago

Renamed the test in the commit as you supposed. My test seems to work, but mongo was not available in the travis build. Locally all tests passed with node 6.3. So I assume this PR can be merged.

mcollina commented 8 years ago

Thanks!!