Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 15 additions & 9 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
# To get started with Dependabot version updates, you'll need to specify which
# package ecosystems to update and where the package manifests are located.
# Please see the documentation for all configuration options:
# https://help.github.com/github/administering-a-repository/configuration-options-for-dependency-updates
# https://docs.github.com/en/code-security/dependabot/working-with-dependabot/dependabot-options-reference

version: 2
updates:
- package-ecosystem: "cargo" # See documentation for possible values
directory: "/generator" # Location of package manifests
- package-ecosystem: "cargo"
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer to move these changes to a separate PR to consider them independently.

Copy link
Author

Choose a reason for hiding this comment

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

They're in a separate commit. I can send a separate PR.

directory: "/generator"
schedule:
interval: "daily"
- package-ecosystem: "github-actions" # See documentation for possible values
directory: "/" # Location of package manifests
interval: "weekly"
groups:
cargo-crates:
patterns:
- "*"
- package-ecosystem: "github-actions"
directory: "/"
schedule:
interval: "daily"
interval: "weekly"
groups:
github-actions:
patterns:
- "*"
2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,5 @@ members = [
"tripactions",
"zoom"
]

resolver = "2"
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -574,4 +574,4 @@ zoom: target/debug/generator $(ZOOM_SPEC)

.PHONY: README.md
README.md: ## Cleans client info in README.md.
@sed -i '/## Clients Generated/q' $@
@sed -i'' '/## Clients Generated/q' $@
6 changes: 3 additions & 3 deletions generator/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ use inflector::cases::{pascalcase::to_pascal_case, snakecase::to_snake_case};

use crate::{
clean_fn_name, clean_name, client::generate_servers, get_parameter_data, make_plural,
oid_to_object_name, path_to_operation_id, struct_name, template::parse, ExtractJsonMediaType,
ParameterDataExt, ReferenceOrExt, TypeId, TypeSpace,
oid_to_object_name, path_to_operation_id, struct_name, template::parse, ParameterDataExt,
ReferenceOrExt, TypeId, TypeSpace,
};

