Carapacik / swagger_parser

Dart package that takes an OpenApi definition file and generates REST clients based on retrofit and data classes for your project.
https://pub.dev/packages/swagger_parser
MIT License
90 stars 38 forks source link

Dart Multipart Generation: Missing Required, Descriptions, and dart:convert Import #144

Closed naipaka closed 6 months ago

naipaka commented 8 months ago

Hello.

First of all, I want to express my gratitude for your excellent work on this package. It has been a valuable tool in my projects.

However, I have encountered a few issues with the automatic code generation for multipart requests in Dart. These issues arise specifically from OpenAPI definitions, similar to the ones provided in the Swagger documentation under Multipart Requests.

Here is an example of the OpenAPI 3.0.3 definition I used, written in YAML format:

requestBody:
  content:
    multipart/form-data:
      schema:
        type: object
        properties:
          id: 
            description: Sample ID.
            type: string
            format: uuid
          address:
            description: Sample Address.
            type: object
            properties:
              street:
                description: Sample Street.
                type: string
              city:
                description: Sample City.
                type: string
          profileImage:
            description: Sample Profile Image.
            type: string
            format: binary
        required:
          - id
          - address
          - profileImage

From this definition, the following Dart code is generated:

// coverage:ignore-file
// GENERATED CODE - DO NOT MODIFY BY HAND
// ignore_for_file: type=lint

import 'dart:io';

import 'package:dio/dio.dart' hide Headers;
import 'package:retrofit/retrofit.dart';

import '../models/object0.dart';

part 'sample_client.g.dart';

@RestApi()
abstract class SampleClient {
  factory SampleClient(Dio dio, {String? baseUrl}) = _SampleClient;

  /// Sample.
  /// 
  /// Create Sample.
  @MultiPart()
  @POST('/sample')
  Future<void> postSample({
    @Part(name: 'id') String? id,
    @Part(name: 'address') Object0? address,
    @Part(name: 'profileImage') File? profileImage,
  });
}

However, there are several issues with the generated code:

  1. Ignoring required fields in schema: The required fields id, address, and profileImage in the schema are not reflected in the generated postSample method. The parameters in this method lack the required keyword, which is necessary for ensuring data integrity. This makes the code unusable as is. Furthermore, even explicitly specifying nullable: false for each property resulted in the same issue.

  2. Missing dart:convert import: In the sample_client.g.dart file generated by the RetrofitGenerator, jsonEncode is implemented for the address, but since there is no import for dart:convert, it results in a compilation error.

  3. Omission of descriptions in documentation comments: Although the schema's description defines explanations for id, address, and profileImage, the doc comments for the postSample method do not include descriptions for these arguments.

The ideal solution would be for the code generation to respect these aspects of the OpenAPI definition, producing something along these lines:

// coverage:ignore-file
// GENERATED CODE - DO NOT MODIFY BY HAND
// ignore_for_file: type=lint

import 'dart:convert';  // Added line
import 'dart:io';

import 'package:dio/dio.dart' hide Headers;
import 'package:retrofit/retrofit.dart';

import '../models/object0.dart';

part 'sample_client.g.dart';

@RestApi()
abstract class SampleClient {
  factory SampleClient(Dio dio, {String? baseUrl}) = _SampleClient;

  /// Sample.
  ///
  /// Create Sample.
  ///
  /// [id] - Sample ID.  // Added line
  /// [address] - Sample Address.  // Added line
  /// [profileImage] - Sample Profile Image.  // Added line
  @MultiPart()
  @POST('/sample')
  Future<void> postSample({
    @Part(name: 'id') required String id,  // Modified line
    @Part(name: 'address') required Object0 address,  // Modified line
    @Part(name: 'profileImage') required File profileImage,  // Modified line
  });
}

I hope these issues can be addressed in a future update to improve the tool's functionality and usability. Thank you for your efforts in maintaining this valuable project.

SasLuca commented 8 months ago

I wanted to add to this issue since it deals with an area I am having problems with as well.

My swagger.json is generated by the web framework I'm using and for all my requests the request parameters are not inlined with the endpoint but instead put under a model and referenced. Eg:

"AgathaApiEndpointsAdministrationDoctorsPatchDoctorMediaContentRequest": {
    "type": "object",
    "additionalProperties": false,
    "properties": {
        "description": {
            "type": "array",
            "nullable": true,
            "items": {
                "$ref": "#/components/schemas/AgathaApiStrongTypesTranslation"
            }
        },
        "url": {
            "type": "string",
            "nullable": true
        },
        "image": {
            "type": "string",
            "format": "binary",
            "nullable": true
        }
    }
  },

Because of this, swagger_parser tries to create a class for it with freezedthat looks like this

// coverage:ignore-file
// GENERATED CODE - DO NOT MODIFY BY HAND
// ignore_for_file: type=lint

import 'dart:io';

import 'package:freezed_annotation/freezed_annotation.dart';

import 'agatha_api_strong_types_translation.dart';

part 'administration_doctors_patch_doctor_media_content_request.freezed.dart';
part 'administration_doctors_patch_doctor_media_content_request.g.dart';

@Freezed()
class AdministrationDoctorsPatchDoctorMediaContentRequest with _$AdministrationDoctorsPatchDoctorMediaContentRequest {
  const factory AdministrationDoctorsPatchDoctorMediaContentRequest({
    List<AgathaApiStrongTypesTranslation>? description,
    String? url,
    File? image,
  }) = _AdministrationDoctorsPatchDoctorMediaContentRequest;

  factory AdministrationDoctorsPatchDoctorMediaContentRequest.fromJson(Map<String, Object?> json) => _$AdministrationDoctorsPatchDoctorMediaContentRequestFromJson(json);
}

Because freezed can't deal with File this results in build_runner errors.

If there is some workaround here please let me know.

StarProxima commented 7 months ago

@SasLuca Hi, can you provide the full expected result of the generated code? As far as I understand, this class is not expected to be generated at all, but all the properties from it should be present in the query itself, like in the @naipaka example?

I will look into this problem shortly.

SasLuca commented 7 months ago

Hi @StarProxima, not sure I understand what you are asking but I made a reproduction of my issue.

The jist of it is that json_serializable cant handle File instances in data structures, which means we cant have code generation for endpoints with file form data (eg: an endpoint for uploading a file picture).

It would be nice if this can be fixed (or maybe it is and just needs documentation?) so that we can also generate endpoints with Files sent as form data.

Reproduction: https://github.com/SasLuca/SwaggerParserIssueRepro/tree/master

If you have more questions let me know.

StarProxima commented 6 months ago

@naipaka Can you see if the generation on your schema works correctly for the swagger_parser version from the git?

@SasLuca Your problem is now related to https://github.com/Carapacik/swagger_parser/issues/154

SasLuca commented 6 months ago

Hi @StarProxima, i am on holiday til Friday, but i will ask someone at my office to try if they have time until then and report back here. If not i will try it on friday and let you know.

SasLuca commented 6 months ago

Ah, seems like you wanted someone else to test not me. If you need me to test anything let me know with a ping.

naipaka commented 6 months ago

@StarProxima I have confirmed that all three issues initially mentioned in the issue have been resolved. Your support in this matter has been incredibly helpful. Thank you very much.