IntegerValidator is silly...

Question

Class, Test and very strange results below. Last time I checked, 2 was right there between 1 and 3.

 

namespace Fido.SnmpPollerServer.Configuration

{

using System;

using System.Collections.Generic;

using System.Configuration;

using System.Text;

/// <summary>Represents a host element tag on the PollConfigSection</summary>

public sealed class HostElement : ConfigurationElement

{

internal static class Tags

{

internal const String HostName = "hostName";

internal const String Community = "community";

internal const String SnmpVersion = "snmpVersion";

internal const String SnmpQueryTimeout = "snmpQueryTimeOut";

internal const String Targets = "targets";

internal const String AddTarget = "addTarget";

internal const String RemoveTarget = "removeTarget";

internal const String ClearTargets = "clearTargets";

}

[ConfigurationProperty(HostElement.Tags.HostName, IsKey = true, IsRequired = true)]

public String HostName

{

get { return (String)base[HostElement.Tags.HostName]; }

set { base[HostElement.Tags.HostName] = value; }

}

[ConfigurationProperty(HostElement.Tags.Community, IsRequired = true)]

public String Community

{

get { return (String)base[HostElement.Tags.Community]; }

set { base[HostElement.Tags.Community] = value; }

}

[ConfigurationProperty(HostElement.Tags.SnmpVersion, IsRequired = true)]

[IntegerValidator(MinValue = 1, MaxValue = 3, ExcludeRange = false)]

public Int32 SnmpVersion

{

get { return (Int32)base[HostElement.Tags.SnmpVersion]; }

set { base[HostElement.Tags.SnmpVersion] = value; }

}

 

[ConfigurationProperty(HostElement.Tags.SnmpQueryTimeout, IsRequired = true)]

public Int32 SnmpQueryTimeOut

{

get { return (Int32)base[HostElement.Tags.SnmpQueryTimeout]; }

set { base[HostElement.Tags.SnmpQueryTimeout] = value; }

}

[ConfigurationProperty(HostElement.Tags.Targets, IsRequired = true)]

[ConfigurationCollection(typeof(TargetElementCollection), AddItemName = HostElement.Tags.AddTarget, ClearItemsName = HostElement.Tags.ClearTargets, RemoveItemName = HostElement.Tags.RemoveTarget)]

public TargetElementCollection Targets

{

get { return (TargetElementCollection)base[HostElement.Tags.Targets]; }

}

}

}

 

-----------------

[TestMethod()]

public void CanAllPropertiesBeWrittenAndReadProperly__HostElement()

{

HostElement hostElement = new HostElement();

String hostName = "testHostName";

String community = "testCommunity";

Int32 snmpVersion = 2; // <-- really this is between 1 and 3... I PROMISE!

Int32 snmpQueryTimeout = 30;

hostElement.HostName = hostName;

hostElement.Community = community;

hostElement.SnmpVersion = snmpVersion;

hostElement.SnmpQueryTimeOut = snmpQueryTimeout;

Assert.AreEqual<String>(hostName, hostElement.HostName);

Assert.AreEqual<String>(community, hostElement.Community);

Assert.AreEqual<Int32>(snmpVersion, hostElement.SnmpVersion);

Assert.AreEqual<Int32>(snmpQueryTimeout, hostElement.SnmpQueryTimeOut);

Assert.IsNotNull(hostElement.Targets);

}

 

--------------------

// I know it's a silly test, but look what it found for me...

Test method Fido.CoreServers.Test.SnmpPollerServer.ConfigurationTests.CanAllPropertiesBeWrittenAndReadProperlyForHostElement threw exception:  System.Configuration.ConfigurationErrorsException: The value for the property 'snmpVersion' is not valid. The error is: The value must be inside the range 1-3. --->  System.ArgumentException: The value must be inside the range 1-3..

