After thinking about it some more, the code generation could just access
the static Map in the extensible enums class and find out what instances
are registered. That would work but assumes that the configuration (or at
least the initialization of the static Map in Levels) is complete before
code generation.
On Sun, Jan 26, 2014 at 2:49 PM, Remko Popma <remko.po...@gmail.com> wrote:
> Did you mean that a custom Level implementation cannot extend StdLevel?
> So a custom Level implementation would need to include the levels defined
> in StdLevel (DEBUG, INFO, ...) in addition to the custom levels.
> I think I see now.
>
> How much of a problem is this though? Or, do we need to start thinking
> about the code generation bit in order to be able to decide if the
> trade-off is worth it?
>
>
>
> On Sun, Jan 26, 2014 at 2:39 PM, Remko Popma <remko.po...@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’t a Level because it can’t extend it if it is an enum,
>> so I can’t 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.go...@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’m 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.po...@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 = 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.go...@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 “standard”
>>>> 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’t 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’t
>>>> done it yet.
>>>> 4. Your example of creating the extension shows doing a new Level().
>>>> This doesn’t 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 <
>>>> nicho...@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 = 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 = new Object();
>>>> > map = new Hashtable<String, Level>();
>>>> > values = new TreeMap<Integer, Level>();
>>>> > OFF = new Level("OFF", 0) {};
>>>> > FATAL = new Level("FATAL", 100) {};
>>>> > ERROR = new Level("ERROR", 200) {};
>>>> > WARN = new Level("WARN", 300) {};
>>>> > INFO = new Level("INFO", 400) {};
>>>> > DEBUG = new Level("DEBUG", 500) {};
>>>> > TRACE = new Level("TRACE", 600) {};
>>>> > ALL = 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 == null || name.length() == 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 = name;
>>>> > this.intLevel = intLevel;
>>>> > this.ordinal = 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 <= level.intLevel;
>>>> > }
>>>> >
>>>> >
>>>> > public boolean isAtLeastAsSpecificAs(final int level) {
>>>> > return this.intLevel <= level;
>>>> > }
>>>> >
>>>> >
>>>> > public boolean lessOrEqual(final Level level) {
>>>> > return this.intLevel <= level.intLevel;
>>>> > }
>>>> >
>>>> >
>>>> > public boolean lessOrEqual(final int level) {
>>>> > return this.intLevel <= 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 == 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 == null)
>>>> > return defaultLevel;
>>>> > name = 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 == null)
>>>> > throw new IllegalArgumentException("Unknown level constant
>>>> [" + name + "].");
>>>> > name = 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 = 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 <garydgreg...@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 <
>>>> pbened...@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.po...@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 <pbened...@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 <
>>>> garydgreg...@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: garydgreg...@gmail.com | ggreg...@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-unsubscr...@logging.apache.org
>>>> > For additional commands, e-mail: log4j-dev-h...@logging.apache.org
>>>> >
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: log4j-dev-unsubscr...@logging.apache.org
>>>> For additional commands, e-mail: log4j-dev-h...@logging.apache.org
>>>>
>>>>
>>>
>>>
>>
>
