Skip to content

#4241, #4236 Add cutom Json serializer and proxy settings to config #4466

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ namespace {{packageName}}.Client
RestRequest request = new RestRequest(Method(method))
{
Resource = path,
JsonSerializer = new CustomJsonCodec(configuration)
JsonSerializer = new CustomJsonCodec(configuration.SerializerSettings,configuration)
};

if (options.PathParameters != null)
Expand Down Expand Up @@ -380,7 +380,10 @@ namespace {{packageName}}.Client

private {{#supportsAsync}}async Task<ApiResponse<T>>{{/supportsAsync}}{{^supportsAsync}}ApiResponse<T>{{/supportsAsync}} Exec<T>(RestRequest req, IReadableConfiguration configuration)
{
RestClient client = new RestClient(_baseUrl);
RestClient client = new RestClient(_baseUrl)
{
Proxy = configuration.Proxy
};

client.ClearHandlers();
var existingDeserializer = req.JsonSerializer as IDeserializer;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ using System.Collections.Concurrent;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Net;
using System.Text;
using Newtonsoft.Json;

namespace {{packageName}}.Client
{
Expand Down Expand Up @@ -223,6 +225,13 @@ namespace {{packageName}}.Client
/// <value>The access token.</value>
public virtual string AccessToken { get; set; }


/// <inheritdoc />
public JsonSerializerSettings SerializerSettings { get; set; } = new JsonSerializerSettings();
Copy link
Member

@jimschubert jimschubert Nov 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will need to be moved elsewhere… maybe as a constructor argument for ApiClient or as a settable property (or both?).

Configuration should be decoupled from framework implementation. For example, ApiClient is tightly coupled to RestSharp and JSON.net. Having Configuration rely only on "standard" types means that users may replace ApiClient with their desired accessor implementations. Including Json.NET types in Configuration will make this unusable for users who don't pull that type into their client.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its possible to move it to the constructor. But everything is configurated inside the Configuration type but just not this? I think every C# developer use Newtonsoft.Json. Even if the ApiClient changes. When it moved to the constructor of the ApiClient , the class that wrap this must also use Newtonsoft.Json.
If you want to use this type just inside the ApiClient , we have to create a custom type and map everything. But this is not a good way.


/// <inheritdoc />
public WebProxy Proxy { get; set; }

/// <summary>
/// Gets or sets the temporary folder path to store the files downloaded from the server.
/// </summary>
Expand Down Expand Up @@ -408,7 +417,9 @@ namespace {{packageName}}.Client
Password = second.Password ?? first.Password,
AccessToken = second.AccessToken ?? first.AccessToken,
TempFolderPath = second.TempFolderPath ?? first.TempFolderPath,
DateTimeFormat = second.DateTimeFormat ?? first.DateTimeFormat
DateTimeFormat = second.DateTimeFormat ?? first.DateTimeFormat,
Proxy = second.Proxy ?? first.Proxy,
SerializerSettings = second.SerializerSettings ?? first.SerializerSettings
};
return config;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

using System;
using System.Collections.Generic;
using System.Net;
using Newtonsoft.Json;

namespace {{packageName}}.Client
{
Expand Down Expand Up @@ -89,5 +91,15 @@ namespace {{packageName}}.Client
/// <param name="apiKeyIdentifier">API key identifier (authentication scheme).</param>
/// <returns>API key with prefix.</returns>
string GetApiKeyWithPrefix(string apiKeyIdentifier);

/// <summary>
/// Gets the json serializer settings
/// </summary>
JsonSerializerSettings SerializerSettings { get; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous comment about scoping this to ApiClient only.


/// <summary>
/// Gets the proxy
/// </summary>
WebProxy Proxy { get; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: changing this interface is a breaking change for existing client consumers.
Also, formatting is off by 4 spaces too many here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this interface is just used from the Configuration ? And this class is also generated. Using this proxy is optional

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since 5.0 is released soon, I think the breaking change here will be fine for client release. I still think the JsonSerializerSettings should stay in ApiClient, however, because this isn't API configuration but an implementation detail of ApiClient. Since we allow for users to define their own ApiClients (and bring their own serializers), it doesn't make a lot of sense to put ApiClient implementation-specific details into the Configuration object. That is, this configuration object should refer to System namespaces only.

I'd be happy to resolve this in a separate commit.

}
}
186 changes: 186 additions & 0 deletions samples/client/petstore/csharp/SwaggerClientNetCoreProject/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
# Ref: https://gist.github.com/kmorcinek/2710267
# Download this file using PowerShell v3 under Windows with the following comand
# Invoke-WebRequest https://gist.githubusercontent.com/kmorcinek/2710267/raw/ -OutFile .gitignore

# User-specific files
*.suo
*.user
*.sln.docstates
./nuget

# Build results

[Dd]ebug/
[Rr]elease/
x64/
build/
[Bb]in/
[Oo]bj/

# NuGet Packages
*.nupkg
# The packages folder can be ignored because of Package Restore
**/packages/*
# except build/, which is used as an MSBuild target.
!**/packages/build/
# Uncomment if necessary however generally it will be regenerated when needed
#!**/packages/repositories.config

# MSTest test Results
[Tt]est[Rr]esult*/
[Bb]uild[Ll]og.*

*_i.c
*_p.c
*.ilk
*.meta
*.obj
*.pch
*.pdb
*.pgc
*.pgd
*.rsp
*.sbr
*.tlb
*.tli
*.tlh
*.tmp
*.tmp_proj
*.log
*.vspscc
*.vssscc
.builds
*.pidb
*.log
*.scc

# OS generated files #
.DS_Store*
ehthumbs.db
Icon?
Thumbs.db

# Visual C++ cache files
ipch/
*.aps
*.ncb
*.opensdf
*.sdf
*.cachefile

# Visual Studio profiler
*.psess
*.vsp
*.vspx

# Guidance Automation Toolkit
*.gpState

# ReSharper is a .NET coding add-in
_ReSharper*/
*.[Rr]e[Ss]harper

# TeamCity is a build add-in
_TeamCity*

# DotCover is a Code Coverage Tool
*.dotCover

# NCrunch
*.ncrunch*
.*crunch*.local.xml

# Installshield output folder
[Ee]xpress/

# DocProject is a documentation generator add-in
DocProject/buildhelp/
DocProject/Help/*.HxT
DocProject/Help/*.HxC
DocProject/Help/*.hhc
DocProject/Help/*.hhk
DocProject/Help/*.hhp
DocProject/Help/Html2
DocProject/Help/html

# Click-Once directory
publish/

# Publish Web Output
*.Publish.xml

# Windows Azure Build Output
csx
*.build.csdef

# Windows Store app package directory
AppPackages/

# Others
sql/
*.Cache
ClientBin/
[Ss]tyle[Cc]op.*
~$*
*~
*.dbmdl
*.[Pp]ublish.xml
*.pfx
*.publishsettings
modulesbin/
tempbin/

# EPiServer Site file (VPP)
AppData/

# RIA/Silverlight projects
Generated_Code/

# Backup & report files from converting an old project file to a newer
# Visual Studio version. Backup files are not needed, because we have git ;-)
_UpgradeReport_Files/
Backup*/
UpgradeLog*.XML
UpgradeLog*.htm

# vim
*.txt~
*.swp
*.swo

# svn
.svn

# SQL Server files
**/App_Data/*.mdf
**/App_Data/*.ldf
**/App_Data/*.sdf


#LightSwitch generated files
GeneratedArtifacts/
_Pvt_Extensions/
ModelManifest.xml

# =========================
# Windows detritus
# =========================

# Windows image file caches
Thumbs.db
ehthumbs.db

# Folder config file
Desktop.ini

# Recycle Bin used on file shares
$RECYCLE.BIN/

# Mac desktop service store files
.DS_Store

# SASS Compiler cache
.sass-cache

# Visual Studio 2014 CTP
**/*.sln.ide
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# OpenAPI Generator Ignore
# Generated by openapi-generator https://github.com/openapitools/openapi-generator

# Use this file to prevent files from being overwritten by the generator.
# The patterns follow closely to .gitignore or .dockerignore.

# As an example, the C# client generator defines ApiClient.cs.
# You can make changes and tell OpenAPI Generator to ignore just this file by uncommenting the following line:
#ApiClient.cs

# You can match any string of characters against a directory, file or extension with a single asterisk (*):
#foo/*/qux
# The above matches foo/bar/qux and foo/baz/qux, but not foo/bar/baz/qux

# You can recursively match patterns against a directory, file or extension with a double asterisk (**):
#foo/**/qux
# This matches foo/bar/qux, foo/baz/qux, and foo/bar/baz/qux

# You can also negate patterns with an exclamation (!).
# For example, you can ignore all files in a docs folder with the file extension .md:
#docs/*.md
# Then explicitly reverse the ignore rule for a single file:
#!docs/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
4.1.3
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio 14
VisualStudioVersion = 14.0.25420.1
MinimumVisualStudioVersion = 10.0.40219.1
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Org.OpenAPITools", "src\Org.OpenAPITools\Org.OpenAPITools.csproj", "{67035b31-f8e5-41a4-9673-954035084f7d}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Release|Any CPU = Release|Any CPU
EndGlobalSection
GlobalSection(ProjectConfigurationPlatforms) = postSolution
{67035b31-f8e5-41a4-9673-954035084f7d}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{67035b31-f8e5-41a4-9673-954035084f7d}.Debug|Any CPU.Build.0 = Debug|Any CPU
{67035b31-f8e5-41a4-9673-954035084f7d}.Release|Any CPU.ActiveCfg = Release|Any CPU
{67035b31-f8e5-41a4-9673-954035084f7d}.Release|Any CPU.Build.0 = Release|Any CPU
{19F1DEBC-DE5E-4517-8062-F000CD499087}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{19F1DEBC-DE5E-4517-8062-F000CD499087}.Debug|Any CPU.Build.0 = Debug|Any CPU
{19F1DEBC-DE5E-4517-8062-F000CD499087}.Release|Any CPU.ActiveCfg = Release|Any CPU
{19F1DEBC-DE5E-4517-8062-F000CD499087}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
EndGlobalSection
EndGlobal
Loading