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

[C++][Pistache] Add compatibility for nlohmann-json 3.5.0 #3306

Merged

Conversation

muttleyxd
Copy link
Contributor

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\. If contributing template-only or documentation-only changes which will change sample output, be sure to build the project first.
  • Filed the PR against the correct branch: master, 4.1.x, 5.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.
    @ravinikam @stkrwork @etherealjoy @MartinDelille

Description of the PR

While generated sample downloads nlohmann-json from GitHub, it could also be provided by operating system. Debian 10 Buster provides nlohmann-json3-dev package with 3.5.0 version https://packages.debian.org/buster/nlohmann-json3-dev.

contains was added in 3.6.0 https://github.com/nlohmann/json/releases/tag/v3.6.0

This change makes generated code compatible with 3.5.0.

Here's the code I tested it with:

#include <nlohmann/json.hpp>
#include <iostream>

void check_with_find(char const * const key, nlohmann::json const &json)
{
  if (json.find(key) != json.end())
    std::cout << "find: " << key << " exists!\n";
  else
    std::cout << "find: " << key << " does not exist!\n";
}

void check_with_contains(char const * const key, nlohmann::json const &json)
{
  if (json.contains(key))
    std::cout << "contains: " << key << " exists!\n";
  else
    std::cout << "contains: " << key << " does not exist!\n";
}

int main()
{
  constexpr char const * existing_key = "hello";
  constexpr char const * not_existing_key = "there";

  nlohmann::json json;  
  json[existing_key] = "general";

  check_with_find(existing_key, json);
  check_with_find(not_existing_key, json);
  check_with_contains(existing_key, json);
  check_with_contains(not_existing_key, json);
}

Output:

$ g++ -std=c++17 -Wall -Wextra -pedantic json-find.cpp -o json-find
$ ./json-find
find: hello exists!
find: there does not exist!
contains: hello exists!
contains: there does not exist!

contains is implemented as:

template<typename KeyT>
bool contains(KeyT&& key) const
{
    return is_object() and m_value.object->find(std::forward<KeyT>(key)) != m_value.object->end();
}

So it's almost the same code

@muttleyxd muttleyxd force-pushed the pistache-compatibility-json-35 branch 2 times, most recently from 5deeadc to 4ef1553 Compare July 9, 2019 10:53
@muttleyxd
Copy link
Contributor Author

How do I restart CI here? CircleCI failed with

--- a/docs/generators/java.md
+++ b/docs/generators/java.md
@@ -55,4 +55,4 @@ sidebar_label: java
 |feignVersion|Version of OpenFeign: '10.x', '9.x' (default)| |false|
 |useReflectionEqualsHashCode|Use org.apache.commons.lang3.builder for equals and hashCode in the models. WARNING: This will fail under a security manager, unless the appropriate permissions are set up correctly and also there's potential performance impact.| |false|
 |caseInsensitiveResponseHeaders|Make API response's headers case-insensitive. Available on okhttp-gson, jersey2 libraries| |false|
