parse-community / parse-server-push-adapter

A push notification adapter for Parse Server
https://parseplatform.org
MIT License
88 stars 100 forks source link

parse server removing device tokens despite disabled in config #112

Closed funkenstrahlen closed 6 years ago

funkenstrahlen commented 6 years ago

I have set PARSE_SERVER_CLEANUP_INVALID_INSTALLATIONS: 0 in my heroku config.

However I do see it removes device tokens in the logs:

2018-04-12T13:58:37.911121+00:00 app[web.1]: info: Removing device tokens on 1 _Installations

I run parse-server 2.7.4.

I did have a look at the code checking the config value and it looks correct in my eyes. I do not know whats going wrong.

funkenstrahlen commented 6 years ago

I noticed it does not remove device token if the environment variable PARSE_SERVER_CLEANUP_INVALID_INSTALLATIONS is not set at all.

Why does it not understand PARSE_SERVER_CLEANUP_INVALID_INSTALLATIONS=0 as disabled?

mman commented 6 years ago

@funkenstrahlen Because JavaScript 🤷‍♂️.

The https://github.com/parse-community/parse-server/blob/c1a734714361ba65e32c9781d3c8f924f1187df5/src/StatusHandler.js#L207 uses a statement:

cleanupInstallations = process.env.PARSE_SERVER_CLEANUP_INVALID_INSTALLATIONS

Which according to the node doc here (https://nodejs.org/api/process.html#process_process_env) returns something but definitely not a boolean - most probably a string or object.

The subsequent test here (https://github.com/parse-community/parse-server/blob/c1a734714361ba65e32c9781d3c8f924f1187df5/src/StatusHandler.js#L261):

if (devicesToRemove.length > 0 && cleanupInstallations) {

does definitely not test whether the value is set to 0 or 1 but simply that the value is not undefined.

I guess the suggested fix here should be should be to use something like

if (devicesToRemove.length > 0 && cleanupInstallations == '1') {

I don't have a time to test this today, but looks like a simple and non-risky fix to try to get in.

funkenstrahlen commented 6 years ago

Awesome! Thanks for your great explanation.