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

[BUG][dart-dio] Accept header not generated #15427

Open
5 of 6 tasks
rfuerst87 opened this issue May 5, 2023 · 6 comments
Open
5 of 6 tasks

[BUG][dart-dio] Accept header not generated #15427

rfuerst87 opened this issue May 5, 2023 · 6 comments

Comments

@rfuerst87
Copy link

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • Have you tested with the latest master to confirm the issue still exists?
  • Have you searched for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

dart-dio generator seems to not generate Accept header even though it is specified in responses.<code>.content.<media-type>. According to the docs I would assume that the generator sets the Accept header to <media-type>. This seems not to happen. Am I missing something here?

openapi-generator version

6.5.0

OpenAPI declaration file content or url
openapi: 3.0.1
info:
  title: OpenAPI definition
  version: v0
paths:
  /test:
    get:
      responses:
        "200":
          description: Test
          content:
            application/json:
              schema:
                type: string
                format: uuid
Suggest a fix

The specs above should generate something like this:

Future<Response<String>> testGet({
  CancelToken? cancelToken,
  Map<String, dynamic>? headers,
  Map<String, dynamic>? extra,
  ValidateStatus? validateStatus,
  ProgressCallback? onSendProgress,
  ProgressCallback? onReceiveProgress,
}) async {
  final _path = r'/test';
  final _options = Options(
    method: r'GET',
    headers: <String, dynamic>{
      'Accept': 'application/json',          // <-- this header should be generated
      ...?headers,
    },
    extra: <String, dynamic>{
      'secure': <Map<String, String>>[],
      ...?extra,
    },
    validateStatus: validateStatus,
  );

  // ...
}
@ahmednfwela
Copy link
Contributor

what should happen when there are multiple content children ?

@jaumard (2018/09) @josh-burton (2019/12) @amondnet (2019/12) @sbu-WBT (2020/12) @kuhnroyal (2020/12) @agilob (2020/12) @ahmednfwela (2021/08)

@kuhnroyal
Copy link
Contributor

Yea that is the question, and probably why we don't generate this yet.
But since we only support JSON, we can probably generate the Accept header by default or when a application/json content is defined?

@rfuerst87
Copy link
Author

Another approach is maybe to generate the Accept header only, if there is just a single response media type defined. If multiple types are defined, it's up to the consumer to select one.

The main issue we have with the current implementation is, that it doesn't play well with media type versioning. For example we have something like application/vnd.abc.v2+json that has to be included in the Accept header, otherwise the server isn't able to map the request.

By the way: How do other generators handle this scenario?

@kuhnroyal
Copy link
Contributor

kuhnroyal commented May 9, 2023

Another approach is maybe to generate the Accept header only, if there is just a single response media type defined. If multiple types are defined, it's up to the consumer to select one.

Well yea, but we do not support xml or whatnot so we can ignore those and reduce it to json/text content types. And after that, check what is left and then see if there is a single one left.

By the way: How do other generators handle this scenario?

I don't know but some support XML and also some other wire formats, so that doesn't really matter here.

@ahmednfwela
Copy link
Contributor

This question came to me when I was doing #14346
one way we do that is to have multiple methods for each endpoint

  • a "Raw" method that is concerned about the networking part (sending and receiving raw request/response data)
  • and another "Typed" method, that's concerned about adapting input/output (sending and receiving serialized models)

