Return-Path: X-Original-To: apmail-logging-log4j-dev-archive@www.apache.org Delivered-To: apmail-logging-log4j-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id F0BAB10FA8 for ; Sun, 26 Jan 2014 06:19:00 +0000 (UTC) Received: (qmail 51956 invoked by uid 500); 26 Jan 2014 06:18:59 -0000 Delivered-To: apmail-logging-log4j-dev-archive@logging.apache.org Received: (qmail 51922 invoked by uid 500); 26 Jan 2014 06:18:58 -0000 Mailing-List: contact log4j-dev-help@logging.apache.org; run by ezmlm Precedence: bulk List-Unsubscribe: List-Help: List-Post: List-Id: "Log4J Developers List" Reply-To: "Log4J Developers List" Delivered-To: mailing list log4j-dev@logging.apache.org Received: (qmail 51914 invoked by uid 99); 26 Jan 2014 06:18:57 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 26 Jan 2014 06:18:57 +0000 X-ASF-Spam-Status: No, hits=1.5 required=5.0 tests=HTML_MESSAGE,RCVD_IN_DNSWL_LOW,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (nike.apache.org: domain of ralph.goers@dslextreme.com designates 209.85.160.170 as permitted sender) Received: from [209.85.160.170] (HELO mail-yk0-f170.google.com) (209.85.160.170) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 26 Jan 2014 06:18:50 +0000 Received: by mail-yk0-f170.google.com with SMTP id 9so8462766ykp.1 for ; Sat, 25 Jan 2014 22:18:29 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:content-type:message-id:mime-version :subject:date:references:to:in-reply-to; bh=el2oPsJzUbKtxHoodwWFIYm1wfEB9nBSOS0xnkS5GWQ=; b=CQ8mUevr7U8tzSR6qdwNWPcOe4Lx6reLPtdGnuWKKpSB3V/uq0LVw83uaXApwbmr9Y QZ8a+UI54tCm6mX5rvas4b3MlZFEqH8GdMe24MAV1s/nLoG8HAKuWO77l1guCdtQNgkG PJhVGw8CP3OdHG2XK1yC+V7LkMbFijdoID5V1S9MAgGIr2JVZaY9cgDB1aAlhMLs4HB1 ZQcKPw30WDekZDPpB0rkX++XXuA4nfj1nsifmRLoy1+PkFJ+ghWWKkMzTDsEPMNWkPnc 6Ffpf+euCbCI3VBuWBelaLMJ71uZCiOuCbiyCaJOinzcY5cFpYauSGWpzRtZ2NK6zceG nDCg== X-Gm-Message-State: ALoCoQnKz6h0LtXY6jCLFoRgaTi9/KskwpAGYisVxeGaKm2LMnNjUrTcdr2j9ujFLd2WQgG3Bvnz X-Received: by 10.236.172.33 with SMTP id s21mr31796yhl.71.1390717109282; Sat, 25 Jan 2014 22:18:29 -0800 (PST) Received: from [192.168.1.92] (99-180-69-21.lightspeed.irvnca.sbcglobal.net. [99.180.69.21]) by mx.google.com with ESMTPSA id s21sm20851804yhk.9.2014.01.25.22.18.28 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Sat, 25 Jan 2014 22:18:28 -0800 (PST) From: Ralph Goers Content-Type: multipart/alternative; boundary="Apple-Mail=_65496D16-E90C-403D-B37C-EDD2A9A609DD" Message-Id: <5F16D415-7C0D-4B4F-BFD7-1BF2CF961625@dslextreme.com> Mime-Version: 1.0 (Mac OS X Mail 7.0 \(1822\)) Subject: Re: Enums and Custom Levels Date: Sat, 25 Jan 2014 22:18:17 -0800 References: <3D63092A-CF7A-4D10-9F7A-094C0DDAB9AF@dslextreme.com> <438F7848-CEDA-46AB-BEEF-581A25A7446A@dslextreme.com> To: Log4J Developers List In-Reply-To: X-Mailer: Apple Mail (2.1822) X-Virus-Checked: Checked by ClamAV on apache.org --Apple-Mail=_65496D16-E90C-403D-B37C-EDD2A9A609DD Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=windows-1252 A malicious app could do=20 for (int i=3D0; i < 100000; ++i) { new Level(=93Level=94 + i, 1000 + i){}; } Sure idiots can do lots of bad things but I don=92t think Levels should = be quite that flexible. Ralph On Jan 25, 2014, at 9:39 PM, Remko Popma wrote: > I don't think client code can do new Level(){} as the constructor = requires String and int arguments. >=20 > By the way, I am unclear on what went wrong with the enum approach you = originally took. > You said: > StdLevel isn=92t a Level because it can=92t extend it if it is an = enum, so I can=92t initialize the levels using that. >=20 > I don't understand. StdLevel implements the Level interface, right? So = what do you mean by "it can't extend it"? >=20 > The reason I ask is that for the code generation, "real" java enums = may be easier to deal with than the extensible enum approach. >=20 >=20 > On Sun, Jan 26, 2014 at 2:30 PM, Ralph Goers = wrote: > Out of curiosity, what exactly is the benefit of declaring the class = abstract when it has a protected constructor? It seems like all you are = accomplishing is making the instantiation syntax uglier. It also bothers = me that open code can just do a new Level(){} - which will do nothing = but cause problems. I=92m beginning to think that we should require an = annotation on the Level declaration of the extension class to avoid = that. >=20 > Ralph=20 >=20 > On Jan 25, 2014, at 9:13 PM, Remko Popma = wrote: >=20 >> Ralph, >> I copied Nick's code _as is_ and had no compile errors. >> The class is abstract, but instances are defined in the static block = as: >> OFF =3D new Level("OFF", 0) {}; // note the {}: this creates an = anonymous concrete subclass >>=20 >> I agree that read access needs to be synchronized as well, not just = write access (the constructor). >> I experimented with several options:=20 >> * synchronizing on plain Object in both constructor and when = accessing the static Map(s) >> * a ReentrantReadWriteLock >> * a lock-free implementation >>=20 >> I decided against ReentrantReadWriteLock as it has more overhead than = plain synchronized access and the write access (in the constructors) is = going to be extremely rare: not worth paying the overhead in the more = common reads. It is also cumbersome to code. >>=20 >> The lock-free implementation uses an AtomicInteger for the ordinals, = and an AtomicReference for the Map. >> In the constructor, create a new Map instance based on = the old copy, add the new instance, and try to call compareAndSet to = replace the old instance with the new instance. Retry on failure. >>=20 >> Finally, simply synchronizing on the constructorLock object in the = Level.toLevel() and Level.values() methods may be simplest.=20 >>=20 >> Which of the last two is best depends on how often the toLevel() and = values() levels are called. >> It turns out they are only called during reconfiguration, so no real = need to optimize these methods. >> I would argue that simple synchronization may be best in this case. >>=20 >> Remko >> =20 >>=20 >>=20 >> On Sun, Jan 26, 2014 at 1:49 PM, Ralph Goers = wrote: >> As I am working on this I just want to point out a number of issues = with the code below: >>=20 >> 1. The class is abstract. The static block is doing a bunch of new = Level() invocations which obviously generate compile errors on an = abstract class. I had to make it be a non-abstract class. >> 2. As I pointed out before there is no way to access the =93standard=94= levels as an enum. I have addressed that. >> 3. Although the constructor is synchronized access to the Map is not. = Trying to get from the map while a Level is being added will result in a = ConcurrentModificationException. I am using a ConcurrentMap instead. >> 3. The constructor requires synchronization because it is modifying = both the map and the ordinal. However, since this isn=92t an enum the = ordinal value is of dubious value. Removing that would allow the removal = of the synchronization in the constructor. I am considering that but I = haven=92t done it yet. >> 4. Your example of creating the extension shows doing a new Level(). = This doesn=92t work because a) the class is abstract and b) the = constructor is protected. I am leaving the constructor protected so = extension will require doing new ExtendedLevel(name, value) and creating = a constructor. Not requiring that means applications can do a new = Level() anywhere and I am opposed to allowing that. >>=20 >> Ralph >>=20 >> On Jan 23, 2014, at 12:42 AM, Nick Williams = wrote: >>=20 >> > Okay, I finally got a minute to read all of these emails, and... >> > >> > EVERYBODY FREEZE! >> > >> > What if I could get you an extensible enum that required no = interface changes and no binary-incompatible changes at all? Sound too = good to be true? I proposed this months ago (LOG4J2-41) and it got shot = down multiple times, but as of now I've heard THREE people say = "extensible enum" in this thread, so here it is, an extensible enum: >> > >> > public abstract class Level implements Comparable, = Serializable { >> > public static final Level OFF; >> > public static final Level FATAL; >> > public static final Level ERROR; >> > public static final Level WARN; >> > public static final Level INFO; >> > public static final Level DEBUG; >> > public static final Level TRACE; >> > public static final Level ALL; >> > >> > >> > private static final long serialVersionUID =3D 0L; >> > private static final Hashtable map; >> > private static final TreeMap values; >> > private static final Object constructorLock; >> > >> > >> > static { >> > // static variables must be constructed in certain order >> > constructorLock =3D new Object(); >> > map =3D new Hashtable(); >> > values =3D new TreeMap(); >> > OFF =3D new Level("OFF", 0) {}; >> > FATAL =3D new Level("FATAL", 100) {}; >> > ERROR =3D new Level("ERROR", 200) {}; >> > WARN =3D new Level("WARN", 300) {}; >> > INFO =3D new Level("INFO", 400) {}; >> > DEBUG =3D new Level("DEBUG", 500) {}; >> > TRACE =3D new Level("TRACE", 600) {}; >> > ALL =3D new Level("ALL", Integer.MAX_VALUE) {}; >> > } >> > >> > >> > private static int ordinals; >> > >> > >> > private final String name; >> > private final int intLevel; >> > private final int ordinal; >> > >> > >> > protected Level(String name, int intLevel) { >> > if(name =3D=3D null || name.length() =3D=3D 0) >> > throw new IllegalArgumentException("Illegal null Level = constant"); >> > if(intLevel < 0) >> > throw new IllegalArgumentException("Illegal Level int = less than zero."); >> > synchronized (Level.constructorLock) { >> > if(Level.map.containsKey(name.toUpperCase())) >> > throw new IllegalArgumentException("Duplicate Level = constant [" + name + "]."); >> > if(Level.values.containsKey(intLevel)) >> > throw new IllegalArgumentException("Duplicate Level = int [" + intLevel + "]."); >> > this.name =3D name; >> > this.intLevel =3D intLevel; >> > this.ordinal =3D Level.ordinals++; >> > Level.map.put(name.toUpperCase(), this); >> > Level.values.put(intLevel, this); >> > } >> > } >> > >> > >> > public int intLevel() { >> > return this.intLevel; >> > } >> > >> > >> > public boolean isAtLeastAsSpecificAs(final Level level) { >> > return this.intLevel <=3D level.intLevel; >> > } >> > >> > >> > public boolean isAtLeastAsSpecificAs(final int level) { >> > return this.intLevel <=3D level; >> > } >> > >> > >> > public boolean lessOrEqual(final Level level) { >> > return this.intLevel <=3D level.intLevel; >> > } >> > >> > >> > public boolean lessOrEqual(final int level) { >> > return this.intLevel <=3D level; >> > } >> > >> > >> > @Override >> > @SuppressWarnings("CloneDoesntCallSuperClone") >> > public Level clone() throws CloneNotSupportedException { >> > throw new CloneNotSupportedException(); >> > } >> > >> > >> > @Override >> > public int compareTo(Level other) { >> > return intLevel < other.intLevel ? -1 : (intLevel > = other.intLevel ? 1 : 0); >> > } >> > >> > >> > @Override >> > public boolean equals(Object other) { >> > return other instanceof Level && other =3D=3D this; >> > } >> > >> > >> > public Class getDeclaringClass() { >> > return Level.class; >> > } >> > >> > >> > @Override >> > public int hashCode() { >> > return this.name.hashCode(); >> > } >> > >> > >> > public String name() { >> > return this.name; >> > } >> > >> > >> > public int ordinal() { >> > return this.ordinal; >> > } >> > >> > >> > @Override >> > public String toString() { >> > return this.name; >> > } >> > >> > >> > public static Level toLevel(String name) { >> > return Level.toLevel(name, Level.DEBUG); >> > } >> > >> > >> > public static Level toLevel(String name, Level defaultLevel) { >> > if(name =3D=3D null) >> > return defaultLevel; >> > name =3D name.toUpperCase(); >> > if(Level.map.containsKey(name)) >> > return Level.map.get(name); >> > return defaultLevel; >> > } >> > >> > >> > public static Level[] values() { >> > return Level.values.values().toArray(new = Level[Level.values.size()]); >> > } >> > >> > >> > public static Level valueOf(String name) { >> > if(name =3D=3D null) >> > throw new IllegalArgumentException("Unknown level = constant [" + name + "]."); >> > name =3D name.toUpperCase(); >> > if(Level.map.containsKey(name)) >> > return Level.map.get(name); >> > throw new IllegalArgumentException("Unknown level constant = [" + name + "]."); >> > } >> > >> > >> > public static > T valueOf(Class enumType, = String name) { >> > return Enum.valueOf(enumType, name); >> > } >> > >> > >> > // for deserialization >> > protected final Object readResolve() throws = ObjectStreamException { >> > return Level.valueOf(this.name); >> > } >> > } >> > >> > Extending it is easy: >> > >> > public final class ExtendedLevels { >> > public static final Level MY_LEVEL =3D new Level("MY_LEVEL", = 250) {}; >> > } >> > >> > I still and have ALWAYS believed this was the best option. If we = used this option, I would be fine with not adding any new Levels because = I could add them myself. >> > >> > Nick >> > >> > On Jan 22, 2014, at 7:04 PM, Remko Popma wrote: >> > >> >> This is only a problem for webapps, right? >> >> Putting log4j jars in WEB-INF/lib avoids that problem (different = class loader). >> >> Apps that really want to share log4j jars with other apps would = need to play nice. Such apps would do well to use a naming convention = like Gary suggests. >> >> Otherwise, the last to register would overwrite any previous level = with the same name. (Should probably emit a StatusLogger warning.) >> >> >> >> Same intLevel for different names should not be a problem. >> >> >> >> >> >> On Thursday, January 23, 2014, Gary Gregory = wrote: >> >> Playing devils advocate: >> >> >> >> What happens when different apps register levels with the same = name and different intLevels? >> >> What happens when different apps register levels with the same = intLevel and different names? >> >> Should there be a convention that custom level names be FQNs? >> >> >> >> Gary >> >> >> >> >> >> On Wed, Jan 22, 2014 at 10:05 PM, Paul Benedict = wrote: >> >> As Gary wanted, a new thread.... >> >> >> >> First, each enum needs an inherit strength. This would be part of = the interface. Forgive me if the word "strength" is wrong; but it's the = 100, 200, 300, etc. number that triggers the log level. So make sure the = interface contains the intLevel() method. >> >> >> >> Second, we need to know the name, right? The name probably = requires a new method since it can't be extracted from the enum anymore. >> >> >> >> public interface Level { >> >> int intLevel(); >> >> String name(); >> >> } >> >> >> >> PS: The intStrength() name seems hackish. What about strength() or = treshold()? >> >> >> >> Third, the registration can be done manually by providing a static = method (as your did Remko) that the client needs to invoke, or you could = have a class-path scanning mechanism. For the latter, you could = introduce a new annotation to be placed on the enum class. >> >> >> >> @CustomLevels >> >> public enum MyCustomEnums { >> >> } >> >> >> >> Paul >> >> >> >> On Wed, Jan 22, 2014 at 8:52 PM, Remko Popma = wrote: >> >> Paul, can you give a bit more detail? >> >> >> >> I tried this: copy the current Level enum to a new enum called = "Levels" in the same package (other name would be fine too). Then change = Level to an interface (removing the constants and static methods, = keeping only the non-static methods). Finally make the Levels enum = implement the Level interface. >> >> >> >> After this, we need to do a find+replace for the references to = Level.CONSTANT to Levels.CONSTANT and Level.staticMethod() to = Levels.staticMethod(). >> >> >> >> Finally, the interesting part: how do users add or register their = custom levels and how do we enable the Levels.staticLookupMethod(String, = Level) to recognize these custom levels? >> >> >> >> >> >> >> >> On Thursday, January 23, 2014, Paul Benedict = wrote: >> >> Agreed. This is not an engineering per se, but really more about = if the feature set makes sense. >> >> >> >> Well if you guys ever look into the interface idea, you'll give = log4j the feature of getting enums to represent custom levels. That's = pretty cool, IMO. I don't know if any other logging framework has that = and that would probably get some positive attention. It shouldn't be so = hard to do a find+replace on the code that accepts Level and replace it = with another name. Yes, there will be some minor refactoring that goes = with it, but hard? It shouldn't be. >> >> >> >> A name I propose for the interface is LevelDefinition. >> >> >> >> Paul >> >> >> >> >> >> On Wed, Jan 22, 2014 at 6:48 PM, Gary Gregory = wrote: >> >> Hi, I do not see this as an engineering problem but more a feature = set definition issue. So while there may be lots of more or less = internally complicated ways of solving this with interfaces, makers and = whatnots, the built in levels are the most user friendly. >> >> >> >> I have have lots of buttons, knobs and settings on my sound system = that I do not use, just like I do not use all the methods in all the = classes in the JRE... >> >> >> >> Gary >> >> >> >> >> >> >> >> >> >> -- >> >> E-Mail: garydgregory@gmail.com | ggregory@apache.org >> >> Java Persistence with Hibernate, Second Edition >> >> JUnit in Action, Second Edition >> >> Spring Batch in Action >> >> Blog: http://garygregory.wordpress.com >> >> Home: http://garygregory.com/ >> >> Tweet! http://twitter.com/GaryGregory >> > >> > >> > = --------------------------------------------------------------------- >> > To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org >> > For additional commands, e-mail: log4j-dev-help@logging.apache.org >> > >>=20 >>=20 >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org >> For additional commands, e-mail: log4j-dev-help@logging.apache.org >>=20 >>=20 >=20 >=20 --Apple-Mail=_65496D16-E90C-403D-B37C-EDD2A9A609DD Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=windows-1252
A = malicious app could do 

for (int i=3D0; i = < 100000; ++i) {
  new Level(=93Level=94 + i, 1000 + = i){};
}

Sure idiots can do lots of = bad things but I don=92t think Levels should be quite that = flexible.

Ralph

On Jan 25, = 2014, at 9:39 PM, Remko Popma <remko.popma@gmail.com> = wrote:

I don't think client code can do new = Level(){} as the constructor requires String and int = arguments.

By the way, I am unclear on what went = wrong with the enum approach you originally took.
You said:
    StdLevel isn=92t a = Level because it can=92t extend it if it is an enum, so I can=92t = initialize the levels using that.

I don't understand. = StdLevel implements the Level interface, right? So what do you mean by = "it can't extend it"?

The reason I ask is = that for the code generation, "real" java enums may be easier to deal = with than the extensible enum approach.


On Sun, Jan 26, 2014 at 2:30 PM, = Ralph Goers <ralph.goers@dslextreme.com> = wrote:
Out of curiosity, what exactly = is the benefit of declaring the class abstract when it has a protected = constructor?  It seems like all you are accomplishing is making the = instantiation syntax uglier. It also bothers me that open code can just = do a new Level(){} - which will do nothing but cause problems.  I=92m= beginning to think that we should require an annotation on the Level = declaration of the extension class to avoid that.

Ralph 
=

On Jan 25, 2014, at 9:13 PM, Remko Popma = <remko.popma@gmail.com> wrote:

Ralph,
I copied = Nick's code _as is_ and had no compile errors.
The class is = abstract, but instances are defined in the static block = as:
OFF =3D new Level("OFF", 0) {}; // note the {}: this = creates an anonymous concrete subclass

I agree that read access needs to be = synchronized as well, not just write access (the = constructor).
I experimented with several = options: 
* synchronizing on plain Object in both = constructor and when accessing the static Map(s)
* a ReentrantReadWriteLock
* a lock-free = implementation

I decided against = ReentrantReadWriteLock as it has more overhead than plain synchronized = access and the write access (in the constructors) is going to be = extremely rare: not worth paying the overhead in the more common reads. = It is also cumbersome to code.

The lock-free implementation uses an AtomicInteger = for the ordinals, and an AtomicReference for the Map<String, = Level>.
In the constructor, create a new Map<String, = Level> instance based on the old copy, add the new instance, and try = to call compareAndSet to replace the old instance with the new instance. = Retry on failure.

Finally, simply synchronizing on the constructorLock = object in the Level.toLevel() and Level.values() methods may be = simplest. 

Which of the last two is best = depends on how often the toLevel() and values() levels are called.
It turns out they are only called during reconfiguration, so no = real need to optimize these methods.
I would argue that simple = synchronization may be best in this = case.

Remko
 


On Sun, Jan 26, 2014 at 1:49 PM, Ralph Goers <ralph.goers@dslextreme.com> wrote:
As I am working on = this I just want to point out a number of issues with the code = below:

1. The class is abstract. The static block is doing a bunch of new = Level() invocations which obviously generate compile errors on an = abstract class.  I had to make it be a non-abstract class.
2. As I pointed out before there is no way to access the =93standard=94 = levels as an enum. I have addressed that.
3. Although the constructor is synchronized access to the Map is not. = Trying to get from the map while a Level is being added will result in a = ConcurrentModificationException. I am using a ConcurrentMap instead.
3. The constructor requires synchronization because it is modifying both = the map and the ordinal. However, since this isn=92t an enum the ordinal = value is of dubious value. Removing that would allow the removal of the = synchronization in the constructor. I am considering that but I haven=92t = done it yet.
4. Your example of creating the extension shows doing a new Level(). = This doesn=92t work because a) the class is abstract and b) the = constructor is protected. I am leaving the constructor protected so = extension will require doing new ExtendedLevel(name, value) and creating = a constructor. Not requiring that means applications can do a new = Level() anywhere and I am opposed to allowing that.

Ralph

On Jan 23, 2014, at 12:42 AM, Nick Williams <nicholas@nicholaswilliams.net> wrote:

> Okay, I finally got a minute to read all of these emails, = and...
>
> EVERYBODY FREEZE!
>
> What if I could get you an extensible enum that required no = interface changes and no binary-incompatible changes at all? Sound too = good to be true? I proposed this months ago (LOG4J2-41) and it got shot = down multiple times, but as of now I've heard THREE people say = "extensible enum" in this thread, so here it is, an extensible enum:
>
> public abstract class Level implements Comparable<Level>, = Serializable {
>    public static final Level OFF;
>    public static final Level FATAL;
>    public static final Level ERROR;
>    public static final Level WARN;
>    public static final Level INFO;
>    public static final Level DEBUG;
>    public static final Level TRACE;
>    public static final Level ALL;
>
>
>    private static final long serialVersionUID =3D 0L;
>    private static final Hashtable<String, Level> = map;
>    private static final TreeMap<Integer, Level> = values;
>    private static final Object constructorLock;
>
>
>    static {
>        // static variables must be constructed = in certain order
>        constructorLock =3D new Object();
>        map =3D new Hashtable<String, = Level>();
>        values =3D new TreeMap<Integer, = Level>();
>        OFF =3D new Level("OFF", 0) {};
>        FATAL =3D new Level("FATAL", 100) = {};
>        ERROR =3D new Level("ERROR", 200) = {};
>        WARN =3D new Level("WARN", 300) {};
>        INFO =3D new Level("INFO", 400) {};
>        DEBUG =3D new Level("DEBUG", 500) = {};
>        TRACE =3D new Level("TRACE", 600) = {};
>        ALL =3D new Level("ALL", = Integer.MAX_VALUE) {};
>    }
>
>
>    private static int ordinals;
>
>
>    private final String name;
>    private final int intLevel;
>    private final int ordinal;
>
>
>    protected Level(String name, int intLevel) {
>        if(name =3D=3D null || name.length() =3D=3D= 0)
>            throw new = IllegalArgumentException("Illegal null Level constant");
>        if(intLevel < 0)
>            throw new = IllegalArgumentException("Illegal Level int less than zero.");
>        synchronized (Level.constructorLock) = {
>           =  if(Level.map.containsKey(name.toUpperCase()))
>                throw new = IllegalArgumentException("Duplicate Level constant [" + name + = "].");
>           =  if(Level.values.containsKey(intLevel))
>                throw new = IllegalArgumentException("Duplicate Level int [" + intLevel + "].");
>            this.name =3D name;
>            this.intLevel =3D = intLevel;
>            this.ordinal =3D = Level.ordinals++;
>           =  Level.map.put(name.toUpperCase(), this);
>            Level.values.put(intLevel, = this);
>        }
>    }
>
>
>    public int intLevel() {
>        return this.intLevel;
>    }
>
>
>    public boolean isAtLeastAsSpecificAs(final Level = level) {
>        return this.intLevel <=3D = level.intLevel;
>    }
>
>
>    public boolean isAtLeastAsSpecificAs(final int level) = {
>        return this.intLevel <=3D level;
>    }
>
>
>    public boolean lessOrEqual(final Level level) {
>        return this.intLevel <=3D = level.intLevel;
>    }
>
>
>    public boolean lessOrEqual(final int level) {
>        return this.intLevel <=3D level;
>    }
>
>
>    @Override
>    @SuppressWarnings("CloneDoesntCallSuperClone")
>    public Level clone() throws CloneNotSupportedException = {
>        throw new = CloneNotSupportedException();
>    }
>
>
>    @Override
>    public int compareTo(Level other) {
>        return intLevel < other.intLevel ? -1 = : (intLevel > other.intLevel ? 1 : 0);
>    }
>
>
>    @Override
>    public boolean equals(Object other) {
>        return other instanceof Level && = other =3D=3D this;
>    }
>
>
>    public Class<Level> getDeclaringClass() {
>        return Level.class;
>    }
>
>
>    @Override
>    public int hashCode() {
>        return this.name.hashCode();
>    }
>
>
>    public String name() {
>        return this.name;
>    }
>
>
>    public int ordinal() {
>        return this.ordinal;
>    }
>
>
>    @Override
>    public String toString() {
>        return this.name;
>    }
>
>
>    public static Level toLevel(String name) {
>        return Level.toLevel(name, = Level.DEBUG);
>    }
>
>
>    public static Level toLevel(String name, Level = defaultLevel) {
>        if(name =3D=3D null)
>            return defaultLevel;
>        name =3D name.toUpperCase();
>        if(Level.map.containsKey(name))
>            return = Level.map.get(name);
>        return defaultLevel;
>    }
>
>
>    public static Level[] values() {
>        return Level.values.values().toArray(new = Level[Level.values.size()]);
>    }
>
>
>    public static Level valueOf(String name) {
>        if(name =3D=3D null)
>            throw new = IllegalArgumentException("Unknown level constant [" + name + "].");
>        name =3D name.toUpperCase();
>        if(Level.map.containsKey(name))
>            return = Level.map.get(name);
>        throw new = IllegalArgumentException("Unknown level constant [" + name + "].");
>    }
>
>
>    public static <T extends Enum<T>> T = valueOf(Class<T> enumType, String name) {
>        return Enum.valueOf(enumType, name);
>    }
>
>
>    // for deserialization
>    protected final Object readResolve() throws = ObjectStreamException {
>        return Level.valueOf(this.name);
>    }
> }
>
> Extending it is easy:
>
> public final class ExtendedLevels {
>    public static final Level MY_LEVEL =3D new = Level("MY_LEVEL", 250) {};
> }
>
> I still and have ALWAYS believed this was the best option. If we = used this option, I would be fine with not adding any new Levels because = I could add them myself.
>
> Nick
>
> On Jan 22, 2014, at 7:04 PM, Remko Popma wrote:
>
>> This is only a problem for webapps, right?
>> Putting log4j jars in WEB-INF/lib avoids that problem = (different class loader).
>> Apps that really want to share log4j jars with other apps would = need to play nice. Such apps would do well to use a naming convention = like Gary suggests.
>> Otherwise, the last to register would overwrite any previous = level with the same name. (Should probably emit a StatusLogger = warning.)
>>
>> Same intLevel for different names should not be a problem.
>>
>>
>> On Thursday, January 23, 2014, Gary Gregory <garydgregory@gmail.com> wrote:
>> Playing devils advocate:
>>
>> What happens when different apps register levels with the same = name and different intLevels?
>> What happens when different apps register levels with the same = intLevel and different names?
>> Should there be a convention that custom level names be = FQNs?
>>
>> Gary
>>
>>
>> On Wed, Jan 22, 2014 at 10:05 PM, Paul Benedict <pbenedict@apache.org> wrote:
>> As Gary wanted, a new thread....
>>
>> First, each enum needs an inherit strength. This would be part = of the interface. Forgive me if the word "strength" is wrong; but it's = the 100, 200, 300, etc. number that triggers the log level. So make sure = the interface contains the intLevel() method.
>>
>> Second, we need to know the name, right? The name probably = requires a new method since it can't be extracted from the enum = anymore.
>>
>> public interface Level {
>> int intLevel();
>> String name();
>> }
>>
>> PS: The intStrength() name seems hackish. What about strength() = or treshold()?
>>
>> Third, the registration can be done manually by providing a = static method (as your did Remko) that the client needs to invoke, or = you could have a class-path scanning mechanism. For the latter, you = could introduce a new annotation to be placed on the enum class.
>>
>> @CustomLevels
>> public enum MyCustomEnums {
>> }
>>
>> Paul
>>
>> On Wed, Jan 22, 2014 at 8:52 PM, Remko Popma <remko.popma@gmail.com> wrote:
>> Paul, can you give a bit more detail?
>>
>> I tried this: copy the current Level enum to a new enum called = "Levels" in the same package (other name would be fine too). Then change = Level to an interface (removing the constants and static methods, = keeping only the non-static methods). Finally make the Levels enum = implement the Level interface.
>>
>> After this, we need to do a find+replace for the references to = Level.CONSTANT to Levels.CONSTANT and Level.staticMethod() to = Levels.staticMethod().
>>
>> Finally, the interesting part: how do users add or register = their custom levels and how do we enable the = Levels.staticLookupMethod(String, Level) to recognize these custom = levels?
>>
>>
>>
>> On Thursday, January 23, 2014, Paul Benedict <pbenedict@apache.org> wrote:
>> Agreed. This is not an engineering per se, but really more = about if the feature set makes sense.
>>
>> Well if you guys ever look into the interface idea, you'll give = log4j the feature of getting enums to represent custom levels. That's = pretty cool, IMO. I don't know if any other logging framework has that = and that would probably get some positive attention. It shouldn't be so = hard to do a find+replace on the code that accepts Level and replace it = with another name. Yes, there will be some minor refactoring that goes = with it, but hard? It shouldn't be.
>>
>> A name I propose for the interface is LevelDefinition.
>>
>> Paul
>>
>>
>> On Wed, Jan 22, 2014 at 6:48 PM, Gary Gregory <garydgregory@gmail.com> wrote:
>> Hi, I do not see this as an engineering problem but more a = feature set definition issue. So while there may be lots of more or less = internally complicated ways of solving this with interfaces, makers and = whatnots, the built in levels are the most user friendly.
>>
>> I have have lots of buttons, knobs and settings on my sound = system that I do not use, just like I do not use all the methods in all = the classes in the JRE...
>>
>> Gary
>>
>>
>>
>>
>> --
>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>> Java Persistence with Hibernate, Second Edition
>> JUnit in Action, Second Edition
>> Spring Batch in Action
>> Blog: http://garygregory.wordpress.com
>> Home: http://garygregory.com/
>> Tweet! http://twitter.com/GaryGregory
>
>
> = ---------------------------------------------------------------------
> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>


= ---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org





= --Apple-Mail=_65496D16-E90C-403D-B37C-EDD2A9A609DD--