{"action":"created","issue":{"url":"https://api.github.com/repos/elastic/elasticsearch-net/issues/2183","repository_url":"https://api.github.com/repos/elastic/elasticsearch-net","labels_url":"https://api.github.com/repos/elastic/elasticsearch-net/issues/2183/labels{/name}","comments_url":"https://api.github.com/repos/elastic/elasticsearch-net/issues/2183/comments","events_url":"https://api.github.com/repos/elastic/elasticsearch-net/issues/2183/events","html_url":"https://github.com/elastic/elasticsearch-net/pull/2183","id":168612586,"number":2183,"title":"Lazily resolve JsonSerializers in JsonNetSerializer","user":{"login":"tsliang","id":1897041,"avatar_url":"https://avatars.githubusercontent.com/u/1897041?v=3","gravatar_id":"","url":"https://api.github.com/users/tsliang","html_url":"https://github.com/tsliang","followers_url":"https://api.github.com/users/tsliang/followers","following_url":"https://api.github.com/users/tsliang/following{/other_user}","gists_url":"https://api.github.com/users/tsliang/gists{/gist_id}","starred_url":"https://api.github.com/users/tsliang/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tsliang/subscriptions","organizations_url":"https://api.github.com/users/tsliang/orgs","repos_url":"https://api.github.com/users/tsliang/repos","events_url":"https://api.github.com/users/tsliang/events{/privacy}","received_events_url":"https://api.github.com/users/tsliang/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2016-08-01T10:14:57Z","updated_at":"2016-08-01T22:44:01Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/elastic/elasticsearch-net/pulls/2183","html_url":"https://github.com/elastic/elasticsearch-net/pull/2183","diff_url":"https://github.com/elastic/elasticsearch-net/pull/2183.diff","patch_url":"https://github.com/elastic/elasticsearch-net/pull/2183.patch"},"body":"The current JsonNetSerializer implementation creates the default\r\nserializers\r\nwithin its constructor, which then calls ModifyJsonSerializerSettings.\r\nThis\r\nmeans that a subclass of JsonNetSerializer cannot modify the\r\nJsonSerializerSettings with fields set by the constructor, because the\r\norder of\r\noperations is:\r\n\r\n  1. Call subclass constructor with parameter A, which will set subclass\r\n  field A; field A will be used in ModifyJsonSerializerSettings.\r\n  2. Subclass constructor calls JsonNetSerializer constructor with\r\n     :base()\r\n  3. JsonNetSerializer constructor calls ModifyJsonSerializerSettings\r\n     as part of JsonSerializer.Create\r\n  4. ModifyJsonSerializerSettings makes use of default value for\r\n     field A, because it has not been set yet.\r\n  5. JsonSerializer constructor exits\r\n  6. Subclass constructor sets value of field A from parameter\r\n     A.\r\n\r\n  By using the Lazy<T> class, we can ensure that ModifyJsonSerializerSettings is\r\n  called after the subclass constructor has exited, which allows fields to\r\n  be set and thereby used in ModifyJsonSerializerSettings. The unit test\r\n  in Connection.doc.cs has been expanded to demonstrate this capability\r\n  with a (fairly trivial) use case."},"comment":{"url":"https://api.github.com/repos/elastic/elasticsearch-net/issues/comments/236730431","html_url":"https://github.com/elastic/elasticsearch-net/pull/2183#issuecomment-236730431","issue_url":"https://api.github.com/repos/elastic/elasticsearch-net/issues/2183","id":236730431,"user":{"login":"gmarz","id":1594777,"avatar_url":"https://avatars.githubusercontent.com/u/1594777?v=3","gravatar_id":"","url":"https://api.github.com/users/gmarz","html_url":"https://github.com/gmarz","followers_url":"https://api.github.com/users/gmarz/followers","following_url":"https://api.github.com/users/gmarz/following{/other_user}","gists_url":"https://api.github.com/users/gmarz/gists{/gist_id}","starred_url":"https://api.github.com/users/gmarz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gmarz/subscriptions","organizations_url":"https://api.github.com/users/gmarz/orgs","repos_url":"https://api.github.com/users/gmarz/repos","events_url":"https://api.github.com/users/gmarz/events{/privacy}","received_events_url":"https://api.github.com/users/gmarz/received_events","type":"User","site_admin":false},"created_at":"2016-08-01T22:44:01Z","updated_at":"2016-08-01T22:44:01Z","body":"Thanks for this @tsliang :+1: \r\n\r\nFor the most part, this LGTM.  However, calling a virtual method in a base constructor as we're doing here  when calling `ModifyJsonSerializerSettings()` is generally a bad idea, for reasons exactly like this.  Lazy instantiating the serializers feels like we're working around the problem rather than addressing it...but it might be the only way to fix this though.\r\n\r\nWould love to get @Mpdreamz and @russcam s opinion on this.\r\n\r\n"}}
{"actor":{"display_login":"gmarz"}}