[GitHub] kafka pull request #3995: KAFKA-5514 KafkaConsumer ignores default values in...

[Galus <gi...@git.apache.org>]

GitHub user Galus opened a pull request:

    https://github.com/apache/kafka/pull/3995

    KAFKA-5514 KafkaConsumer ignores default values in Properties object because of incorrect use of Properties object.

    This problem plagues anywhere Properties are being misused. I have this issue with KafkaProducer(Properties properties) constructor. I bet this problem spans across more classes just like this KafkaConsumer(Properties properties).
    
    Clarification:
    
    putAll is implemented in Hashtable.java and at that point we are already too late.
    When we cast to Map/Hashtable we lose  Properties's 'defaults member which is another Properties object. Whenever you use Properties's getProperty() method, if the Property object does not explicity contain the property, it will refer to it's internal defaults Properties object which can be set as so:
    Properties myProps = new Properties(oldProperties)
    if oldProperties has a key:value of foo:bar
    calling myProps.get(foo) will not return bar
    whereas, calling myProps.getProperty(foo) will return bar
    
    Reference:
    
    
    ```
    public
    class Properties extends Hashtable<Object,Object> {
    ...
        /**
         * A property list that contains default values for any keys not
         * found in this property list.
         *
         * @serial
         */
        protected Properties defaults;
    
        /**
         * Creates an empty property list with the specified defaults.
         *
         * @param   defaults   the defaults.
         */
        public Properties(Properties defaults) {
            this.defaults = defaults;
        }
    ...
        public String getProperty(String key) {
            Object oval = super.get(key);
            String sval = (oval instanceof String) ? (String)oval : null;
            return ((sval == null) && (defaults != null)) ? defaults.getProperty(key) : sval;
        }
    ...
    ```
    
    
    
    Here is the fix:
    
    ```
            for (final String name: properties.stringPropertyNames()) {
                if (properties.get(name) == null) {
                    map.put(name, properties.getProperty(name));
                }
            }
    ```
    
    
    This pulls all the 'default' inner Properties object's entries up into the outer level Properties object.
    This should be applied to all the public methods that are being passed in a Properties object
    See https://github.com/Galus/kafka/commit/8af611dfd9c8f3cb37f1d2b400890525c5a646d3 and https://github.com/Galus/kafka/commit/130b4cde2da4d9ede7aa6d6a99997af32d1a00fc
    
    
    Similar Issues addressing this issue: KAFKA-2184, KAFKA-3049, KAFKA-3161
    @Kamal15 
    
    This contribution is my original work and I hereby license it for use by the apache kafka project's open source license.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/Galus/kafka trunk

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/kafka/pull/3995.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #3995
    
----
commit 8053f294ff938c6cc295b42f8afa6bec2eb5dc75
Author: Galus <mg...@gmail.com>
Date:   2017-09-29T19:15:07Z

    KAFKA-5514 Better Handling Of Properties
    
    KafkaProducer update.

commit 8af611dfd9c8f3cb37f1d2b400890525c5a646d3
Author: Galus <mg...@gmail.com>
Date:   2017-09-29T19:29:34Z

    KAFKA-5514 Better Properties Handling
    
    Fixes KafkaConsumer's inability to handle Properties object's inner Properties object 'defaults'

commit 130b4cde2da4d9ede7aa6d6a99997af32d1a00fc
Author: Galus <mg...@gmail.com>
Date:   2017-09-29T19:31:56Z

    KAFKA-5514 Better Properties Handling
    
    Fixes KafkaProducer's inability to handle Properties object's inner Properties object 'defaults'

----


---