swarrot/swarrot — pull request #62 by astorije (2014-10-02 22:34:03+00:00)
Title: Properly set message properties when using the PhpAmqpLibLMessageProvider

On #61, @odolbeau pointed that message properties were not properly set by the provider when using `amqplib` library.

It appeared that the existing code was just setting additional headers and skipping all other properties.

## Rationale on implementation choices

One way to solve that could have been this elegant (based on my very own and objective opinion :p) one-liner:

```php
$properties = union_merge($envelope->get_properties(), $envelope->delivery_info);
```

Problem then comes when a client tries to access a key that has no value, as it raises an exception.

The new implementation is less elegant but at least avoids this problem. I didn't delve too much into the C implementation for the pecl package, but does a function like `$envelope->getContentType()` return something or raises an exception when the `content-type` is not set at message publication?

The second set of properties, `delivery_info` is tested using `isset` rather than `has` because, [according to the doc](https://github.com/videlalvaro/php-amqplib/blob/091/doc/AMQPMessage.md):

> If you don't want to access the delivery_info array directly you can also use $msg->get('delivery_tag') but keep in mind that's slower than just accessing the array by key.

## Issues

This PR, in my opinion, introduces some new issues.

Some property field names differ whether one is using the `PeclPackageMessageProvider` or the `PhpAmqpLibMessageProvider`:

- `is_redelivery` (`PeclPackageMessageProvider`) / `redelivered` (`PhpAmqpLibMessageProvider`)
- `exchange_name` (`PeclPackageMessageProvider`) / `exchange` (`PhpAmqpLibMessageProvider`)
- `headers` (`PeclPackageMessageProvider`) / `application_headers` (`PhpAmqpLibMessageProvider`)

Also, the following fields are now set by the `PhpAmqpLibMessageProvider` but not by the `PeclPackageMessageProvider`:

- `user_id`
- `cluster_id`
- `channel`
- `consumer_tag`

In both cases, since this library is abstracting providers, I think they should use a uniform vocabulary. So far, I have set the one above to simply reflect the terminology of the library, but I actually suggest changing to your liking. For the second set of properties, they should either be added to the other provider (with a default value if not available from the perl package?) or sadly removed from this provider.

Another thing that might need adjusting is the default values. I arbitrarily set them but will update them to your liking. Here again, best would be to make default values between both providers uniform.

## Tests

No tests so far as this PR will probably change, but do you have preferences / suggestions on this?