Refactor mqtt_processor get_vhost functions

In order to clarify preference of different methods. This commit oes
not change functionality.

This highlights some inconsistences:

- If both User/Password and SslLogin are provided, in
  `check_credentials` User/Password takes precedence while in
  `get_vhost` SslLogin

- If SslLogin is provided (but no mapping is found) vhost from port
  mapping has precedence over vhost from UserName, while in case of
  no ssl it is the other way around.

Author: gomoripeti

deps/rabbitmq_mqtt/src/rabbit_mqtt_processor.erl

@@ -1185,65 +1185,56 @@ get_vhost(UserBin, SslLogin, Port) ->
     get_vhost_ssl(UserBin, SslLogin, Port).
 
 get_vhost_no_ssl(UserBin, Port) ->
-    case vhost_in_username(UserBin) of
-        true  ->
-            {vhost_in_username_or_default, get_vhost_username(UserBin)};
-        false ->
-            PortVirtualHostMapping = rabbit_runtime_parameters:value_global(
-                                       mqtt_port_to_vhost_mapping
-                                      ),
-            case get_vhost_from_port_mapping(Port, PortVirtualHostMapping) of
+    case get_vhost_username(UserBin) of
+        undefined ->
+            case get_vhost_from_port_mapping(Port) of
                 undefined ->
-                    {plugin_configuration_or_default_vhost, {rabbit_mqtt_util:env(vhost), UserBin}};
-                VHost ->
-                    {port_to_vhost_mapping, {VHost, UserBin}}
-            end
+                    VhostFromConfig = rabbit_mqtt_util:env(vhost),
+                    {plugin_configuration_or_default_vhost, {VhostFromConfig, UserBin}};
+                VHostFromPortMapping ->
+                    {port_to_vhost_mapping, {VHostFromPortMapping, UserBin}}
+            end;
+        VHostUser ->
+            {vhost_in_username, VHostUser}
     end.
 
 get_vhost_ssl(UserBin, SslLoginName, Port) ->
-    UserVirtualHostMapping = rabbit_runtime_parameters:value_global(
-                               mqtt_default_vhosts
-                              ),
-    case get_vhost_from_user_mapping(SslLoginName, UserVirtualHostMapping) of
+    case get_vhost_from_user_mapping(SslLoginName) of
         undefined ->
-            PortVirtualHostMapping = rabbit_runtime_parameters:value_global(
-                                       mqtt_port_to_vhost_mapping
-                                      ),
-            case get_vhost_from_port_mapping(Port, PortVirtualHostMapping) of
+            case get_vhost_from_port_mapping(Port) of
                 undefined ->
-                    {vhost_in_username_or_default, get_vhost_username(UserBin)};
+                    case get_vhost_username(UserBin) of
+                        undefined ->
+                            VhostFromConfig = rabbit_mqtt_util:env(vhost),
+                            {plugin_configuration_or_default_vhost, {VhostFromConfig, UserBin}};
+                        VHostUser ->
+                            {vhost_in_username, VHostUser}
+                    end;
                 VHostFromPortMapping ->
                     {port_to_vhost_mapping, {VHostFromPortMapping, UserBin}}
             end;
         VHostFromCertMapping ->
             {client_cert_to_vhost_mapping, {VHostFromCertMapping, UserBin}}
     end.
 
-vhost_in_username(UserBin) ->
-    case application:get_env(?APP_NAME, ignore_colons_in_username) of
-        {ok, true} -> false;
-        _ ->
-            %% split at the last colon, disallowing colons in username
-            case re:split(UserBin, ":(?!.*?:)") of
-                [_, _]      -> true;
-                [UserBin]   -> false;
-                []          -> false
-            end
-    end.
-
 get_vhost_username(UserBin) ->
-    Default = {rabbit_mqtt_util:env(vhost), UserBin},
     case application:get_env(?APP_NAME, ignore_colons_in_username) of
-        {ok, true} -> Default;
+        {ok, true} -> undefined;
         _ ->
             %% split at the last colon, disallowing colons in username
             case re:split(UserBin, ":(?!.*?:)") of
                 [Vhost, UserName] -> {Vhost,  UserName};
-                [UserBin]         -> Default;
-                []                -> Default
+                [UserBin]         -> undefined;
+                []                -> undefined
             end
     end.
 
+get_vhost_from_user_mapping(User) ->
+    UserVirtualHostMapping = rabbit_runtime_parameters:value_global(
+                               mqtt_default_vhosts
+                              ),
+    get_vhost_from_user_mapping(User, UserVirtualHostMapping).
+
 get_vhost_from_user_mapping(_User, not_found) ->
     undefined;
 get_vhost_from_user_mapping(User, Mapping) ->
@@ -1255,6 +1246,12 @@ get_vhost_from_user_mapping(User, Mapping) ->
             VHost
     end.
 
+get_vhost_from_port_mapping(Port) ->
+    PortVirtualHostMapping = rabbit_runtime_parameters:value_global(
+                               mqtt_port_to_vhost_mapping
+                              ),
+    get_vhost_from_port_mapping(Port, PortVirtualHostMapping).
+
 get_vhost_from_port_mapping(_Port, not_found) ->
     undefined;
 get_vhost_from_port_mapping(Port, Mapping) ->

deps/rabbitmq_mqtt/test/processor_SUITE.erl

@@ -45,9 +45,13 @@ end_per_testcase(_, Config) -> Config.
 ignore_colons(B) -> application:set_env(rabbitmq_mqtt, ignore_colons_in_username, B).
 
 ignores_colons_in_username_if_option_set(_Config) ->
-    ignore_colons(true),
-    ?assertEqual({rabbit_mqtt_util:env(vhost), <<"a:b:c">>},
-                  rabbit_mqtt_processor:get_vhost_username(<<"a:b:c">>)).
+    clear_vhost_global_parameters(),
+     ignore_colons(true),
+    ?assertEqual(undefined,
+                  rabbit_mqtt_processor:get_vhost_username(<<"a:b:c">>)),
+    ?assertEqual({plugin_configuration_or_default_vhost,
+                  {rabbit_mqtt_util:env(vhost), <<"a:b:c">>}},
+                  rabbit_mqtt_processor:get_vhost(<<"a:b:c">>, none, 1883)).
 
 interprets_colons_in_username_if_option_not_set(_Config) ->
    ignore_colons(false),