-|library|library template (sub-template) to use|<dl><dt>**jersey1**</dt><dd>HTTP client: Jersey client 1.19.x. JSON processing: Jackson 2.8.x. Enable Java6 support using '-DsupportJava6=true'. Enable gzip request encoding using '-DuseGzipFeature=true'. IMPORTANT NOTE: jersey 1.x is no longer actively maintained so please upgrade to 'jersey2' or other HTTP libaries instead.</dd><dt>**jersey2**</dt><dd>HTTP client: Jersey client 2.25.1. JSON processing: Jackson 2.8.x</dd><dt>**feign**</dt><dd>HTTP client: OpenFeign 9.x or 10.x. JSON processing: Jackson 2.8.x. To enable OpenFeign 10.x, set the 'feignVersion' option to '10.x'</dd><dt>**okhttp-gson**</dt><dd>[DEFAULT] HTTP client: OkHttp 3.x. JSON processing: Gson 2.8.x. Enable Parcelable models on Android using '-DparcelableModel=true'. Enable gzip request encoding using '-DuseGzipFeature=true'.</dd><dt>**retrofit**</dt><dd>HTTP client: OkHttp 2.x. JSON processing: Gson 2.x (Retrofit 1.9.0). IMPORTANT NOTE: retrofit1.x is no longer actively maintained so please upgrade to 'retrofit2' instead.</dd><dt>**retrofit2**</dt><dd>HTTP client: OkHttp 3.x. JSON processing: Gson 2.x (Retrofit 2.3.0). Enable the RxJava adapter using '-DuseRxJava[2]=true'. (RxJava 1.x or 2.x)</dd><dt>**resttemplate**</dt><dd>HTTP client: Spring RestTemplate 4.x. JSON processing: Jackson 2.8.x</dd><dt>**webclient**</dt><dd>HTTP client: Spring WebClient 5.x. JSON processing: Jackson 2.9.x</dd><dt>**resteasy**</dt><dd>HTTP client: Resteasy client 3.x. JSON processing: Jackson 2.8.x</dd><dt>**vertx**</dt><dd>HTTP client: VertX client 3.x. JSON processing: Jackson 2.8.x</dd><dt>**google-api-client**</dt><dd>HTTP client: Google API client 1.x. JSON processing: Jackson 2.8.x</dd><dt>**rest-assured**</dt><dd>HTTP client: rest-assured : 3.x. JSON processing: Gson 2.x. Only for Java8</dd><dl>|okhttp-gson|
+|library|library template (sub-template) to use|<dl><dt>**jersey1**</dt><dd>HTTP client: Jersey client 1.19.x. JSON processing: Jackson 2.8.x. Enable Java6 support using '-DsupportJava6=true'. Enable gzip request encoding using '-DuseGzipFeature=true'. IMPORTANT NOTE: jersey 1.x is no longer actively maintained so please upgrade to 'jersey2' or other HTTP libaries instead.</dd><dt>**jersey2**</dt><dd>HTTP client: Jersey client 2.25.1. JSON processing: Jackson 2.8.x</dd><dt>**feign**</dt><dd>HTTP client: OpenFeign 9.x or 10.x. JSON processing: Jackson 2.8.x. To enable OpenFeign 10.x, set the 'feignVersion' option to '10.x'</dd><dt>**okhttp-gson**</dt><dd>[DEFAULT] HTTP client: OkHttp 3.x. JSON processing: Gson 2.8.x. Enable Parcelable models on Android using '-DparcelableModel=true'. Enable gzip request encoding using '-DuseGzipFeature=true'.</dd><dt>**retrofit**</dt><dd>HTTP client: OkHttp 2.x. JSON processing: Gson 2.x (Retrofit 1.9.0). IMPORTANT NOTE: retrofit1.x is no longer actively maintained so please upgrade to 'retrofit2' instead.</dd><dt>**retrofit2**</dt><dd>HTTP client: OkHttp 3.x. JSON processing: Gson 2.x (Retrofit 2.3.0). Enable the RxJava adapter using '-DuseRxJava[2]=true'. (RxJava 1.x or 2.x)</dd><dt>**resttemplate**</dt><dd>HTTP client: Spring RestTemplate 4.x. JSON processing: Jackson 2.8.x</dd><dt>**webclient**</dt><dd>HTTP client: Spring WebClient 5.x. JSON processing: Jackson 2.9.x</dd><dt>**resteasy**</dt><dd>HTTP client: Resteasy client 3.x. JSON processing: Jackson 2.8.x</dd><dt>**vertx**</dt><dd>HTTP client: VertX client 3.x. JSON processing: Jackson 2.8.x</dd><dt>**google-api-client**</dt><dd>HTTP client: Google API client 1.x. JSON processing: Jackson 2.8.x</dd><dt>**rest-assured**</dt><dd>HTTP client: rest-assured : 4.x. JSON processing: Gson 2.x. Only for Java8</dd><dl>|okhttp-gson|
Perform git status
On branch pull/3306
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

	modified:   docs/generators/java.md

no changes added to commit (use "git add" and/or "git commit -a")
Please run 'bin/utils/ensure-up-to-date' locally and commit changes (UNCOMMITTED CHANGES ERROR)
Exited with code 1

I don't think the error is related to my change

@muttleyxd muttleyxd force-pushed the pistache-compatibility-json-35 branch from 4ef1553 to 184b7ca Compare July 11, 2019 06:35
@wing328
Copy link
Member

wing328 commented Jul 12, 2019

@muttleyxd next time you can restart the CI by closing and then reopening the PR.

@stkrwork stkrwork merged commit c1df082 into OpenAPITools:master Jul 13, 2019
@wing328 wing328 added this to the 4.1.0 milestone Aug 9, 2019
@wing328
Copy link
Member

wing328 commented Aug 10, 2019

@muttleyxd thanks for the PR, which has been included in the 4.1.0 release: https://twitter.com/oas_generator/status/1160000504455319553

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants