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++][Restbed] Add handler callback methods #2911

Merged
merged 2 commits into from
May 30, 2019

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.
    Added second commit with updated samples (not 100% sure what I should do with them), are there any tests for these? I didn't find any.
  • 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 @fvarose @etherealjoy @MartinDelille

Description of the PR

Related issue: #273
What I don't like about current generator: you need to modify generated code, there isn't a way for injecting your own handler. librestbed does not allow to replace already existing handler, which means you either modify generated code or don't use it at all. I modified it a little, to allow setting a callback, which responds with std::pair<int (HTTP status code), std::string (response)>

I'm not an expert on Mustache, so I would be really grateful for some tips where I could optimize/refactor it.

@muttleyxd muttleyxd force-pushed the restbed-callback branch 4 times, most recently from b3d3c09 to 6829e11 Compare May 16, 2019 09:29
@etherealjoy
Copy link
Contributor

A generic comment here, Callback is also an OpenApi 3 keyword, won't this be confusing?

@muttleyxd
Copy link
Contributor Author

A generic comment here, Callback is also an OpenApi 3 keyword, won't this be confusing?

Definitely a wrong name then (I didn't know this feature before). Should I change 'callback' to 'handler'?

@etherealjoy
Copy link
Contributor

I think handler would be a better name

@muttleyxd muttleyxd changed the title [C++][Restbed] Add callback methods [C++][Restbed] Add handler callback methods May 29, 2019
@muttleyxd
Copy link
Contributor Author

I think handler would be a better name

Done

@stkrwork stkrwork requested a review from etherealjoy May 29, 2019 18:38
@stkrwork stkrwork merged commit 87adba9 into OpenAPITools:master May 30, 2019
@muttleyxd muttleyxd deleted the restbed-callback branch May 30, 2019 18:25
@wing328 wing328 added this to the 4.0.1 milestone May 31, 2019
@wing328
Copy link
Member

wing328 commented Jun 3, 2019

@muttleyxd thanks again for your contribution, which has been included in the v4.0.1 release (https://twitter.com/oas_generator/status/1135534738658062336)

jimschubert added a commit to jimschubert/openapi-generator that referenced this pull request Jun 5, 2019
* 4.1.x: (56 commits)
  sync master
  Update compatibility table
  Ruby client: escape path parameters (OpenAPITools#3039)
  [gradle plugin] Release 4.0.1 fixes (OpenAPITools#3051)
  Update version to 4.0.2-SNAPSHOT (OpenAPITools#3047)
  Map number to double time since float is also parsed as double in Qt5 C++ (OpenAPITools#3046)
  Prepare 4.0.1 release (OpenAPITools#3041)
  [gradle] Reworking publishing pipeline (OpenAPITools#2886)
  [typescript-fetch] Fix uploading files (OpenAPITools#2900)
  Resolves OpenAPITools#2962 - Add properties config to Maven parameters (OpenAPITools#2963)
  fix(golang): Check error of xml Encode (OpenAPITools#3027)
  [C++][Restbed] Add handler callback methods (OpenAPITools#2911)
  Remove null checks for C# value types (OpenAPITools#2933)
  [python-server] Support python 3.7 for all server-generators (OpenAPITools#2884)
  Use golang's provided method names (gin) (OpenAPITools#2983)
  [python] Adding constructor parameters to Configuration and improving documentation (OpenAPITools#3002)
  Add new option to maven plugin's readme (OpenAPITools#3025)
  Engine param in maven plugin. (OpenAPITools#2881)
  Added support for patterns on model properties (OpenAPITools#2948)
  [csharp] Make API response headers dictionary case insensitive (OpenAPITools#2998)
  ...
@engpedrorafael
Copy link

I appreciate and agree that this is a very important feature, though I don't see how to make use of the set_handler_ GET/POST/DELETE without changing the generated code.
The methods set_handler_* are generated as member functions of the *Resource classes which can only be called through an object instantiated from the respective class.
From the generated code, we can see that at the constructor of the API class is where the *Resources are created as local objects and passed to the Base Class using the method publish:

PetApi::PetApi() {
	std::shared_ptr<PetApiPetResource> spPetApiPetResource = std::make_shared<PetApiPetResource>();
	this->publish(spPetApiPetResource);	
	...

I don't see a way of accessing the Resource objects pubished after the creation of the object, in order to call the respective set_handler_ to set my custom handlers, and this could not have been done before since we cannot call it without an object.

Can you please explain, maybe with a small example, how to make use of this in order to set custom handlers

@muttleyxd
Copy link
Contributor Author

Yeah, I think I'd probably need to change generated constructor or add another method for setting handlers.
Maybe virtual dispatch would be good here, what do you think about it? So one would have to do

class Derived : public PetApi
{
public:
  <type> PetApiPetResource_GET(...) override;
};

For now I usually create new PetApi class and and use the resources myself, something like that

class MyPetApi
{
public:
  MyPetApi() {
    std::shared_ptr<PetApiPetResource> spPetApiPetResource = std::make_shared<PetApiPetResource>();
    spPetApiPetResource.set_handler_PUT([&](auto const& body) { return handlePetPut(body); })
	this->publish(spPetApiPetResource);
  }

private:
  std::pair<int, std::string> handlePetPut(std::string const& body) {
    // Do something here
    return {200, "Success"};
  }
};

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.

5 participants