Skip to content
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

refactor(galileo+admin) use lua-cjson's new granular encoding capabilities for empty arrays #1463

Merged
merged 2 commits into from
Aug 6, 2016

Conversation

thibaultcha
Copy link
Member

@thibaultcha thibaultcha commented Aug 3, 2016

Summary

Now that openresty/lua-cjson#6 has been merged
and OpenResty has been bumped to 1.9.15.1, we can use more granular
empty array encoding options.

Full changelog

  • use cjson.empty_array_mt for ALF serializer
  • use cjson.empty_array for empty page results in API
  • got rid of send_HTTP()'s raw argument as it is not needed anymore
    (temporary workaround) for this use-case
  • appropriate tests and removed now obsolete test for send_HTTP()'s raw
    argument
  • move request size limit tests to the proper suite (/apis/ route)

@thibaultcha thibaultcha changed the title refactor(galileo/api) use lua-cjson's new granular encoding capabilities for empty arrays refactor(galileo+admin) use lua-cjson's new granular encoding capabilities for empty arrays Aug 4, 2016
return gsub(encoded, "\\/", "/"), #self.entries

--return cjson.encode(buf)
return cjson.encode(buf)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing second gsub and second return value here (+ not tested apparently)

@thibaultcha thibaultcha force-pushed the refactor/cjson-empty-array branch 2 times, most recently from a28f873 to 5ddf0aa Compare August 6, 2016 00:50
Now that openresty/lua-cjson#6 has been merged
and OpenResty has been bumped to 1.9.15.1, we can use more granular
empty array encoding for ALF serialization.
As per our previous commit, we use lua-cjson's new granular encoding
capabilities for empty objects into array. We don't use the metatable
here because it seems those are removed before the actual JSON encoding.

* use cjson.empty_array for empty page results
* got rid of send_HTTP()'s `raw` argument as it is not needed anymore
  (temporary workaround) for this use-case
* appropriate tests and removed now obsolete test for send_HTTP()'s raw
  argument
* move request size limit tests to the proper suite (/apis/ route)
* add tests for number of entries returned by `serialize()`'s 2nd return
  value, and removal of escaped slashes in ALF encoding.
@thibaultcha thibaultcha force-pushed the refactor/cjson-empty-array branch from 5ddf0aa to 82ba540 Compare August 6, 2016 00:53
@thibaultcha thibaultcha added pr/ready (but hold merge) No more concerns, but do not merge yet (probably a conflict of interest with another PR or release) and removed pr/status/needs review labels Aug 6, 2016
@thibaultcha thibaultcha merged commit 45a371c into next Aug 6, 2016
@thibaultcha thibaultcha deleted the refactor/cjson-empty-array branch August 6, 2016 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/ready (but hold merge) No more concerns, but do not merge yet (probably a conflict of interest with another PR or release)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant