Skip to content

Commit b3ed18f

Browse files
committed
refactor: addressing pr comments
1 parent ab984e4 commit b3ed18f

File tree

174 files changed

+2240
-1151
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

174 files changed

+2240
-1151
lines changed

.github/actions/setup-api-tools/action.yml

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
name: 'Setup API Tools'
22
description: 'Setup Rust, Bun, and Python dependencies for API tools'
33

4+
inputs:
5+
setup_algokit:
6+
description: 'Whether to install and start algokit localnet'
7+
required: false
8+
default: 'false'
9+
410
runs:
511
using: 'composite'
612
steps:
@@ -24,4 +30,14 @@ runs:
2430
- name: Install uv dependencies
2531
working-directory: api/oas_generator
2632
shell: bash
27-
run: uv sync --extra dev
33+
run: uv sync --extra dev
34+
35+
- name: Optionally install algokit
36+
if: ${{ inputs.setup_algokit == 'true' }}
37+
shell: bash
38+
run: uv tool install algokit
39+
40+
- name: Optionally start localnet
41+
if: ${{ inputs.setup_algokit == 'true' }}
42+
shell: bash
43+
run: algokit localnet start

.github/workflows/api_ci.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,8 @@ jobs:
8181
steps:
8282
- uses: actions/checkout@v4
8383
- uses: ./.github/actions/setup-api-tools
84+
with:
85+
setup_algokit: 'true'
8486
- uses: actions/setup-node@v4
8587
with:
8688
node-version: "20"
@@ -108,3 +110,7 @@ jobs:
108110
- name: Build (tsc) ${{ matrix.pkg }}
109111
working-directory: packages/typescript/${{ matrix.pkg }}
110112
run: bun run build
113+
114+
- name: Test (${{ matrix.pkg }})
115+
working-directory: packages/typescript/${{ matrix.pkg }}
116+
run: bun test

api/oas_generator/ts_oas_generator/cli.py

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -30,24 +30,17 @@ def parse_command_line_args(args: list[str] | None = None) -> argparse.Namespace
3030
formatter_class=argparse.RawDescriptionHelpFormatter,
3131
epilog="""
3232
Examples:
33-
%(prog)s --runtime-only --output ./packages/algod_client --package-name algod_client
3433
%(prog)s ../specs/algod.oas3.json --output ./packages/algod_client --package-name algod_client
3534
%(prog)s ../specs/indexer.oas3.json -o ./packages/indexer_client -p indexer_client
3635
""",
3736
)
3837

3938
parser.add_argument(
4039
"spec_file",
41-
nargs="?",
4240
type=Path,
4341
help="Path to OpenAPI specification file (JSON or YAML)",
4442
metavar="SPEC_FILE",
4543
)
46-
parser.add_argument(
47-
"--runtime-only",
48-
action="store_true",
49-
help="Generate only the base runtime files (no models/APIs); does not require a spec file",
50-
)
5144
parser.add_argument(
5245
"--output",
5346
"-o",
@@ -86,10 +79,7 @@ def parse_command_line_args(args: list[str] | None = None) -> argparse.Namespace
8679
parsed_args = parser.parse_args(args)
8780

8881
# Validate inputs
89-
if not parsed_args.runtime_only and parsed_args.spec_file is None:
90-
parser.error("SPEC_FILE is required unless --runtime-only is provided")
91-
92-
if parsed_args.spec_file is not None and not parsed_args.spec_file.exists():
82+
if not parsed_args.spec_file.exists():
9383
parser.error(f"Specification file not found: {parsed_args.spec_file}")
9484

9585
return parsed_args
@@ -141,19 +131,12 @@ def main(args: list[str] | None = None) -> int:
141131
with backup_and_prepare_output_dir(parsed_args.output_dir):
142132
generator = TsCodeGenerator(template_dir=parsed_args.template_dir)
143133

144-
if parsed_args.runtime_only:
145-
generated_files = generator.generate_runtime(
146-
parsed_args.output_dir,
147-
parsed_args.package_name,
148-
custom_description=parsed_args.custom_description,
149-
)
150-
else:
151-
generated_files = generator.generate_full(
152-
parsed_args.spec_file,
153-
parsed_args.output_dir,
154-
parsed_args.package_name,
155-
custom_description=parsed_args.custom_description,
156-
)
134+
generated_files = generator.generate_full(
135+
parsed_args.spec_file,
136+
parsed_args.output_dir,
137+
parsed_args.package_name,
138+
custom_description=parsed_args.custom_description,
139+
)
157140

158141
# Write files to disk (overwrite safely)
159142
write_files_to_disk(generated_files)

api/oas_generator/ts_oas_generator/generator/filters.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,8 @@ def _inline_object(schema: dict[str, Any], schemas: dict[str, Any] | None) -> st
105105
parts: list[str] = []
106106

107107
for prop_name, prop_schema in properties.items():
108-
ts_name = ts_property_name(prop_name)
108+
# Generate camelCase TS property names for better DX
109+
ts_name = ts_camel_case(prop_name)
109110
ts_t = ts_type(prop_schema, schemas)
110111
opt = "" if prop_name in required else "?"
111112
parts.append(f"{ts_name}{opt}: {ts_t};")
@@ -121,12 +122,12 @@ def _inline_object(schema: dict[str, Any], schemas: dict[str, Any] | None) -> st
121122
return "{" + (" ".join(parts)) + "}"
122123

123124

124-
def _map_primitive(schema_type: str, _schema_format: str | None, schema: dict[str, Any]) -> str:
125-
if schema.get("x-algokit-bigint") is True and schema_type == "integer":
126-
return "bigint"
127-
125+
def _map_primitive(schema_type: str, _schema_format: str | None, _schema: dict[str, Any]) -> str:
128126
if schema_type == "integer":
129-
return "number"
127+
# Default to bigint for blockchain-sized integers; keep int32 as number
128+
if _schema_format == "int32":
129+
return "number"
130+
return "bigint"
130131

131132
if schema_type in ("number",):
132133
return "number"

api/oas_generator/ts_oas_generator/generator/template_engine.py

Lines changed: 58 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,23 @@ def generate_runtime(
7171
"""Generate runtime files under the provided output directory."""
7272
output_dir = Path(output_dir)
7373
src_core = output_dir / "src" / "core"
74+
base_name = ts_pascal_case(package_name)
75+
base_core = base_name[:-6] if base_name.lower().endswith("client") else base_name
76+
client_class_name = f"{base_core}Client"
77+
service_class_name = f"{base_core}Api"
78+
7479
context = {
7580
"package_name": package_name,
7681
"custom_description": custom_description,
82+
"client_class_name": client_class_name,
83+
"service_class_name": service_class_name,
7784
}
7885

7986
files: dict[Path, str] = {}
8087
# Core runtime files
81-
files[src_core / "OpenAPI.ts"] = self.template_engine.render_template("base/src/core/OpenAPI.ts.j2", context)
88+
files[src_core / "ClientConfig.ts"] = self.template_engine.render_template(
89+
"base/src/core/ClientConfig.ts.j2", context
90+
)
8291
files[src_core / "BaseHttpRequest.ts"] = self.template_engine.render_template(
8392
"base/src/core/BaseHttpRequest.ts.j2", context
8493
)
@@ -92,6 +101,7 @@ def generate_runtime(
92101
)
93102
files[src_core / "json.ts"] = self.template_engine.render_template("base/src/core/json.ts.j2", context)
94103
files[src_core / "msgpack.ts"] = self.template_engine.render_template("base/src/core/msgpack.ts.j2", context)
104+
files[src_core / "casing.ts"] = self.template_engine.render_template("base/src/core/casing.ts.j2", context)
95105

96106
# Index barrel (runtime-only)
97107
files[output_dir / "src" / "index.ts"] = self.template_engine.render_template("base/src/index.ts.j2", context)
@@ -222,8 +232,8 @@ def collect_model_types(type_str: str) -> set[str]:
222232
param = node
223233
schema = param.get("schema", {}) or {}
224234
t = ts_type(schema, schemas)
225-
# BigInt params accept number | bigint
226-
if schema.get("x-algokit-bigint") is True and t == "bigint":
235+
# When a parameter resolves to bigint, accept number | bigint for ergonomics
236+
if t == "bigint":
227237
t_sig = "number | bigint"
228238
stringify_bigint = True
229239
else:
@@ -256,23 +266,44 @@ def collect_model_types(type_str: str) -> set[str]:
256266
}
257267
)
258268

259-
# Request body (pick json if present, else msgpack/text)
269+
# Request body handling
260270
request_body_ctx: dict[str, Any] | None = None
271+
request_body_supports_msgpack = False
272+
request_body_supports_json = False
261273
rb = op.get("requestBody")
262274
if isinstance(rb, dict):
263275
content = rb.get("content", {})
276+
277+
# Check what content types are supported
278+
if "application/msgpack" in content:
279+
request_body_supports_msgpack = True
280+
if "application/json" in content:
281+
request_body_supports_json = True
282+
283+
# Determine the type to use for TypeScript typing
284+
# Prefer JSON schema for typing even when msgpack is used at runtime
264285
if "application/json" in content:
265286
sch = (content["application/json"] or {}).get("schema", {})
266287
request_body_ctx = {
267-
"mediaType": "application/json",
288+
"mediaType": (
289+
"application/msgpack"
290+
if request_body_supports_msgpack and not request_body_supports_json
291+
else "application/json"
292+
),
268293
"tsType": ts_type(sch, schemas),
269294
"required": rb.get("required", False),
295+
"supportsMsgpack": request_body_supports_msgpack,
296+
"supportsJson": request_body_supports_json,
270297
}
271298
elif "application/msgpack" in content:
299+
# msgpack-only endpoint - infer type from msgpack schema if available
300+
sch = (content["application/msgpack"] or {}).get("schema", {})
272301
request_body_ctx = {
273302
"mediaType": "application/msgpack",
274-
"tsType": "Uint8Array",
303+
"tsType": ts_type(sch, schemas) if sch else "any",
275304
"required": rb.get("required", False),
305+
"supportsMsgpack": True,
306+
"supportsJson": False,
276307
}
277308
elif "application/x-binary" in content or "application/octet-stream" in content:
278309
# Raw binary transaction submission etc.
@@ -281,13 +312,17 @@ def collect_model_types(type_str: str) -> set[str]:
281312
"mediaType": mt,
282313
"tsType": "Uint8Array",
283314
"required": rb.get("required", False),
315+
"supportsMsgpack": False,
316+
"supportsJson": False,
284317
}
285318
elif "text/plain" in content:
286319
sch = (content["text/plain"] or {}).get("schema", {})
287320
request_body_ctx = {
288321
"mediaType": "text/plain",
289322
"tsType": ts_type(sch, schemas),
290323
"required": rb.get("required", False),
324+
"supportsMsgpack": False,
325+
"supportsJson": False,
291326
}
292327

293328
# Responses
@@ -387,6 +422,8 @@ def collect_model_types(type_str: str) -> set[str]:
387422
"acceptsMsgpack": returns_msgpack,
388423
"returnsMsgpack": returns_msgpack,
389424
"supportsJson": supports_json,
425+
"requestBodySupportsMsgpack": request_body_supports_msgpack,
426+
"requestBodySupportsJson": request_body_supports_json,
390427
"hasFormatParam": has_format_param,
391428
"formatVarName": format_var_name,
392429
"signature": ", ".join(sig_parts),
@@ -465,21 +502,32 @@ def generate_full(
465502
for t in op.get("importTypes", []):
466503
import_types.add(t)
467504

505+
# Determine class names from package name (e.g., algod_client -> AlgodClient, AlgodApi)
506+
base_name = ts_pascal_case(package_name)
507+
base_core = base_name[:-6] if base_name.lower().endswith("client") else base_name
508+
client_class_name = f"{base_core}Client"
509+
service_class_name = f"{base_core}Api"
510+
468511
svc_content = self.template_engine.render_template(
469512
"apis/service.ts.j2",
470-
{"tag_name": "api", "operations": all_ops, "import_types": sorted(import_types)},
513+
{
514+
"tag_name": "api",
515+
"operations": all_ops,
516+
"import_types": sorted(import_types),
517+
"service_class_name": service_class_name,
518+
},
471519
)
472520
files[apis_dir / "api.service.ts"] = svc_content
473521

474522
files[apis_dir / "index.ts"] = self.template_engine.render_template(
475523
"apis/index.ts.j2",
476-
{},
524+
{"service_class_name": service_class_name},
477525
)
478526

479527
# Client (single service)
480528
files[Path(output_dir) / "src" / "client.ts"] = self.template_engine.render_template(
481529
"client.ts.j2",
482-
{},
530+
{"service_class_name": service_class_name, "client_class_name": client_class_name},
483531
)
484532

485533
# Replace index with full barrel
@@ -488,12 +536,6 @@ def generate_full(
488536
{},
489537
)
490538

491-
# Smoke tests (generated)
492-
files[Path(output_dir) / "tests" / "generated" / "smoke.generated.spec.ts"] = (
493-
self.template_engine.render_template(
494-
"full/tests/smoke.spec.ts.j2",
495-
{},
496-
)
497-
)
539+
# Note: Generated smoke tests removed; prefer manual integration tests maintained by developers
498540

499541
return files
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
// Barrel file for services
2-
export { ApiService } from './api.service';
2+
export { {{ service_class_name }} } from './api.service';

api/oas_generator/ts_oas_generator/templates/apis/service.ts.j2

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { BaseHttpRequest, ApiRequestOptions } from '../core/BaseHttpRequest
33
import type { {{ import_types | sort | join(', ') }} } from '../models/index';
44
{% endif %}
55

6-
export class ApiService {
6+
export class {{ service_class_name }} {
77
constructor(public readonly httpRequest: BaseHttpRequest) {}
88

99
{% for op in operations %}
@@ -25,16 +25,15 @@ export class ApiService {
2525
): Promise<{{ op.responseTsType }}> {
2626
const headers: Record<string, string> = {};
2727
{% if op.acceptsMsgpack %}
28-
// Content negotiation:
29-
// - If explicit format=json, prefer JSON
30-
// - Else if server supports JSON, prefer JSON by default
31-
// - Else fall back to msgpack
28+
// Content negotiation (aligned with Rust behavior):
29+
// - Default to msgpack when available (better performance, smaller payload)
30+
// - Only use JSON if explicitly requested via format=json
3231
{% if op.hasFormatParam and op.formatVarName %}
33-
const hasExplicitJson = params?.{{ op.formatVarName }} === 'json';
34-
const prefersJson = hasExplicitJson || {{ 'true' if op.supportsJson else 'false' }};
35-
headers['Accept'] = prefersJson ? 'application/json' : 'application/msgpack';
32+
const useJson = params?.{{ op.formatVarName }} === 'json';
33+
headers['Accept'] = useJson ? 'application/json' : 'application/msgpack';
3634
{% else %}
37-
headers['Accept'] = {{ "'application/json'" if op.supportsJson else "'application/msgpack'" }};
35+
// Prefer msgpack when available, fallback to JSON
36+
headers['Accept'] = 'application/msgpack';
3837
{% endif %}
3938
{% else %}
4039
headers['Accept'] = 'application/json';
@@ -65,12 +64,27 @@ export class ApiService {
6564
},
6665
headers,
6766
body: {% if op.requestBody %}params?.body{% else %}undefined{% endif %},
68-
mediaType: {% if op.requestBody %}'{{ op.requestBody['mediaType'] }}'{% else %}undefined{% endif %},
67+
{% if op.requestBody %}
68+
{% if op.requestBodySupportsMsgpack and op.requestBodySupportsJson and op.hasFormatParam and op.formatVarName %}
69+
// Dynamic mediaType based on format parameter (prefer msgpack by default)
70+
mediaType: params?.{{ op.formatVarName }} === 'json' ? 'application/json' : 'application/msgpack',
71+
{% elif op.requestBodySupportsMsgpack and not op.requestBodySupportsJson %}
72+
// Only msgpack supported for request body
73+
mediaType: 'application/msgpack',
74+
{% elif op.requestBodySupportsMsgpack and op.requestBodySupportsJson %}
75+
// Both supported, prefer msgpack for better performance
76+
mediaType: 'application/msgpack',
77+
{% else %}
78+
mediaType: '{{ op.requestBody['mediaType'] }}',
79+
{% endif %}
80+
{% else %}
81+
mediaType: undefined,
82+
{% endif %}
6983
{% if op.returnsMsgpack %}
7084
{% if op.hasFormatParam and op.formatVarName %}
71-
expectBinary: params?.{{ op.formatVarName }} === 'json' ? false : {{ 'false' if op.supportsJson else 'true' }},
85+
expectBinary: params?.{{ op.formatVarName }} === 'json' ? false : true,
7286
{% else %}
73-
expectBinary: {{ 'false' if op.supportsJson else 'true' }},
87+
expectBinary: true,
7488
{% endif %}
7589
{% else %}
7690
expectBinary: false,

0 commit comments

Comments
 (0)