Skip to content

Commit

Permalink
check-config: Produce error when using scoped properties
Browse files Browse the repository at this point in the history
In the confusion of scoped properties, some sites have used the
scoping operator in their local configuration (probably not realizing
it was an operator and treating it as if it was part of the name).

When upgrading to 2.10, those sites will not receive any warnings or
errors about such definitions, as only the unscoped names are marked
obsolete/deprecated/forbidden. dCache does output an info level message
saying that dCache itself doesn't use the property, but sites tend
to ignore that message (based on a sample size of one).

This patch resolves this by flagging scoped properties as errors.
This prevents a slash from being used in a property name, but given
the history of scoped properties, it is probably wise to disallow
that symbol for some years.

Demo:

    $ packages/system-test/target/dcache/bin/dcache check-config
    [ERROR] dcache.conf:2: Property xrootd/xrootdPlugins is a scoped property.
                           Scoped properties are no longer supported.
    [INFO] system-test.conf:1: Property system-test.home is not a standard
                               property
    Found 1 error.

Target: trunk
Request: 2.10
Request: 2.9
Request: 2.8
Request: 2.7
Require-notes: yes
Require-book: yes
Acked-by: Karsten Schwank <karsten.schwank@desy.de>
Acked-by: Paul Millar <paul.millar@desy.de>
Patch: https://rb.dcache.org/r/7401/
(cherry picked from commit f455fdc)
  • Loading branch information
gbehrmann committed Oct 27, 2014
1 parent 190e28a commit d2fe485
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 0 deletions.
Expand Up @@ -271,6 +271,9 @@ protected void checkIsAllowed(AnnotatedKey key, String value)
if (existingKey != null) {
checkKeyValid(existingKey, key);
checkDataValid(existingKey, value);
} else if (name.indexOf('/') > -1) {
_problemConsumer.error(
"Property " + name + " is a scoped property. Scoped properties are no longer supported.");
} else if (!_usageChecker.isStandardProperty(defaults, name)) {
// TODO: It would be nice if we could check whether the property is actually
// used, ie if it appears as part of the value of a standard property. To do this
Expand Down
Expand Up @@ -93,6 +93,8 @@ public class ConfigurationPropertiesTests {

private static final String EXPANDING_PROPERTY_NAME = "expanding-key";

private static final String SCOPED_PROPERTY_NAME = "scoped/property";

private ConfigurationProperties _properties;
private ConfigurationProperties _initiallyEmptyProperties;
private ConfigurationProperties _standardProperties;
Expand Down Expand Up @@ -295,6 +297,11 @@ public void testForbiddenWithErrorPropertyPut() {
}
}

@Test(expected = IllegalArgumentException.class)
public void testScopedPropertyPut() {
_properties.put(SCOPED_PROPERTY_NAME, "some value");
}

@Test
public void testStringPropertyNames()
{
Expand Down

0 comments on commit d2fe485

Please sign in to comment.