-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixing null pointer exception on DAO update when config is missing #571
Conversation
@thibaultcha - this fix seems to work. |
It should probably rather be fixed at the model layer by setting it to an empty object by default. |
By doing so, will the DAO store an empty JSON object (as a result of the serialization of an empty table into a string)? |
Yes, but since a configuration should be mandatory for all plugins, it might be better having an empty objects that handling the null case (this is an example of having to handle the null case, kong.lua is another one) |
6eeaf08
to
6cb10cf
Compare
I am experiencing a weird behavior in my tests. Adding a Failure → spec/integration/dao/cassandra/base_dao_spec.lua @ 612
Cassandra Base DAO plugins :update() should properly update a plugin with an empty configuration
spec/integration/dao/cassandra/base_dao_spec.lua:623: Expected to be falsy, but value was:
(table) {
[is_dao_error] = true
[message] = {
[config.hide_credentials] = 'hide_credentials is an unknown field'
[config.key_names] = 'key_names is an unknown field' }
[schema] = true } at the following line: https://github.com/Mashape/kong/blob/fix/dao-update/spec/integration/dao/cassandra/base_dao_spec.lua#L623 It seems like the error is completely unrelated since I am explicitly saving a |
Seems like these lines are affecting the test later on: https://github.com/Mashape/kong/blob/fix/dao-update/spec/integration/dao/cassandra/base_dao_spec.lua#L111-L118 Commenting them out makes everything work. |
Ok I will check this this we |
Because the schema validation was expecting a literal, it did not copy the empty object in case of using it to specify the `default` of a property, thus leading later unrelated inserted plugins to have invalid configs. See test case in entities_schemas_spec. Fix #570
6cb10cf
to
18020aa
Compare
Rewrote the PR, see updated changes |
Fixing null pointer exception on DAO update when config is missing
* Mashape/master: (23 commits) Update README.md Update README.md Closing Kong#562 Adding wait time before ratelimiting tests Fixing test fix(jwt) handle `iss` not being found in jwt credentials Update README.md docs(update) remove redundancy docs(update) fix layout fix(test) fix config test after Kong#563 Update README.md Adding missing statement for Kong#571 perf(analytics) global optimizations fix(plugins) make default config for plugins an empty object Closes Kong#445 dbocs(changelog) 0.5.0 changes Better content-type check in response-transformer plugin Closes Kong#535 Fixes the root problem at Kong#565 fix(key-auth) remove support for key in request body ...
Fix for #570.