magiconair / properties

Java properties scanner for Go
BSD 2-Clause "Simplified" License
323 stars 77 forks source link

Bugs when properties are specified as metadata of struct #32

Open rohans81 opened 6 years ago

rohans81 commented 6 years ago

There are some bugs when we specify properties as part of the struct metadata. e.g

config file

graphite.host=localhost
graphite.metric.name=gtest

cloud.tenant.name=xxx-qa
cloud.keystone.url=https://xxx.zzz.ccc.aaa.xx/v2.0
cloud.swift.username=yyy-bb-ccc
cloud.swift.passwd=changeme

kafka.topic=ttt
kafka.servers=localhost:9092,zzz.dev.yyy.ca:9092
kafka.consumergroup=test
kafka.consumeFromStart=true

bulk.delete.max_wait=10s
log.level=debug
// Configuration holds external configuration for the consumers
type Configuration struct {
    CloudTenant           string              `properties:"cloud.tenant.name"`
    KeystoneURL           string              `properties:"cloud.keystone.url"`
    Username              string              `properties:"cloud.swift.username"`
    Userpwd               string              `properties:"cloud.swift.passwd"`
    Servers               []string            `properties:"kafka.servers"`
    Cgroupname            string              `properties:"kafka.consumergroup"`
    Topic                 string              `properties:"kafka.topic"`
    ConsumeFromStart      bool                `properties:"kafka.consumeFromStart,default=false"`
        KafkaVersion          sarama.KafkaVersion `properties:"kafka.version,default=0.11.0.2"`
    Port                  int                 `properties:"port,default=8383"`
    Graphite              string              `properties:"graphite.host, default=carbon.service.consul"`
    GraphiteMetricName    string              `properties:"graphite.metric.name"`
    LogLevel              string              `properties:"log.level,default=info"`
    BulkDeleteMaxItems    int                 `properties:"bulk.delete.max_items,default=100"`
    BulkDeleteMaxWaitTime time.Duration       `properties:"bulk.delete.max_wait,default=5s"`

}

It fails on the first field/property it has a issue within the struct. All fields after this filed (in struct order) are not processed. e.g if the field KafkaVersion could not be decoded then all fields below it will also not be decoded.

There are issues in following cases 1> The property is missing in property file and no default value is mentioned in the struct 2> if the field type is not supported by decoder eg (sarama.KafkaVersion) 3> if the fields in the struct do not have the property tag and a property with the field name is not present in the property file.

Is there any way we can exclude a field from decode logic? Regards Rohan

magiconair commented 6 years ago

Fields with properties:"-" are ignored but that would still be a bug.

magiconair commented 6 years ago

When I try this specific example Decode returns an error. Do you check all the errors?

Added this to decode_test.go

func TestDecodeSkipFail(t *testing.T) {
    type S struct {
        V sarama.KafkaVersion `properties:"kafka.version,default=0.11.0.2"`
        S string
    }
    in := `S=some value`
    out := &S{sarama.V0_11_0_2, "some value"}
    testDecode(t, in, &S{}, out)
}
--- FAIL: TestDecodeSkipFail (0.00s)
    decode_test.go:298: got cannot set version want nil
rohans81 commented 6 years ago

yes. I had missed checking the error. It would be helpful to add the details around ignoring properties to README file. Thanks.

magiconair commented 6 years ago

This should be in the godoc or isn’t it?

magiconair commented 6 years ago

https://godoc.org/github.com/magiconair/properties#Properties.Decode