Update ClassBasedRoutingKeyResolver.php

[holtkamp]

Allow other separator characters. Amazon SQS does not allow the (default) dots in queue names...

[holtkamp]

Guess this does not make sense as we can not influence how resolveRoutingKeyFor() is invoked. Need a separate RoutingKeyResolver for this...

[cmodijk]

What we could do is the following

class ClassBasedRoutingKeyResolver implements RoutingKeyResolver
{
    private $separator;

    public function __construct($separator = '.')
    {
        $this->separator = $separator;
    }

    public function resolveRoutingKeyFor($message)
    {
        return str_replace(
            '\\',
            $this->separator,
            get_class($message)
        );
    }
}

[holtkamp]

An additional feature that "would be nice" / I found myself implementing, is allowing a prefix.

UseCase
In our DTAP approach (Development, Test, Acceptation, Production) it is required to separate the Message Queues properly. Since a provider like Amazon SQS does not offer an (easy) way to create separate "projects" and using separate accounts / API credentials seems overkill: we prefix the Message Queues, for a class Project\Domain\Command\DoStuff this would result in queues like:

• development-Project-Domain-Command-DoStuff
• test-Project-Domain-Command-DoStuff
• acceptation-Project-Domain-Command-DoStuff
• production-Project-Domain-Command-DoStuff

So next to a separator, a prefix would be nice as well 😄

[coveralls]

Coverage decreased (-13.6%) to 86.441% when pulling b47ecc8 on holtkamp:patch-1 into 82756d6 on SimpleBus:master.

[holtkamp]

The rationale of

is_object($message) ? get_class($message) : $message

is to be able to determine a queue name easily:

$queueName = $queueResolver->resolveRoutingKeyFor(\Project\Domain\Command\Name::class);

When the queue name is known, it is easier to consume messages from specific queues only

[holtkamp]

@cmodijk Anything missing for a merge?

[holtkamp]

@cmodijk shameless bump, encountered this again 😄

[cmodijk]

The prefix part is really specific to your use case and i would suggest to keep this class simple. Maybe ad a new one and use inversion to implement a prefix/suffix style helper like this.

# We change the simple bus method to this 
class ClassBasedRoutingKeyResolver implements RoutingKeyResolver
{
    private $separator;

    public function __construct($separator = '.')
    {
        $this->separator = $separator;
    }

    public function resolveRoutingKeyFor($message)
    {
        return str_replace(
            '\\',
            $this->separator,
            get_class($message)
        );
    }
}

# Create a new class i'm not sure if this is something for this repo but this is a example
class PrefixBasedResolver implements RoutingKeyResolver
{
    private $parent;
    private $prefix;
    private $suffix;

    public function __construct(RoutingKeyResolver $parent, $prefix = null, $suffix = null)
    {
        $this->parent = $parent;
        $this->prefix = $prefix;
        $this->suffix = $suffix;
    }

    public function resolveRoutingKeyFor($message)
    {
        $message = '';

        if (isset($this->prefix)) {
            $message .= $this->prefix;
        }

        $message .= $this->parent->resolveRoutingKeyFor($message);

        if (isset($this->suffix)) {
            $message .= $this->prefix;
        }

        return $message;
    }
}

# In your case u can use this like this
new PrefixBasedResolver(new ClassBasedRoutingKeyResolver('-'), 'development-', '-more-stuff')

[holtkamp]

@cmodijk mmm, yeah, might be better to keep the default as simple as possible.

Do you agree with this part though?

#9 (comment)

[cmodijk]

Do you agree with this part though?

Does this happen that the message provide is not a object? If so yes we should add this check.

[holtkamp]

Does this happen that the message provide is not a object?

Yes, this allows us to:

• determine the Queue name for specific Commands (each type of Command has a different Queue)
• check this specific Queue for Messages and consume/process them

This can be handy when certain Commands (on a specific Queue) should be assigned a higher priority / should not be dealt with during office hours (database intensive processing), etc.

[cmodijk]

@holtkamp Yeah we could leave that in but it would still mean that this pull request needs a update to only be able to change the separator like I suggest earlier #9 (comment)

[holtkamp]

@cmodijk would you accept a PrefixBasedResolver class in this repository?

Your comment states:

Create a new class i'm not sure if this is something for this repo but this is a example

You want me to add such a class to a new PR?

[cmodijk]

You want me to add such a class to a new PR?

Sure if you can update this PR i'm willing to merge it into repository as long as it is a new class.

[cmodijk]

@holtkamp Do you still need this issue, if so can you update the pull request?

[holtkamp]

mmm, thanks for the reminder, will try to work on it this week

[cmodijk]

@holtkamp No problem I did not had much time lately.

[holtkamp]

@cmodijk done! Separated the classes 🤓

[cmodijk]

@holtkamp Can you add a tests for this?

[holtkamp]

@cmodijk done, good you requested this, some serious stupidities were in my code 😨

[cmodijk]

After talking to @ruudk we are closing this we don't want to support this in the main repository. With the interfaces you are already able to support this in your own project. If you have any feedback or other suggestions please let us know.

[holtkamp]

If you have any feedback or other suggestions please let us know.

mmm, thought now that is a well separated class this would not harm anyone to get merged...