Provide a AdWordsAppConfig constructor that works with IConfigurationSection

[aevitas]

This is more of a design issue than an actual functional issue, but in my opinion, it's a fairly significant design mistake that should be corrected. I am using this library with ASP.NET Core 2.1 specifically.

Tl;dr - it should accept a IConfigurationSection instead

Currently, AdWordsAppConfig has two public ctors:

   public AdWordsAppConfig();
   public AdWordsAppConfig(IConfigurationRoot configurationRoot);

The first constructor is a bit of a magic constructor. It does stuff by reading from a config section, but it's not very apparent where it's getting its values from, and it fails without a meaningful exception message.

The second constructor accepts an IConfigurationRoot. In ASP.NET Core's Startup.cs file, configuration is already mostly handled for us, and we'll only have an IConfiguration instance. We can cast this to an IConfigurationRoot and pass it to the AdWordsAppConfig, but that's less than ideal.

Passing the configuration root eventually hits ReadSettings. This method doesn't handle "surplus" values in the configuration very well, and instead of ignoring them, it'll throw a NullReferenceException. This means that we can not have anything in our config file except the AdWords credentials, or it'll throw.

This could fairly easily be fixed by either adding an overload which accepts IConfigurationSection, so we can pass Configuration.GetSection("AdWordsCredentials"); or similar. Personally however, I think the current overload accepting the configuration root is strictly wrong from a conceptual perspective and should not exist.

As it stands, the only way I've gotten it to work in ASP.NET Core is by creating a separate configuration file and registering the resulting IConfigurationRoot with the DI container:

    var builder = new ConfigurationBuilder();
    builder.AddJsonFile("adwordssettings.json");

    var config = builder.Build();

    services.AddSingleton(config);

Which works, but in my opinion is far from ideal.

Looking forward to your thoughts on this, and thank you very much for the great work you guys have done to make this library!

[christopherseeley]

This seems reasonable, I'll take a look. Thanks for all the context and details!

[AnashOommen]

I'll add an extra constructor that looks like

public AdWordsAppConfig(IConfigurationSection configurationSection);

that does what you requested.

[AnashOommen]

Fixed in 24.3.0