so take for example this existing generated method

  Future<Response<Pet>> testEchoBodyPet({
    Pet? pet,
    CancelToken? cancelToken,
    Map<String, dynamic>? headers,
    Map<String, dynamic>? extra,
    ValidateStatus? validateStatus,
    ProgressCallback? onSendProgress,
    ProgressCallback? onReceiveProgress,
  }) async {
    final _path = r'/echo/body/Pet';
    final _options = Options(
      method: r'POST',
      headers: <String, dynamic>{
        ...?headers,
      },
      extra: <String, dynamic>{
        'secure': <Map<String, String>>[],
        ...?extra,
      },
      contentType: 'application/json',
      validateStatus: validateStatus,
    );

    dynamic _bodyData;

    try {
      const _type = FullType(Pet);
      _bodyData = pet == null
          ? null
          : _serializers.serialize(pet, specifiedType: _type);
    } catch (error, stackTrace) {
      throw DioError(
        requestOptions: _options.compose(
          _dio.options,
          _path,
        ),
        type: DioErrorType.other,
        error: error,
      )..stackTrace = stackTrace;
    }

    final _response = await _dio.request<Object>(
      _path,
      data: _bodyData,
      options: _options,
      cancelToken: cancelToken,
      onSendProgress: onSendProgress,
      onReceiveProgress: onReceiveProgress,
    );

    Pet _responseData;

    try {
      const _responseType = FullType(Pet);
      _responseData = _serializers.deserialize(
        _response.data!,
        specifiedType: _responseType,
      ) as Pet;
    } catch (error, stackTrace) {
      throw DioError(
        requestOptions: _response.requestOptions,
        response: _response,
        type: DioErrorType.other,
        error: error,
      )..stackTrace = stackTrace;
    }

    return Response<Pet>(
      data: _responseData,
      headers: _response.headers,
      isRedirect: _response.isRedirect,
      requestOptions: _response.requestOptions,
      redirects: _response.redirects,
      statusCode: _response.statusCode,
      statusMessage: _response.statusMessage,
      extra: _response.extra,
    );
  }
}

it can be refactored into 2 parts

  1. A raw method, which doesn't care about serialization/deserialization
Future<Response<Object>> testEchoBodyPetRaw({
  Object? data,
  CancelToken? cancelToken,
  Map<String, dynamic>? headers,
  Map<String, dynamic>? extra,
  ValidateStatus? validateStatus,
  ProgressCallback? onSendProgress,
  ProgressCallback? onReceiveProgress,
  //Extra parameters for raw
  String? requestContentType,
  String? acceptContentType,
}) async {
  final _path = r'/echo/body/Pet';
  final _options = Options(
    method: r'POST',
    headers: <String, dynamic>{
      if (acceptContentType != null) 'Accept': acceptContentType,
      ...?headers,
    },
    extra: <String, dynamic>{
      'secure': <Map<String, String>>[],
      ...?extra,
    },
    contentType: requestContentType,
    validateStatus: validateStatus,
  );

  final _response = await _dio.request<Object>(
    _path,
    data: data,
    options: _options,
    cancelToken: cancelToken,
    onSendProgress: onSendProgress,
    onReceiveProgress: onReceiveProgress,
  );

  return _response;
}
  1. default method which uses json or binary depending on the data types
Future<Response<Pet>> testEchoBodyPet({
  Pet? pet,
  CancelToken? cancelToken,
  Map<String, dynamic>? headers,
  Map<String, dynamic>? extra,
  ValidateStatus? validateStatus,
  ProgressCallback? onSendProgress,
  ProgressCallback? onReceiveProgress,
}) async {
  final _bodyData = pet == null
      ? null
      : _serializers.serialize(pet, specifiedType: const FullType(Pet));

  final _response = await testEchoBodyPetRaw(
    data: _bodyData,
    // change these depending on the return type
    // sometimes we want to send files, or receive files
    requestContentType: 'application/json',
    acceptContentType: 'application/json',
    cancelToken: cancelToken,
    extra: extra,
    headers: headers,
    onReceiveProgress: onReceiveProgress,
    onSendProgress: onSendProgress,
    validateStatus: validateStatus,
  );

  final _rawResponseData = _response.data;
  final _responseData = _rawResponseData == null
      ? null
      : _serializers.deserialize(
          _rawResponseData,
          specifiedType: const FullType(Pet),
        ) as Pet;

  return Response<Pet>(
    data: _responseData,
    headers: _response.headers,
    isRedirect: _response.isRedirect,
    requestOptions: _response.requestOptions,
    redirects: _response.redirects,
    statusCode: _response.statusCode,
    statusMessage: _response.statusMessage,
    extra: _response.extra,
  );
}

this way consumers can select different content types, on the expense they handle serialization/deserialization themselves

@kuhnroyal
Copy link
Contributor

I like the RAW idea but I think this is out of scope here. Can you summarise this in another issue? :)

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

No branches or pull requests

3 participants