commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Elijah Zupancic <eli...@zupancic.name>
Subject Re: [chain][v2] chain-58
Date Sun, 27 Nov 2011 20:23:02 GMT
Hi Everyone,

After a bunch of work, I was able to genericize chain into using contexts
with the signature Context<K, V> as per Paul's suggestion. It took sweeping
code changes to the chain codebase. Please see the patch attached to the
issue: https://issues.apache.org/jira/browse/CHAIN-58

The primary approach that I took was to assume that all functionality
outside of the core Context interface are implemented as Context<String,
Object>. This allowed me to build off of the current contract. Now, the
library builds with no warnings and all SuppressWarnings("unchecked") are
commented. Although their comments are often just stating that we are
following the existing contract.

I realize that this will take a while to review, but my hope is that these
changes will please all parties who felt that the changes to the Context
generic's signature were too drastic.

Please let me know what you think.

After this, I would love to get onto removing deprecated methods,
genericizing the unit tests and breaking apart the portlet, servlet, etc
code into different maven packages.

Thanks,
-Elijah

On Tue, Sep 20, 2011 at 1:22 AM, Simone Tripodi <simonetripodi@apache.org>wrote:

> Hi Elijah,
> by default Maven doesn't show warnings, you have to modify the pom.xml
> in that way:
>
> {code}
>            <plugin>
>                <groupId>org.apache.maven.plugins</groupId>
>                <artifactId>maven-compiler-plugin</artifactId>
>                <version>2.1</version>
>                <configuration>
>                    <source>${maven.compile.source}</source>
>                    <target>${maven.compile.target}</target>
>                    <compilerArgument>-Xlint:all</compilerArgument>
>                    <showWarnings>true</showWarnings>
>                </configuration>
>            </plugin>
> {code}
>
> If you try to run {{mvn clean compile}} to vanilla [chain] code, you
> can already notice some warnings:
>
> $ mvn --version
> Apache Maven 3.0.3 (r1075438; 2011-02-28 18:31:09+0100)
> Maven home: /Applications/apache-maven-3.0.3
> Java version: 1.5.0_30, vendor: Apple Inc.
> Java home: /System/Library/Frameworks/JavaVM.framework/Versions/1.5.0/Home
> Default locale: en_US, platform encoding: MacRoman
> OS name: "mac os x", version: "10.7.1", arch: "i386", family: "unix"
>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> $ mvn clean compile
>
> [INFO] Compiling 63 source files to
> /Users/simonetripodi/Documents/workspace/commons-chain/target/classes
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/impl/ContextBase.java:[633,46]
> [unchecked] unchecked conversion
> found   : java.util.Map.Entry
> required: java.util.Map.Entry<java.lang.String,java.lang.Object>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/impl/ContextBase.java:[652,76]
> [unchecked] unchecked cast
> found   : java.lang.Object
> required: java.util.Map.Entry<java.lang.String,java.lang.Object>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/impl/ContextBase.java:[779,48]
> [unchecked] unchecked conversion
> found   : java.util.Map.Entry
> required: java.util.Map.Entry<java.lang.String,java.lang.Object>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/faces/FacesWebContext.java:[133,58]
> [unchecked] unchecked conversion
> found   : java.util.Map
> required: java.util.Map<java.lang.String,java.lang.Object>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/faces/FacesWebContext.java:[133,11]
> [unchecked] unchecked conversion
> found   : java.util.Map
> required: java.util.Map<java.lang.String,java.lang.Object>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/faces/FacesWebContext.java:[145,60]
> [unchecked] unchecked conversion
> found   : java.util.Map
> required: java.util.Map<java.lang.String,java.lang.String>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/faces/FacesWebContext.java:[145,11]
> [unchecked] unchecked conversion
> found   : java.util.Map
> required: java.util.Map<java.lang.String,java.lang.String>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/faces/FacesWebContext.java:[157,66]
> [unchecked] unchecked conversion
> found   : java.util.Map
> required: java.util.Map<java.lang.String,java.lang.String[]>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/faces/FacesWebContext.java:[157,11]
> [unchecked] unchecked conversion
> found   : java.util.Map
> required: java.util.Map<java.lang.String,java.lang.String[]>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/faces/FacesWebContext.java:[169,60]
> [unchecked] unchecked conversion
> found   : java.util.Map
> required: java.util.Map<java.lang.String,java.lang.String>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/faces/FacesWebContext.java:[169,11]
> [unchecked] unchecked conversion
> found   : java.util.Map
> required: java.util.Map<java.lang.String,java.lang.String>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/faces/FacesWebContext.java:[181,63]
> [unchecked] unchecked conversion
> found   : java.util.Map
> required: java.util.Map<java.lang.String,java.lang.String>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/faces/FacesWebContext.java:[181,11]
> [unchecked] unchecked conversion
> found   : java.util.Map
> required: java.util.Map<java.lang.String,java.lang.String>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/faces/FacesWebContext.java:[193,69]
> [unchecked] unchecked conversion
> found   : java.util.Map
> required: java.util.Map<java.lang.String,java.lang.String[]>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/faces/FacesWebContext.java:[193,11]
> [unchecked] unchecked conversion
> found   : java.util.Map
> required: java.util.Map<java.lang.String,java.lang.String[]>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/faces/FacesWebContext.java:[207,64]
> [unchecked] unchecked conversion
> found   : java.util.Map
> required: java.util.Map<java.lang.String,java.lang.Object>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/faces/FacesWebContext.java:[220,24]
> [unchecked] unchecked conversion
> found   : java.util.Map
> required: java.util.Map<java.lang.String,javax.servlet.http.Cookie>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/faces/FacesWebContext.java:[219,70]
> [unchecked] unchecked method invocation:
> <K,V>checkedMap(java.util.Map<K,V>,java.lang.Class<K>,java.lang.Class<V>)
> in java.util.Collections is applied to
>
> (java.util.Map,java.lang.Class<java.lang.String>,java.lang.Class<javax.servlet.http.Cookie>)
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/faces/FacesWebContext.java:[219,70]
> [unchecked] unchecked conversion
> found   : java.util.Map
> required: java.util.Map<java.lang.String,javax.servlet.http.Cookie>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/faces/FacesWebContext.java:[229,30]
> [unchecked] unchecked conversion
> found   : java.util.Map
> required: java.util.Map<java.lang.String,javax.servlet.http.Cookie>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/faces/FacesWebContext.java:[241,54]
> [unchecked] unchecked conversion
> found   : java.util.Map
> required: java.util.Map<java.lang.String,java.lang.Object>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/faces/FacesWebContext.java:[241,11]
> [unchecked] unchecked conversion
> found   : java.util.Map
> required: java.util.Map<java.lang.String,java.lang.Object>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/faces/FacesWebContext.java:[253,54]
> [unchecked] unchecked conversion
> found   : java.util.Map
> required: java.util.Map<java.lang.String,java.lang.Object>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/faces/FacesWebContext.java:[253,11]
> [unchecked] unchecked conversion
> found   : java.util.Map
> required: java.util.Map<java.lang.String,java.lang.Object>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/servlet/RequestParameterMapper.java:[76,16]
> [dep-ann] deprecated name isnt annotated with @Deprecated
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/servlet/RequestParameterMapper.java:[92,18]
> [dep-ann] deprecated name isnt annotated with @Deprecated
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/ChainListener.java:[304,17]
> [dep-ann] deprecated name isnt annotated with @Deprecated
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/config/ConfigParser.java:[159,16]
> [dep-ann] deprecated name isnt annotated with @Deprecated
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/ChainResources.java:[105,16]
> [dep-ann] deprecated name isnt annotated with @Deprecated
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/ChainResources.java:[191,16]
> [dep-ann] deprecated name isnt annotated with @Deprecated
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/config/ConfigRegisterRule.java:[101,55]
> [unchecked] unchecked conversion
> found   : org.apache.commons.chain.Command
> required: org.apache.commons.chain.Command<C>
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/config/ConfigRegisterRule.java:[101,43]
> [unchecked] unchecked method invocation:
> <C>addCommand(java.lang.String,org.apache.commons.chain.Command<C>) in
> org.apache.commons.chain.Catalog is applied to
> (java.lang.String,org.apache.commons.chain.Command)
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/config/ConfigRegisterRule.java:[104,37]
> [unchecked] unchecked call to
> addCommand(org.apache.commons.chain.Command<C>) as a member of the raw
> type org.apache.commons.chain.Chain
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/servlet/ServletPathMapper.java:[61,18]
> [dep-ann] deprecated name isnt annotated with @Deprecated
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/servlet/ServletPathMapper.java:[77,16]
> [dep-ann] deprecated name isnt annotated with @Deprecated
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/servlet/PathInfoMapper.java:[61,18]
> [dep-ann] deprecated name isnt annotated with @Deprecated
>
> [WARNING]
> /Users/simonetripodi/Documents/workspace/commons-chain/src/main/java/org/apache/commons/chain/web/servlet/PathInfoMapper.java:[77,16]
> [dep-ann] deprecated name isnt annotated with @Deprecated
>
> HTH, have a nice day!!!
> All the best,
> Simo
>
> http://people.apache.org/~simonetripodi/
> http://www.99soft.org/
>
>
>
> On Mon, Sep 19, 2011 at 11:56 PM, Elijah Zupancic <elijah@zupancic.name>
> wrote:
> > Hi Simo,
> >
> > Funny, I don't believe think that compiler complained at all. I will
> double
> > check soon and try some other compilers and let you know soon.
> >
> > Thanks,
> > -Elijah
> >
> > On Mon, Sep 19, 2011 at 8:12 AM, Simone Tripodi <
> simonetripodi@apache.org>wrote:
> >
> >> Hi Elijah,
> >> I had e deeper look at your patch and raw more carefully your message,
> >> I just have a BIG trouble: when changing the Context interface to
> >>
> >> public interface Context<K, V> extends Map<K, V> {
> >>
> >> }
> >>
> >> you can easily note that, in Command interface (just to mention one)
> >> compiler complains:
> >>
> >>    "Context is a raw type. References to generic type Context<K, V>
> >> should be parametrized"
> >>
> >> and we can not just ignore it... that's why I thought that adopting
> >> the signature Command<K, V, C extends Context<K, V>> in the command
> >> interface is not nice but needed.
> >> More thoughts?
> >> Simo
> >>
> >> http://people.apache.org/~simonetripodi/
> >> http://www.99soft.org/
> >>
> >>
> >>
> >> On Mon, Sep 19, 2011 at 10:01 AM, Simone Tripodi
> >> <simonetripodi@apache.org> wrote:
> >> > Hi Elijah!
> >> > great report, thanks for your effort! :)
> >> > I'll have a look at your patch as soon as I get a spare time slot,
> >> > I'll let you know ASAP!
> >> > Have a nice day, all the best!
> >> > Simo
> >> >
> >> > http://people.apache.org/~simonetripodi/
> >> > http://www.99soft.org/
> >> >
> >> >
> >> >
> >> > On Mon, Sep 19, 2011 at 3:52 AM, Elijah Zupancic <
> elijah@zupancic.name>
> >> wrote:
> >> >> I just submitted a patch to jira as CHAIN-58. This changes Context
to
> >> >> Context<K,V>.
> >> >>
> >> >> Thanks,
> >> >> -Elijah
> >> >>
> >> >> On Fri, Sep 16, 2011 at 2:16 PM, Simone Tripodi <
> >> simonetripodi@apache.org>wrote:
> >> >>
> >> >>> Hi Paul!
> >> >>> yes it can be done, of course :) I'm not convinced anyway by the
> heavy
> >> >>> notation that, modifying the Context, would impact the Command
and
> >> >>> Filter classes. I think it is because just a matter of taste :P
> >> >>> Feedbacks/suggestions/patches are welcome, if you want to provide
a
> >> >>> solution feel free to fill an issue and attach a patch!!
> >> >>> TIA, all the best!
> >> >>> Simo
> >> >>>
> >> >>> http://people.apache.org/~simonetripodi/
> >> >>> http://www.99soft.org/
> >> >>>
> >> >>>
> >> >>>
> >> >>> On Fri, Sep 16, 2011 at 11:05 PM, Paul Benedict <
> pbenedict@apache.org>
> >> >>> wrote:
> >> >>> > The basic context should be Context<K, V> and then use
interface
> >> >>> > composition to define other things like:
> >> >>> >
> >> >>> > public interface PropertyContext extends Context<String,
Object>,
> >> >>> > Map<String, Object>
> >> >>> >
> >> >>> > It can be done... I think :-)
> >> >>> >
> >> >>> > Paul
> >> >>> >
> >> >>> > On Fri, Sep 16, 2011 at 3:40 PM, Simone Tripodi
> >> >>> > <simonetripodi@apache.org> wrote:
> >> >>> >> Hi Elijah,
> >> >>> >> I spent some spare time trying to figure out how to improve
the
> >> >>> >> Context design, I didn't have a lot of success anyway
:(
> >> >>> >>
> >> >>> >>  * dropping the Map inheritance makes not easy maintaining
the
> >> classes
> >> >>> >> in the 'generic' package;
> >> >>> >>  * adding generics in the Context to specify K,V types,
makes all
> >> the
> >> >>> >> rest of the notation not so nice (IMHO), take a look as
a sample
> a
> >> >>> >> Command<K, V, C extends Context<K, V>> :?
> >> >>> >>
> >> >>> >> Do you have more ideas?
> >> >>> >> Many thanks in advance, all the best!
> >> >>> >> Simo
> >> >>> >>
> >> >>> >> http://people.apache.org/~simonetripodi/
> >> >>> >> http://www.99soft.org/
> >> >>> >>
> >> >>> >>
> >> >>> >>
> >> >>> >> On Wed, Sep 14, 2011 at 4:12 AM, Elijah Zupancic <
> >> elijah@zupancic.name>
> >> >>> wrote:
> >> >>> >>> Hi Everyone,
> >> >>> >>>
> >> >>> >>> I don't have any votes as I'm not a commiter, but
I would still
> >> like to
> >> >>> add
> >> >>> >>> in my suggestion.
> >> >>> >>>
> >> >>> >>> After our previous exchange, I'm of the mind that
we should use
> the
> >> >>> second
> >> >>> >>> option - that is be collection agnostic and work by
> composition. I
> >> may
> >> >>> be
> >> >>> >>> biased towards defined getters and setters, but I
really like
> to be
> >> >>> able to
> >> >>> >>> use auto-complete, automatic code refactoring tools
and static
> code
> >> >>> analysis
> >> >>> >>> tools. If we used only a Map, then the contract for
a context
> >> becomes a
> >> >>> >>> black box of anything. I like the way it is now where
you have
> to
> >> >>> implement
> >> >>> >>> a Map on your context or extend ContextBase. I may
be biased
> out of
> >> >>> habit -
> >> >>> >>> if so, please convince me (by proxy everyone else).
> >> >>> >>>
> >> >>> >>> Thanks,
> >> >>> >>> -Elijah
> >> >>> >>>
> >> >>> >>> On Mon, Sep 12, 2011 at 12:04 AM, Simone Tripodi
> >> >>> >>> <simonetripodi@apache.org>wrote:
> >> >>> >>>
> >> >>> >>>> Hi all guys,
> >> >>> >>>> after mails and mails of discussions, I don't
think there is a
> >> general
> >> >>> >>>> agreement on how Context API should look alike.
> >> >>> >>>> At the end of the discussions I figured out that,
briefly
> >> resuming, we
> >> >>> >>>> have following proposals:
> >> >>> >>>>
> >> >>> >>>>  * be replaced by Map;
> >> >>> >>>>  * be Collection agnostic and work by composition.
> >> >>> >>>>
> >> >>> >>>> Please add what is missing and correct what is
wrong; we need
> to
> >> find
> >> >>> >>>> a general agreement before to continue working
toward the 2.0
> >> release
> >> >>> >>>> :)
> >> >>> >>>>
> >> >>> >>>> TIA, all the best!!!
> >> >>> >>>> Simo
> >> >>> >>>>
> >> >>> >>>> http://people.apache.org/~simonetripodi/
> >> >>> >>>> http://www.99soft.org/
> >> >>> >>>>
> >> >>> >>>>
> >> ---------------------------------------------------------------------
> >> >>> >>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> >>> >>>> For additional commands, e-mail: dev-help@commons.apache.org
> >> >>> >>>>
> >> >>> >>>>
> >> >>> >>>
> >> >>> >>
> >> >>> >>
> >> ---------------------------------------------------------------------
> >> >>> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> >>> >> For additional commands, e-mail: dev-help@commons.apache.org
> >> >>> >>
> >> >>> >>
> >> >>> >
> >> >>> >
> ---------------------------------------------------------------------
> >> >>> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> >>> > For additional commands, e-mail: dev-help@commons.apache.org
> >> >>> >
> >> >>> >
> >> >>>
> >> >>>
> ---------------------------------------------------------------------
> >> >>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> >>> For additional commands, e-mail: dev-help@commons.apache.org
> >> >>>
> >> >>>
> >> >>
> >> >
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> For additional commands, e-mail: dev-help@commons.apache.org
> >>
> >>
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message