Foo-Foo-MQ / foo-foo-mq

Abstractions around RabbitMQ
MIT License
49 stars 24 forks source link

Some properties are not supported by foo-foo-mq ( eg. priority ) #46

Closed shifubrams closed 1 year ago

shifubrams commented 1 year ago

Hello, in the documentation, here, it is mentionned that all the properties available are in this example. But, i tried to put priority ( enforced the typing ) and it ignores it.

Is it possible to manage priority as the package is ? If not, can you please add this field to the publish options ?

List of the supported rabbitmq properties :

image

Example :

try {
      await this.rabbit.publish("my-exchange", {
        priority: 7, // tried to put the property in the root of options
        properties: {
          priority: 5  // tried to put the property in a "properties" field of options
        },
        headers: {priority: "10"}, // tried to put the property as a header field, it's added but not considered as a property
        body: { "f1":"v1" },
      } as PublishOptions);
} catch (error) {
       console.log(error);
}

This example above gives this message in rabbit :

image

Using : rabbitmq : v3.8.3 foo-foo-mq : v8.0.0 NodeJS : 18

shifubrams commented 1 year ago

a solution would be to append this object https://github.com/Foo-Foo-MQ/foo-foo-mq/blob/e492c72d89f871b16ecf80f749e76d8e95097900/src/amqp/exchange.js#L79 to take in account the official properties like priority. ( tested and working ).

  const publishOptions = {
    type: message.type || '',
    contentType: contentType,
    contentEncoding: 'utf8',
    correlationId: message.correlationId || '',
    replyTo: message.replyTo || topology.replyQueue.name || '',
    messageId: message.messageId || message.id || '',
    timestamp: message.timestamp || Date.now(),
    appId: message.appId || info.id,
    headers: message.headers || {},
    expiration: message.expiresAfter || undefined,
    mandatory: message.mandatory || false,
    priority: message.priority || undefined
  };
zlintz commented 1 year ago

@shifubrams please open a pull request for this

zlintz commented 1 year ago

Also if you don't mind including the appropriate documentation updates that would be great

shifubrams commented 1 year ago

@zlintz can we just open a branch on this repo to avoid forking the whole repo ? I get a 403 when i want to push my branch

shifubrams commented 1 year ago

can i have a follow up on this please ?

zlintz commented 1 year ago

Sure I’ll take a look at the repo settings today.

zlintz commented 1 year ago

You should be able to now

shifubrams commented 1 year ago

hello, sorry i was able to create the branch but not to push on it.

image