hi adrian, yes - i moved too many classes during the last refactoring -> thx for the heads-up
@ converter-type logic: i haven't implemented too much of it, because it needs further discussions. @ custom converter: that's right i'll add missing todos for further discussions and afterwards i'll push it to our repository. furthermore, i already have javadoc and several tests for it - i'll push them in a 2nd step as soon as all of them are finished. regards, gerhard 2012/3/13 Adrian Gonzalez <[email protected]> > Hi Gerhard, > > Really nice ! > > Some remarks : > * you return null when config key isn't defined, I would prefer to raise > an exception. > At least by default - if you really want to allow optional > configuration keys, I would add an optional attribute in ConfigProperty. > Otherwise we'll have in most case a NPE further down in our apps. > * missing eager attribute in ConfigProperty. > * for Converter/ConverterFactory : with the current impl, a given > converter can map from only one type to another. > > * can this work for javabean to javabean converter ? (converter will > be registered to convert from Object to Object - so calling > ConverterService.create(A.class, B.class) will fail) > * can I code a single converter doing conversion String to 0..n types > ? > * ConverterFactory.create should raise an exception if no converter is > found. > * ConverterExtension should be renamed ConfigExtension ? > * ConverterKey, DefaultConverterFactory, StringToIntegerConverter should > be in a converter package, not in config. > > Just to be sure : > * about converter precedence : if I code a custom converter in my app, it > will take precedence of the default ones (those provided by Deltaspike) : > correct ? > > Further use cases are based on more out of the box conversion > possibilities : > * String to any primitive / wrapper type. > * String to Locale ? > * String to Collection<X> ? > > Regards, > > P.S. understood about 3rd party projects ;) > > > ----- Mail original ----- > De : Gerhard Petracek <[email protected]> > À : [email protected] > Cc : > Envoyé le : Mardi 13 mars 2012 19h27 > Objet : Re: Re : Re : ConfigResolver : adding @ConfigProperty injection ? > > hi adrian, > > at [1] you can find my >first< draft including some examples. the current > draft supports everything we have discussed including 1-2 additional > features. > so far i haven't looked at your suggestion to avoid that we get in > implementations of spring. > -> i would suggest that you check e.g. the examples -> if needed we can > discuss further use-cases (which might be supported by your draft already). > > for the future: > please never use source-code of 3rd party projects (= projects outside of: > apache and officially donated projects) > -> we can avoid such a redundant effort with other features. > > regards, > gerhard > > [1] http://s.apache.org/Adk > > > > 2012/3/13 Gerhard Petracek <[email protected]> > > > hi adrian, > > > > thx for starting with it! i'll have a look at it tomorrow. > > fyi: we don't copy parts of other projects without official > > permission/donation. > > > > regards, > > gerhard > > > > > > > > 2012/3/13 Adrian Gonzalez <[email protected]> > > > >> Hello, > >> > >> I've implemented ConfigProperty feature [1] > >> Not sure if it's fine, I'm just a CDI newbie. > >> I just hope this can help. > >> My notes are here : [2] > >> > >> > >> Best regards, > >> > >> [1] > https://github.com/gonzalad/incubator-deltaspike/tree/DELTASPIKE-114 > >> [2] notes : > >> > >> 1. ConfigProperty > >> Implemented both approaches : @ConfigProperty and custom annotation. > >> Some questions : > >> * for now I'm throwing an exception if property is missing. > >> I think this is the right way to go (I don't want to have a > >> NullPointerException somewhere after). > >> But perhaps we can add a required attribute in ConfigProperty. > >> * implemented hack for converter attribute which should be optionnal > and > >> default value should be null. > >> Needed to create special class for null value (annotation attribute > >> default value cannot be null). > >> * not sure about equals impl for ConfigPropertyBean > >> * is there a simpler approach for ConfigProperty implementation ? > >> For now I have an extension observing ProcessInjectionTarget > >> and AfterBeanDiscovery. > >> * in ProcessInjectionTarget it scans every injectionPoint looking at > >> @ConfigAttribute or at an annotation annotated with @ConfigAttribute > >> if found ti stores this injectionPoint in a list. > >> * in AfterBeanDiscovery, it adds a CDI bean for every item > >> (configAttribute) in the previous list. > >> Could a parameterized producer be used instead ? Don't think so, > but... > >> * FYI : I think unit tests for extension can collide (i.e Excludes and > >> ConfigPropertyExtension). > >> > >> 2. Converters > >> An explicit converter class can be set with converter attribute. > >> Otherwise, we use default conversion service (should handle any > primitive > >> or wrapper type conversion). > >> The explicit converter must implement Converter interface (in api). > >> ConversionService is packaged in impl for now. > >> I'm not too happy about conversionService. > >> It's a brutal copy/paste from Spring (I removed without reading all that > >> - I was really in a hurry) > >> Not happy about it. > >> > >> 3 options then : > >> * we try a really minimal implementation (basically > >> hashmap<KeyPair<sourceClass,targetClass>,Converter>). > >> * we try to implement it well and completely - IMO this will be long. > >> * we copy / paste all Spring conversion code - soso. > >> > >> Some questions : > >> * some code has been copy/paste from Spring > >> (i.e. StringToBooleanConverter) : donno how to do. > >> I've left initial authors (of course !), but changed header file. > >> Donno if you want to change code impl, or if I need to contact Spring... > >> > >> > >> > >> > >> ________________________________ > >> De : Gerhard Petracek <[email protected]> > >> À : [email protected] > >> Cc : Pete Muir <[email protected]> > >> Envoyé le : Jeudi 8 mars 2012 14h26 > >> Objet : Re: Re : ConfigResolver : adding @ConfigProperty injection ? > >> > >> hi adrian, > >> > >> @ #2: > >> since it's the easiest approach and i would have suggested the same > >> (independent of other libs): +1 for the interface > >> right now we just have an use-case for specifying the converter manually > >> -> > >> we can add e.g. a ConverterFactory later on (as soon as we need it for > an > >> use-case). > >> > >> @ #4: > >> if i remember correctly, someone in an open-source community mentioned > >> such > >> a plan (for the "near" future). > >> > >> regards, > >> gerhard > >> > >> > >> > >> 2012/3/7 Adrian Gonzalez <[email protected]> > >> > >> > Noticed the JIRA issues for this issue, thanks Gerhard ! > >> > Sorry for the delay (quite struggling in my day to day work for now > ;( ) > >> > > >> > > >> > About conversion API (for converter = StringToIntegerConverter.class), > >> > just googled a bit : > >> > 1. there's the javabean specification (soso...) > >> > 2. Spring has a really nice conversion API : > >> > > >> > http://static.springsource.org/spring/docs/3.1.x/spring-framework-reference/html/validation.html#core-convert > >> > built on top of : > >> > public interface Converter<S, T> { > >> > T convert(S source); > >> > } > >> > the upper layers are ConversionService, GenericConverter - not > sure > >> if > >> > it's too early to introduce similar functionnality > >> > 3. of course JSF converters but really tied to JSF (soso...) > >> > 4. didn't found any info about the official Conversion JSR > >> > > >> > > >> > > >> > ________________________________ > >> > De : Jason Porter <[email protected]> > >> > À : [email protected] > >> > Cc : Adrian Gonzalez <[email protected]> > >> > Envoyé le : Mardi 6 mars 2012 22h25 > >> > Objet : Re: ConfigResolver : adding @ConfigProperty injection ? > >> > > >> > +1 to the idea. I also think some more discussion about this is in > >> order. > >> > > >> > On Tue, Mar 6, 2012 at 04:09, Pete Muir <[email protected]> wrote: > >> > > >> > > +1 > >> > > > >> > > On 5 Mar 2012, at 16:17, Adrian Gonzalez wrote: > >> > > > >> > > > What about having both options ? > >> > > > > >> > > > 1. be able to use directly @ConfigProperty > >> > > > @Produces > >> > > > public LoginContext > >> > > > produceLoginContext(@ConfigProperty("loginConfigFile") String > >> > > loginConfigFileName, @ConfigProperty("loginModuleName") String > >> > > loginModuleName) > >> > > > } > >> > > > } > >> > > > Handy when all config values are only used in a centralized class > >> (e.g. > >> > > MyAppConfig). > >> > > > > >> > > > 2. type safe config annotations > >> > > > @ConfigProperty( > >> > > > name = "pool_size", > >> > > > eager = true, //true is also the default value -> the value gets > >> > > converted during the bootstrapping process > >> > > > converter = StringToIntegerConverter.class > >> > > > ) > >> > > > public @interface PoolSize { > >> > > > } > >> > > > > >> > > > ----- Mail original ----- > >> > > > De : Gerhard Petracek <[email protected]> > >> > > > À : [email protected] > >> > > > Cc : > >> > > > Envoyé le : Lundi 5 mars 2012 17h08 > >> > > > Objet : Re: Re : ConfigResolver : adding @ConfigProperty > injection ? > >> > > > > >> > > > hi adrian, > >> > > > > >> > > > @#1: > >> > > > it isn't the default value of the property. > >> > > > eager should be true by default -> the configured value gets > >> converted > >> > > > during bootstrapping (if the value has an invalid format the > >> > > bootstrapping > >> > > > process fails). > >> > > > if eager is false, the configured value will be converted directly > >> > before > >> > > > the injection (e.g. for values stored in dynamic config-sources). > >> > > > > >> > > > @#2: > >> > > > afaik there is an upcoming jsr about it. > >> > > > > >> > > > regards, > >> > > > gerhard > >> > > > > >> > > > > >> > > > > >> > > > 2012/3/5 Adrian Gonzalez <[email protected]> > >> > > > > >> > > >> Hi Gerhard, > >> > > >> > >> > > >> 1. didn't understood the meaning of eager attribute > >> > > >> If eager = default value, it should have been some integer value > in > >> > your > >> > > >> sample > >> > > >> > >> > > >> > >> > > >> 2. StringToIntegerConverter converter > >> > > >> Is there an existing conversion API (standard, in deltaspike, > ...) > >> ? > >> > > >> (otherwise we're gonna need one here - and end up with another > >> > > conversion > >> > > >> API ;) ) > >> > > >> > >> > > >> Thanks ! > >> > > >> > >> > > >> ----- Mail original ----- > >> > > >> De : Gerhard Petracek <[email protected]> > >> > > >> À : [email protected] > >> > > >> Cc : > >> > > >> Envoyé le : Lundi 5 mars 2012 12h57 > >> > > >> Objet : Re: ConfigResolver : adding @ConfigProperty injection ? > >> > > >> > >> > > >> hi adrian, > >> > > >> > >> > > >> if we agree also on adding a bit more to allow e.g.: > >> > > >> > >> > > >> //... > >> > > >> @ConfigProperty( > >> > > >> name = "pool_size", > >> > > >> eager = true, //true is also the default value -> the value > gets > >> > > >> converted during the bootstrapping process > >> > > >> converter = StringToIntegerConverter.class > >> > > >> ) > >> > > >> public @interface PoolSize > >> > > >> { > >> > > >> } > >> > > >> > >> > > >> @Inject > >> > > >> @PoolSize > >> > > >> private int configuredPoolSize; > >> > > >> > >> > > >> then i would vote +1 > >> > > >> (for sure the details need further discussions.) > >> > > >> > >> > > >> regards, > >> > > >> gerhard > >> > > >> > >> > > >> > >> > > >> > >> > > >> 2012/3/5 Adrian Gonzalez <[email protected]> > >> > > >> > >> > > >>> Hello, > >> > > >>> > >> > > >>> Deltaspike config module is based on ConfigResolver usage : > >> > > >>> ConfigResolver.getPropertyValue("test") > >> > > >>> > >> > > >>> > >> > > >>> Wouldn't it be interesting to add on top of it some injection > >> > > >>> capacity ? (i.e. providing @ConfigProperty annotation) > >> > > >>> > >> > > >>> Sample usage [1] : > >> > > >>> @Produces > >> > > >>> public LoginContext > >> > > >> produceLoginContext(@ConfigProperty("loginConfigFile") > >> > > >>> String loginConfigFileName, > >> > > >>> > >> > > >> @ConfigProperty("loginModuleName") > >> > > >>> String loginModuleName) > >> > > >>> blabla > >> > > >>> } > >> > > >>> > >> > > >>> This approach is based on Antonio's petstore application - > config > >> > code > >> > > is > >> > > >>> available in [2] > >> > > >>> > >> > > >>> [1] > >> > > >>> > >> > > >> > >> > > > >> > > >> > https://github.com/agoncal/agoncal-application-petstore-ee6/blob/master/src/main/java/org/agoncal/application/petstore/security/LoginContextProducer.java > >> > > >>> > >> > > >>> [2] > >> > > >>> > >> > > >> > >> > > > >> > > >> > https://github.com/agoncal/agoncal-application-petstore-ee6/blob/master/src/main/java/org/agoncal/application/petstore/util/ConfigProperty.java > >> > > >>> > >> > > >>> > >> > > >> > >> > > > >> > > >> > https://github.com/agoncal/agoncal-application-petstore-ee6/blob/master/src/main/java/org/agoncal/application/petstore/util/ConfigPropertyProducer.java > >> > > >>> > >> > > >>> > >> > > >> > >> > > >> > >> > > > > >> > > > >> > > > >> > > >> > > >> > -- > >> > Jason Porter > >> > http://lightguard-jp.blogspot.com > >> > http://twitter.com/lightguardjp > >> > > >> > Software Engineer > >> > Open Source Advocate > >> > Author of Seam Catch - Next Generation Java Exception Handling > >> > > >> > PGP key id: 926CCFF5 > >> > PGP key available at: keyserver.net, pgp.mit.edu > >> > > >> > > > > > >