at System.Configuration.ValidatorUtils.ValidateRangeImpl[T](T value, T min, T max, Boolean exclusiveRange)
   at System.Configuration.ValidatorUtils.ValidateScalar[T](T value, T min, T max, T resolution, Boolean exclusiveRange)
   at System.Configuration.IntegerValidator.Validate(Object value)
   at System.Configuration.ConfigurationProperty.Validate(Object value)
 --- End of inner exception stack trace ---
    at System.Configuration.ConfigurationProperty.Validate(Object value)
   at System.Configuration.ConfigurationProperty.SetDefaultValue(Object value)
   at System.Configuration.ConfigurationProperty.InitDefaultValueFromTypeInfo(ConfigurationPropertyAttribute attribProperty, DefaultValueAttribute attribStdDefault)
   at System.Configuration.ConfigurationProperty..ctor(PropertyInfo info)
   at System.Configuration.ConfigurationElement.CreateConfigurationPropertyFromAttributes(PropertyInfo propertyInformation)
   at System.Configuration.ConfigurationElement.CreatePropertyBagFromType(Type type)
   at System.Configuration.ConfigurationElement.PropertiesFromType(Type type, ConfigurationPropertyCollection& result)
   at System.Configuration.ConfigurationElement.get_Properties()
   at System.Configuration.ConfigurationElement.set_Item(String propertyName, Object value)
   at Fido.SnmpPollerServer.Configuration.HostElement.set_HostName(String value) in F:\My Projects\WebHosting.net\Fido\Fido.CoreServers\SnmpPollerServer\Configuration\HostElement.cs:line 28
   at Fido.CoreServers.Test.SnmpPollerServer.ConfigurationTests.CanAllPropertiesBeWrittenAndReadProperlyForHostElement() in F:\My Projects\WebHosting.net\Fido\Fido.CoreServers.Test\SnmpPollerServer\ConfigurationTests.cs:line 50

Answer 1

typical...

A few moments after I posted this I looked at the stack trace again. Oops... framework bug here...

at System.Configuration.ConfigurationProperty.SetDefaultValue(Object value)

made me think that perhaps the internal property was getting set to 0 and the validator was looking at this and kicking it back as invalid, so I changed my attribute to have a default value of 1 and the test worked.

 

Answer 2

if you look at the call stack, it's validating during set_HostName.

but at that point in your code, you haven't set the SnmpVersion property yet, so it is set to the default value at that point.

i'd suggest setting the version first and then setting the host name.   if you're still getting problems, you can use Reflector to look at the actual code to see what it's doing.

Answer 3

I'm sorry Kirk, but this is a bug. Look at my attributes again. I'm saying

A: The property is required.
b: The property must be greater then 1
c: The property must be less than 3

This means that if a person accuratly completes my configuration file, if I don't explicitly set a default value it will fail. That's a bug. The usage parameters I'm setting are enough to tell the end user what *they* need to make the library work.

By requiring me to put a default value there, an end user can ignore the tag, the runtime will fill in the default and I could have a scenario where my code is decided for the user what a value would be should they improperly fill out the configuration section.

If you extract just the property in question and *accuratly* fill out the configuration file, without having a DefaultValue= on the attribute the code fails and a misleading exception is raised.

Furthermore is you reivew the trace again, what it happening is

a: I'm setting the property directly
b: the runtime determines there is no value so it creates a default
c: since no value is determined by the attribute and this is a value type it defaults to 0
d:it then attempts to set this value *with* validation

The problem is that *only* the CLR knows anything about this value being set. I, as the developer, don't want 0 to *ever* be a set value.

They should be not validating value type defaults that are not explicitly assigned via the DefaultValue= attribute before a user supplied value is provided.

There is no way around it. It's a sloppy design built around requirements (we need to validate stuff) and not built around scenarios...

Answer 4

Your right on Paul, but the sillyness is not limited to IntegerValidator.   Brian Button ran into basically the same issue using StringValidator a much simpler repro case and filed a bug on it FDBK29746.  

So far, the System.Configuration team has treated this as "By Design".   I've just added a vote for his bug, perhaps you will too.

Answer 5

The validator is working as expected. If you look at the call stack you would see that the exception is generated when the default value for the property is being applied ( the default value must be valid :)

We validate default values when the property bag is created which in the attributed model happens the first time the bag is being used ( which in your case happens to be the moment you try to change the property value ).

So the exception is really telling you your default value is not valid. Change the attribute to

[ConfigurationProperty(HostElement.Tags.SnmpVersion, IsRequired = true, DefaultValue=2)] and you are all set.

Regards
Ivo

Answer 6

Paul D. Murphy wrote:

