tazatechnology / openapi_spec

Dart based OpenAPI specification generator and parser
BSD 3-Clause "New" or "Revised" License
9 stars 6 forks source link

Closes #31 Support oneOf that contains primitive types and objects #32

Closed walsha2 closed 11 months ago

walsha2 commented 11 months ago

@davidmigloz this is a little tricky, how does one discern between the following hypothetical example where there are no union keys between objects:

function_call:
  description: Controls how the model calls functions. 
  oneOf:
    - type: string
      enum: [none, auto]
      title: ChatCompletionFunctionCallMode
    - $ref: "#/components/schemas/ChatCompletionFunctionCallOption"
    - type: object
      additionalParameters: true

Im not a fan of this hypothetical API definition, but it still allowed by the OpenAPI spec.

ChatCompletionFunctionCallOption and the other object cannot be differentiated from Map<String,dynamic> for deserialization logic... just one use case that came to mind. The solution in this PR accounts for that should it arise by having a smart sort order and try/catch when attempting to deserialize a known object union. Best solution I could conjure without making things more difficult than necessary.

Result

For your use case referenced in #31 this would be the result. I am not really sure of a better way to shorten the constructor for ChatCompletionFunctionCall.chatCompletionFunctionCallOption and ensure it is unique. Open to suggestions.

If this is good enough for you, we can merge it in. Will wait for confirmation or any follow-up suggestions.

CreateChatCompletionRequest c;

c = CreateChatCompletionRequest(
  messages: [],
  model: CreateChatCompletionRequestModel.string('custom-model'),
  functionCall: ChatCompletionFunctionCall.enumeration(
    ChatCompletionFunctionCallMode.auto,
  ),
);

print(c.toJson());
print(CreateChatCompletionRequest.fromJson(c.toJson()));

c = CreateChatCompletionRequest(
  messages: [],
  model: CreateChatCompletionRequestModel.string('custom-model'),
  functionCall: ChatCompletionFunctionCall.chatCompletionFunctionCallOption(
    ChatCompletionFunctionCallOption(name: 'option'),
  ),
);

print(c.toJson());
print(CreateChatCompletionRequest.fromJson(c.toJson()));
// ==========================================
// CLASS: ChatCompletionFunctionCall
// ==========================================

/// Controls how the model calls functions.
@freezed
sealed class ChatCompletionFunctionCall with _$ChatCompletionFunctionCall {
  const ChatCompletionFunctionCall._();

  const factory ChatCompletionFunctionCall.enumeration(
    ChatCompletionFunctionCallMode value,
  ) = _UnionChatCompletionFunctionCallEnum;

  const factory ChatCompletionFunctionCall.chatCompletionFunctionCallOption(
    ChatCompletionFunctionCallOption value,
  ) = _UnionChatCompletionFunctionCallChatCompletionFunctionCallOption;

  /// Object construction from a JSON representation
  factory ChatCompletionFunctionCall.fromJson(Map<String, dynamic> json) =>
      _$ChatCompletionFunctionCallFromJson(json);
}

/// Custom JSON converter for [ChatCompletionFunctionCall]
class _ChatCompletionFunctionCallConverter
    implements JsonConverter<ChatCompletionFunctionCall?, Object?> {
  const _ChatCompletionFunctionCallConverter();

  @override
  ChatCompletionFunctionCall? fromJson(Object? data) {
    if (data == null) {
      return null;
    }
    if (data is String &&
        _$ChatCompletionFunctionCallModeEnumMap.values.contains(data)) {
      return ChatCompletionFunctionCall.enumeration(
        _$ChatCompletionFunctionCallModeEnumMap.keys.elementAt(
          _$ChatCompletionFunctionCallModeEnumMap.values.toList().indexOf(data),
        ),
      );
    }
    if (data is Map<String, dynamic>) {
      try {
        return ChatCompletionFunctionCall.chatCompletionFunctionCallOption(
          ChatCompletionFunctionCallOption.fromJson(data),
        );
      } catch (e) {}
    }
    throw Exception(
      'Unexpected value for ChatCompletionFunctionCall: $data',
    );
  }

  @override
  Object? toJson(ChatCompletionFunctionCall? data) {
    return switch (data) {
      _UnionChatCompletionFunctionCallEnum(value: final v) =>
        _$ChatCompletionFunctionCallModeEnumMap[v]!,
      _UnionChatCompletionFunctionCallChatCompletionFunctionCallOption(
        value: final v
      ) =>
        v.toJson(),
      null => null,
    };
  }
}
davidmigloz commented 11 months ago

Thanks again for the fast implementation. lgtm!

I think the ordered try-catch approach is a good solution. Anyway, OpenAPI doesn't explicitly specify how conflicts should be handled, so returning the first schema that matches sounds the most reasonable way to go.

Regarding the factory constructors names, I think it's good enough.

I had in my mind a feature request to allow to specify custom factory names, which (together with the already implemented titles) would be the ultimate piece for full customization support.

My idea was to use Specification Extensions to define the factory name in the schema (e.g. using something like x-factoryName):

function_call:
  title: ChatCompletionFunctionCall
  description: Controls how the model calls functions.
  oneOf:
    - type: string
      title: ChatCompletionFunctionCallMode
      x-factoryName: mode
      enum: [none, auto]
    - $ref: "#/components/schemas/ChatCompletionFunctionCallOption"
ChatCompletionFunctionCallOption:
  type: object
  x-factoryName: functionCallOption
  properties:
    name:
      type: string
      description: The name of the function to call.
  required:
    - name

(but that's out of scope for this PR, we can move it to a separate issue)

walsha2 commented 11 months ago

My idea was to use Specification Extensions to define the factory name in the schema (e.g. using something like x-factoryName):

Yea this package does not introduce any sort of package specific customization like that. The idea was to just be compatible with any API schema out of the box, without introducing any extra dependency or custom definitions.

@davidmigloz alternatively, we can do it at the code generation level with an onUnionConstructorName function that would allow a user to override the constructor of a specific union. Feel free to open an issue and we can move that chat there.

Thanks again for the fast implementation. lgtm!

Will cut a new release with these changes.

walsha2 commented 11 months ago

openapi_spec (v0.7.3) - Published.

davidmigloz commented 11 months ago

I've published the first version of the OpenAI client generated with this package: openai_dart 🚀

Pretty happy with the result! (the OpenAI spec was a good test for the generator, it has all kinds of edge cases 😄)

There are a couple of areas where we could still improve the generator:

walsha2 commented 11 months ago

OpenAI spec was a good test for the generator

100%. Adding it as another test to exercise the generators has been a great addition.

Will open a couple issues from the comments above.