Skip to content

Comments

Added support for custom extensions#171

Closed
darrelmiller wants to merge 1 commit intomasterfrom
dm/pluggable-extensions
Closed

Added support for custom extensions#171
darrelmiller wants to merge 1 commit intomasterfrom
dm/pluggable-extensions

Conversation

@darrelmiller
Copy link
Member

This PR is a proposal for potentially implementing strongly typed custom extensions. I have introduced a new interface IOpenApiExtension which IOpenApiAny now implements so it does still work as it did before. However, you can now also create custom classes for those extensions.

Currently the custom classes need to deserialize from an IOpenApiAny instance, but this might not be the best approach. I was going to make them deserialize from ParseNode but that would require making that set of classes public, which I'm not too sure we want to do.

I also want to make sure we can use the validation rules to validate these custom extensions. That's the next thing I want to try. But in the meanwhile, if I can get some feedback on the approach so far and your feeling about parsing from ParseNode vs OpenApiAny that would be great.

@PerthCharern
Copy link
Contributor

Thanks Darrel.

  • Exposing ParseNode will be a big change, given a bunch of things that should remain internal. I think using the Any type like you did is a better approach.
  • I wonder if IOpenApiAny : IOpenApiExtension is the right approach. I see that we currently need this to be able to add the primitive/null into the extension slot, and we have a catch-all type from .CreateAny() that can be used when ExtensionParser is not present. It seems rather complicated, but I don't have a better solution for this.

@darrelmiller
Copy link
Member Author

@PerthCharern I'm not really happy with the solution, but every other alternative that I have considered so far seems worse!

@PerthCharern
Copy link
Contributor

I believe this PR is a subset of #176, right? I'll give comments directly on #176 if that's the case.

@darrelmiller
Copy link
Member Author

@PerthCharern Yes. In fact I'm just going to close this PR.

@darrelmiller darrelmiller deleted the dm/pluggable-extensions branch January 4, 2020 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants