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

Generate clients from LINE OpenAPI #296

Closed
wants to merge 14 commits into from
Closed

Conversation

zenizh
Copy link
Member

@zenizh zenizh commented Jul 10, 2023

I generated clients according to the definition of LINE OpenAPI using OpenAPI Generator. I also set up the workflow for GitHub Actions and other related tasks.

Policy of change

This PR uses the OpenAPI Generator to build clients. The OpenAPI Generator generates clients as gems, which means it generates as many gems as there are clients.

On the other hand, we want a single gem, line-bot-api. So we ignore the auto-generated files that we need as a gem with .gitignore.

This makes it work as a single gem by reading each client entry point from lib/line/client.rb.

I will also add a script called bin/generate-clients. This helps to generate clients in the local environment and is also used to generate clients on GitHub Actions.

Change details

The main files changed in this PR are:

File Overview
bin/generate-clients Script to generate clients
lib/clients List of generated clients
lib/line/client.rb Load the client list
.github/workflows/generate-clients.yml Generate client on GitHub Actions -> Set up PR
spec Minimal testing to ensure there are no dependency issues
src/main/resources/ruby-client/README.mustache Override the generated client README
.gitignore Define unnecessary files for generated clients
Gemfile Remove unnecessary dependencies
README.md Rewritten for this change
line-bot-spi.gemspec Organizing dependencies

How to review

Due to the large number of files in the difference, it is easier to review the PR source branch:
https://github.com/zenizh/line-bot-sdk-ruby/tree/openapi


This pull request resolves #292, resolves #283, resolves #282, resolves #255, resolves #252, resolves #251, resolves #244, resolves #232, resolves #191.

This was linked to issues Jul 10, 2023
@zenizh
Copy link
Member Author

zenizh commented Jul 13, 2023

To maintain compatibility with previous versions, I have made the following commit. This allows methods from previous versions to be used without modification, while still displaying the warning message.
c8b141c

Comment on lines -10 to -11
def client
@client ||= Line::Bot::Client.new do |config|
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this pull request deletes examples/** files? I think it would help line-bot-sdk-ruby beginners.

@Yang-33 Yang-33 requested a review from kimoto July 18, 2023 07:37
@Yang-33 Yang-33 assigned Yang-33 and zenizh and unassigned Yang-33 Jul 18, 2023
@kimoto kimoto requested review from toduq and gom July 24, 2023 05:14
@tokuhirom
Copy link
Member

validate_signature method should be implemented in the latest version.

@tokuhirom
Copy link
Member

I guess, generated codes in lib/clients/webhook doesn't support deserializing webhook's payload.
req = LINE::Client::Webhook::CalbackRequest.new(JSON.parse(request_body)) won't parse Hash object to nested object.
I mean, req is instance of LINE::Client::Webhook::CalbackRequest. But req.events[0] is just Hash, not an instance of the LINE::Client::Webhook::Event.

It's suprising.

@zenizh
Copy link
Member Author

zenizh commented Sep 22, 2023

I added the following code:

@tokuhirom
Copy link
Member

https://github.com/zenizh/line-bot-sdk-ruby/blob/openapi/lib/clients/channel-access-token/lib/line_client_channel_access_token/api/channel_access_token_api.rb#L237-L283

is bit strange.

This endpoint has two functionalities: 'Issue from channel ID and channel secret' and 'Issue from JWT assertion.'

For 'Issue from channel ID and channel secret,' you need to make the request with 'grant_type,' 'client_id,' and 'client_secret.'

For 'Issue from JWT assertion,' you need to make the request with 'grant_type,' 'client_assertion_type,' and 'client_assertion.'

The OpenAPI definition specifies that one of these requests is required: link to the OpenAPI definition.

However, in this PR, all the required parameters for both patterns are listed, and there are a few issues:

  1. It's defined as [String] in the RDoc, which means nil cannot be passed.
  2. Due to client-side validation like here, you are required to provide valid values for parameters even if you are not using them in one of the patterns."

Implemented KitchenSink example based on openapi
@zenizh
Copy link
Member Author

zenizh commented Mar 5, 2024

This pull request will be submitted as a new pull request, so it is closed for now.

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