#[derive(Debug, Default)]
Expand Down Expand Up @@ -151,7 +151,7 @@ pub fn generate_files(

let (body_param, body_func) = if let Some(b) = &o.request_body {
if let Ok(b) = b.item() {
if b.is_binary()? {
if crate::is_binary(b)? {
Copy link
Contributor

@augustuswm augustuswm Apr 10, 2025

Choose a reason for hiding this comment

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

What is the measurable value to moving this to a free function?

Copy link
Author

Choose a reason for hiding this comment

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

It's explained in the commit message, and it's very clear if you look at the commit by itself: a78ba1a?w=1

it removes one copy of a function.

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand the change, I was more asking if this has a measurable impact on compilation or runtime performance? I was not able to see it initially on my machine, but that is only one data point.

Copy link
Author

Choose a reason for hiding this comment

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

I didn't perform science. I saw duplicated code and removed it.

let (ct, _) = b.content.first().unwrap();
body_content_type_header = Some(ct.to_string());

Expand Down
181 changes: 48 additions & 133 deletions generator/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,157 +380,72 @@ impl ParameterDataExt for openapiv3::ParameterData {
}
}

trait ExtractJsonMediaType {
fn is_binary(&self) -> Result<bool>;
fn content_json(&self) -> Result<openapiv3::MediaType>;
trait HasContent {
fn get_content(&self, key: &str) -> Option<&openapiv3::MediaType>;
}

impl ExtractJsonMediaType for openapiv3::Response {
fn content_json(&self) -> Result<openapiv3::MediaType> {
// We do not need to check the length of the content because there might be
// more than one. For example, if xml or some other format is also defined.
if let Some(mt) = self.content.get("application/json") {
Ok(mt.clone())
} else {
bail!(
"could not find application/json, only found {}",
self.content.keys().next().unwrap()
);
}
}

fn is_binary(&self) -> Result<bool> {
if self.content.is_empty() {
/*
* XXX If there are no content types, I guess it is not binary?
*/
return Ok(false);
}

// We do not need to check the length of the content because there might be
// more than one. For example, if xml or some other format is also defined.
if let Some(mt) = self.content.get("application/octet-stream") {
if !mt.encoding.is_empty() {
bail!("XXX encoding");
}

if let Some(s) = &mt.schema {
use openapiv3::{SchemaKind, StringFormat, Type, VariantOrUnknownOrEmpty::Item};

if let Ok(s) = s.item() {
if s.schema_data.nullable {
bail!("XXX nullable binary?");
}
if s.schema_data.default.is_some() {
bail!("XXX default binary?");
}
if s.schema_data.discriminator.is_some() {
bail!("XXX binary discriminator?");
}
match &s.schema_kind {
SchemaKind::Type(Type::String(st)) => {
if st.min_length.is_some() || st.max_length.is_some() {
bail!("binary min/max length");
}
if !matches!(st.format, Item(StringFormat::Binary)) {
bail!("expected binary format string, got {:?}", st.format);
}
if st.pattern.is_some() {
bail!("XXX pattern");
}
if !st.enumeration.is_empty() {
bail!("XXX binary enumeration {:?}", st);
}
return Ok(true);
}
x => {
bail!("XXX schemakind type {:?}", x);
}
}
} else {
return Ok(false);
}
} else {
bail!("binary thing had no schema?");
}
}

Ok(false)
impl HasContent for openapiv3::Response {
fn get_content(&self, key: &str) -> Option<&openapiv3::MediaType> {
self.content.get(key)
}
}

impl ExtractJsonMediaType for openapiv3::RequestBody {
fn content_json(&self) -> Result<openapiv3::MediaType> {
// We do not need to check the length of the content because there might be
// more than one. For example, if xml or some other format is also defined.
if let Some(mt) = self.content.get("application/json") {
Ok(mt.clone())
} else {
bail!(
"could not find application/json, only found {}",
self.content.keys().next().unwrap()
);
}
impl HasContent for openapiv3::RequestBody {
fn get_content(&self, key: &str) -> Option<&openapiv3::MediaType> {
self.content.get(key)
}
}

fn is_binary(&self) -> Result<bool> {
if self.content.is_empty() {
/*
* XXX If there are no content types, I guess it is not binary?
*/
return Ok(false);
fn is_binary(this: &impl HasContent) -> Result<bool> {
// We do not need to check the length of the content because there might be
// more than one. For example, if xml or some other format is also defined.
if let Some(mt) = this.get_content("application/octet-stream") {
if !mt.encoding.is_empty() {
bail!("XXX encoding");
}

// We do not need to check the length of the content because there might be
// more than one. For example, if xml or some other format is also defined.
if let Some(mt) = self.content.get("application/octet-stream") {
if !mt.encoding.is_empty() {
bail!("XXX encoding");
}

if let Some(s) = &mt.schema {
use openapiv3::{SchemaKind, StringFormat, Type, VariantOrUnknownOrEmpty::Item};
if let Some(s) = &mt.schema {
use openapiv3::{SchemaKind, StringFormat, Type, VariantOrUnknownOrEmpty::Item};

if let Ok(s) = s.item() {
if s.schema_data.nullable {
bail!("XXX nullable binary?");
}
if s.schema_data.default.is_some() {
bail!("XXX default binary?");
}
if s.schema_data.discriminator.is_some() {
bail!("XXX binary discriminator?");
}
match &s.schema_kind {
SchemaKind::Type(Type::String(st)) => {
if st.min_length.is_some() || st.max_length.is_some() {
bail!("binary min/max length");
}
if !matches!(st.format, Item(StringFormat::Binary)) {
bail!("expected binary format string, got {:?}", st.format);
}
if st.pattern.is_some() {
bail!("XXX pattern");
}
if !st.enumeration.is_empty() {
bail!("XXX enumeration");
}
return Ok(true);
if let Ok(s) = s.item() {
if s.schema_data.nullable {
bail!("XXX nullable binary?");
}
if s.schema_data.default.is_some() {
bail!("XXX default binary?");
}
if s.schema_data.discriminator.is_some() {
bail!("XXX binary discriminator?");
}
match &s.schema_kind {
SchemaKind::Type(Type::String(st)) => {
if st.min_length.is_some() || st.max_length.is_some() {
bail!("binary min/max length");
}
if !matches!(st.format, Item(StringFormat::Binary)) {
bail!("expected binary format string, got {:?}", st.format);
}
x => {
bail!("XXX schemakind type {:?}", x);
if st.pattern.is_some() {
bail!("XXX pattern");
}
if !st.enumeration.is_empty() {
bail!("XXX binary enumeration {:?}", st);
}
return Ok(true);
}
x => {
bail!("XXX schemakind type {:?}", x);
}
} else {
return Ok(false);
}
} else {
bail!("binary thing had no schema?");
return Ok(false);
}
} else {
bail!("binary thing had no schema?");
}

Ok(false)
}

Ok(false)
}

trait ReferenceOrExt<T> {
Expand Down