search

eu-federation-gateway-service / efgs-federation-gateway

The goal of this project is to develop the official European solution for the interoperability between national backend servers of decentralised contact tracing applications to combat COVID-19.
Apache License 2.0
59 stars 25 forks source link

Uploader batch tag uniqueness should be checked per country, not globally #246

Closed solita-alperttiti closed 3 years ago

solita-alperttiti commented 3 years ago

Describe the bug

In UploadController uploader batchTag is documented like this (which is suggesting that uploader batchTag does not have to be unique):

 @Parameter(
         name = "batchTag",
         in = ParameterIn.HEADER,
         required = true,
         description = "Required Tag to tag the send batch (must be not unique).",
         example = "20200731-1"
       )

However, in actual implementation batchTag uniqueness is required:

https://github.com/eu-federation-gateway-service/efgs-federation-gateway/blob/a9d29a3c82480d034e84b244b0230933bf1b40aa/src/main/java/eu/interop/federationgateway/controller/UploadController.java#L162

And it's also checked against all uploader batchTags sent from any country:

https://github.com/eu-federation-gateway-service/efgs-federation-gateway/blob/a9d29a3c82480d034e84b244b0230933bf1b40aa/src/main/java/eu/interop/federationgateway/service/DiagnosisKeyEntityService.java#L48

So, documenation conflicts with implementation. Also, check is made globally which is error prone solution.

Expected behaviour

At for start, documentation should be fixed to be unambiguous with actual implementation.

Secondly, I think uniqueness shouldn't be checked globally, because at national backend level it's hard to guess what kind of batchTags other countries would generate. This is also error prone solution for changes in batchTag naming logic. It also seems that uploader batchTag is not unique field in the database, so this is an implementation level issue.

So, I suggest required changes will be done for checking uniqueness per country.

Possible Fix

Starting point could be to change this count query to be per country:

https://github.com/eu-federation-gateway-service/efgs-federation-gateway/blob/a9d29a3c82480d034e84b244b0230933bf1b40aa/src/main/java/eu/interop/federationgateway/repository/DiagnosisKeyEntityRepository.java#L43

Obviously, there might be some other changes required.

Steps to reproduce the issue

Just upload something with provided example (yyyymmdd-1) and there is probably going to be conflicts in some point.

daniel-eder commented 3 years ago

Hi @solita-alperttiti, thank you for your report! Our developers will review this issue and discuss your suggested changes.

ascheibal commented 3 years ago

Hi @solita-alperttiti, currently there is an overall uniqueness needed. A 'per-country' uniqueness would be an improvement to the current system. We will update the openAPI documents accordingly to prevent further misunderstanding. PR: #255