I'm sorry Kirk, but this is a bug. Look at my attributes again. I'm saying A: The property is required. b: The property must be greater then 1 c: The property must be less than 3 This means that if a person accuratly completes my configuration file, if I don't explicitly set a default value it will fail. That's a bug. The usage parameters I'm setting are enough to tell the end user what *they* need to make the library work. By requiring me to put a default value there, an end user can ignore the tag, the runtime will fill in the default and I could have a scenario where my code is decided for the user what a value would be should they improperly fill out the configuration section. If you extract just the property in question and *accuratly* fill out the configuration file, without having a DefaultValue= on the attribute the code fails and a misleading exception is raised. Furthermore is you reivew the trace again, what it happening is a: I'm setting the property directly b: the runtime determines there is no value so it creates a default c: since no value is determined by the attribute and this is a value type it defaults to 0 d:it then attempts to set this value *with* validation The problem is that *only* the CLR knows anything about this value being set. I, as the developer, don't want 0 to *ever* be a set value. They should be not validating value type defaults that are not explicitly assigned via the DefaultValue= attribute before a user supplied value is provided. There is no way around it. It's a sloppy design built around requirements (we need to validate stuff) and not built around scenarios...

Paul,

During our development cycle we ran into a lot of configuration sections developed inside Microsoft which were setting default values which would be invalid or sometimes even not the right type for the property. Thats why we decided that if a property has a validator the default value MUST conform to this validator's requirements. This way if a section developer put something wrong as a default they would see the exception right away. Otherwise they may see it or they may not.

You have a point that in the case where the property is a required one the default value would never directly affect the current state of the property so we shouldn't run the validator. However the default value is exposed through the PropertyInformation.Default and can be accessed from within the ConfigurationElement derived class and it makes making code errors a lot harder if we ensure that this value is valid once and we do it as soon as possible.

Answer 7

Ivo Jeglov wrote:

The validator is working as expected. If you look at the call stack you would see that the exception is generated when the default value for the property is being applied ( the default value must be valid :) We validate default values when the property bag is created which in the attributed model happens the first time the bag is being used ( which in your case happens to be the moment you try to change the property value ). So the exception is really telling you your default value is not valid. Change the attribute to [ConfigurationProperty(HostElement.Tags.SnmpVersion, IsRequired = true, DefaultValue=2)] and you are all set. Regards Ivo

umm... no...

See I don't want the DefaultValue to be 2. I want the user to be forced to specify a value. If I put the default value at 2, they can put nothing and things will work.

Answer 8

I agree with Paul, the design is flawed. 

Requiring that a default value be validated by the input validator is logical and expected.  However, the problem is that the ConfigurationElement is setting default values for properties without having a default value assigned by the developer of the derived class, and THESE values cannot be guaranteed to be valid in regards to the validator.  The value 0 (zero) may be an integer, but it is not between 1 and 3.  Ivo recommends setting a valid devault value, but herein lies the true problem with the design.

In many cases, having a default value for a property is counter productive to requiring said property.   As Paul says, setting a default value enables the configuring user/developer to omit the tag attribute and rely on the assigned default value - thus the tag attribute is no longer required.  If there is a default value provided, there is no longer a requirement for the user to supply a value.

Perhaps this is a difference of perspective.  The concept using a ConfigurationPropertyAttribute to make a property of a ConfigurationElement "required" should mean that the XML tag in the config file requires an attribute of that name, it should not mean that the ConfigurationElement class itself requires a value for that property.   There is a difference.  Correct me if I'm wrong, but during a config file read a ConfigurationElement is created by the framework before it is deserialized from the file, so there is a time when the properties may be empty, nothing, null, zero, but since they have not yet had a "set" attempted there is no invalid input.  

I haven't checked on this myself, but what happens when the property of a ConfigurationElement is an object?  Does the ConfigurationElement base constructor use the TypeConverter to attempt to create an object as a default value if I don't supply one?  What if my custom validator for that property can correctly validate my object type, but can't validate the default that the ConfigurationElement tries to assign?

Aside from this flaw, I also feel that the documentation on the 2.0 ConfigurationElement derived classes is extremely thin.  For example, nowhere in the documentation did I find that you need to set the ConfigurationPropertyAttribute name to an empty string to allow a collection's parent node to be omitted from the XML.  (Thank you, Google.)  Aslo, there are numerous places where "The following is an example" is not followed by an example.

The new configuration model for .NET 2.0 is powerful, but still contains minor flaws and is very poorly documented.

 

Useful links:

http://fredrik.nsquared2.com/viewpost.aspx?PostID=233

http://msdn2.microsoft.com/en-us/library/ms228062.aspx

http://blogs.msdn.com/markgabarra/archive/2006/06/27/648742.aspx

http://blogs.msdn.com/markgabarra/archive/2006/06/25/646775.aspx

http://jason.diamond.name/weblog/2006/05/25/custom-configuration-section-validator-weirdness

http://msdn.microsoft.com/msdnmag/issues/06/06/ConfigureThis/

http://www.agileprogrammer.com/oneagilecoder/archive/2005/06/11/3689.aspx