frmscoe / admin-service

admin-service
Apache License 2.0
0 stars 1 forks source link

BUG: condition creation returns an error all the time #81

Closed Justus-at-Tazama closed 1 week ago

Justus-at-Tazama commented 2 weeks ago

Regardless of the payload, the submission of a new condition to /v1/admin/event-flow-control/entity returns the following error:

{
    "statusCode": 500,
    "error": "Internal Server Error",
    "message": "ERR invalid expire time in 'set' command"
}

Sample payloads:

Unknown entity, override ```JSON { "xprtnDtTm": "2024-09-05T02:50:13.791Z", "evtTp": [ "pain.001.001.11", "pain.013.001.09", "pacs.008.001.10", "pacs.002.001.12" ], "condTp": "override", "prsptv": "both", "condRsn": "Default Test Condition", "ntty": { "id": "c63a63d413bf476ca0dd85c2d302d817", "schmeNm": { "prtry": "TAZAMA_EID" } }, "forceCret": true, "usr": "POSTMAN" } ```
Unknown entity, non-overridable-block ```JSON { "evtTp": [ "pain.001.001.11", "pain.013.001.09", "pacs.008.001.10", "pacs.002.001.12" ], "condTp": "non-overridable-block", "prsptv": "both", "condRsn": "Default Test Condition", "ntty": { "id": "e03ee7acdcb747c7bb1cf761c892b917", "schmeNm": { "prtry": "TAZAMA_EID" } }, "forceCret": true, "usr": "POSTMAN" } ```
Unknown entity, overridable-block ```JSON { "evtTp": [ "pain.001.001.11", "pain.013.001.09", "pacs.008.001.10", "pacs.002.001.12" ], "condTp": "overridable-block", "prsptv": "both", "condRsn": "Default Test Condition", "ntty": { "id": "6e98c511c84b471ba91b7397213fcbce", "schmeNm": { "prtry": "TAZAMA_EID" } }, "forceCret": true, "usr": "POSTMAN" } ```

The error occurs on /v1/admin/event-flow-control/account as well. The condition is created successfully anyway, regardless of the error response.

Justus-at-Tazama commented 2 weeks ago

The problem appears to be here:

https://github.com/frmscoe/admin-service/blob/3b3ab6b99eb8b1336cdcd6db3f299d5b7079f550/src/utils/update-cache.ts#L11C55-L11C63

https://valkey.io/commands/set/

Redis/Valkey cannot accept a TTL value of 0.

The cache in ValKey for conditions should never expire. The correct way to specify this in the code would be to omit the TTL altogether, though it should be noted that the .env.template specifies a sample TTL of 1000:

CACHE_TTL=1000

This value should be omitted in the .env.template as well, though I am not sure if the admin-service can always be expected to only handle cache interaction for the event flow sub-system. If the admin service does handle other cache-capable sub-systems in the future, then the cache TTL value will be in conflict across different sub-systems.

I recommend that the instruction to populate the cache with a new condition simply doesn't specify the TTL at all.

@johanfol - please confirm?

PS: Sadly, simply omitting the value from the .env doesn't work since the config.ts defaults it to 0 anyway:

https://github.com/frmscoe/admin-service/blob/3b3ab6b99eb8b1336cdcd6db3f299d5b7079f550/src/config.ts#L28

Why did the unit testing not pick up this issue?

cshezi commented 2 weeks ago

Unit tests are mocking every interaction of the library which could be the reason why the unit tests never picked this issue up