Duplicated service description entries within gateway config toml file should not result in duplication of service entries

[shankar-in]

Required information

Operating system: Iceoryx Enviroments Linux / QNX

Compiler version: Normal Iceoryx build setup

Observed result or behaviour: If the gateway config toml file has duplicated service descriptions the parser doesnt validates this and creates duplicated service description entries

Expected result or behaviour: The parser should validate this and service entry duplication should not happen

Conditions where it occurred / Performed steps: Ex:

 serviceEntry->insert("service", "service");
    serviceEntry->insert("instance", "instance");
    serviceEntry->insert("event", "event");

    serviceEntry->insert("service", "service");
    serviceEntry->insert("instance", "instance");
    serviceEntry->insert("event", "event");

[elBoberido]

I'm not quite familiar with the gateway config but your example shows C++ code. Could you please clarify this a bit?

[orecham]

Creating duplicate publishers/subscribers/data writers/data readers is probably the wrong behaviour. I can't think of any reason why they should be duplicated. Possible solutions:

• skip over duplicate entries found in the configuration
• terminate application with error, prompting the user to fix the config

The fix should be relatively simple, relevant code is here:

• https://github.com/eclipse-iceoryx/iceoryx/blob/master/iceoryx_dds/include/iceoryx_dds/internal/gateway/iox_to_dds.inl#L41
• https://github.com/eclipse-iceoryx/iceoryx/blob/master/iceoryx_dds/include/iceoryx_dds/internal/gateway/dds_to_iox.inl#L36

Either change return type to be an expected and return an error when duplicates are found or do the filtering in these methods.

[shankar-in]

@elBoberido Yes. its a code snippet to write the data into the toml file. This can be found within iceoryx_posh/test/moduletests/test_popo_toml_gateway_config_parser.cpp.

@ithier yes agreed with your solution

[dkroenke]

@ithier Maybe it make more sense to detect duplicate entries early in the parse() stage? I would make the solution not DDS-specific and apply this to the TomlGatewayConfigParser class which is intended used by all gateways. There is already a validate function available that can be extended to detect and handle the duplicate service entries: https://github.com/eclipse-iceoryx/iceoryx/blob/master/iceoryx_posh/source/gateway/toml_gateway_config_parser.cpp#L78

[elfenpiff]

@dkroenke I agree and I would not return an error when duplicate entries are found. This does in no way produce an unexpected outcome, destabilize our software or would somehow seem as a bug. We just state in the readme of the Toml-Config - for every unique entry one publisher is created. Identical entries are ignored. But it is maybe helpful for the user when we print a warning message since clearly someone has messed up when such a config is created.

[dkroenke]

According to service_description.hpp it is possible to have a "reduced" ServiceDescription without EventID. For example the service discovery protocol of SOME/IP needs only a service and event identifier. Currently the gateway config enforces to have the "three friends" available in the config. We should also allow the reduced variant here.

Edit: The reduced ServiceDescription is only needed for ServiceDiscovery, but in a gateway you normally want to do more than only SD with a service, therefore it is ok to enforce the full ServiceDescription here.

[dkroenke]

A question came up in this thread: https://github.com/eclipse-iceoryx/iceoryx/pull/579#discussion_r586646938 Is it ok to print only a logmessage if the filepath for the config is not found or empty? Or should we maybe return a cxx::error here?

[elBoberido]

With the simplification for #1949 I discovered a bug in the tests. DuplicatedServicesDescriptionInTomlFileReturnOnlyOneEntry was buggy. I fixed the test and skipped it, since no de-duplication is performed. From the discussion above the desired behaviour it is not clear to me.