forced.getProperty () for enum field returns first enum value instead of null

[tfabien]

Tested on: jodd-util:6.0.0

I don't know if this is "by design" or a simple mishap, but I found this quite disturbing, I would have expected this to return "null" rather than an arbitrary value

@Slf4j
public class TestBeanUtilsGetForcedEnum {

  @Test
  public void testForcedGetEnum() {
    Dummy dummy = new Dummy();
    String key = "myEnum";
    log.info("dummy forcedSilent.hasProperty({}) -> {}", key, BeanUtil.forcedSilent.hasProperty(dummy, key));
    MyEnum myEnum = dummy.getMyEnum();
    log.info("dummy.getMyEnum() -> {}", myEnum);
    assertNull(myEnum);
    log.info("dummy forcedSilent.getProperty({}) -> {}", key, BeanUtil.forcedSilent.getProperty(dummy, key));
    myEnum = dummy.getMyEnum();
    log.info("dummy.getMyEnum() -> {}", myEnum);
    assertNull(myEnum);		
  }
  
  @Data
  public static class Dummy {
    String foo;
    String bar;
    MyEnum myEnum;
  }
  
  public static enum MyEnum {
    VAL1,VAL2,VAL3;
  }
}

Produces:

dummy forcedSilent.hasProperty(myEnum) -> true
dummy.getMyEnum() -> null
dummy forcedSilent.getProperty(myEnum) -> VAL1
dummy.getMyEnum() -> VAL1 (expected null)

I get that you can't instanciate a "new MyEnum", but IMO a "null enum" might be what's closest to an empty Object

"Problem" (if any...) is located in ClassUtil#newInstance(final Class<T> type)

public static <T> T newInstance(final Class<T> type) throws IllegalAccessException, InstantiationException, NoSuchMethodException, InvocationTargetException {
   // ..
  if (type.isEnum()) {
    // Maybe return null here?
    return type.getEnumConstants()[0];
  }
  // ...
}

[igr]

You are right, the last call should return null!

The thing with forced mode is that it tries to build everything on the way to get the property, but should not build the last property, in this case the enum.

Btw, try using BeanUtil.declared - that's one commonly used.

[igr]

Sharing my thoughts here: the forced mode is mainly for setting properties. Getting properties in forced mode was never in the focus.

Maybe getting the property in the forced mode should not create any nested property?

[igr]

I have the patch ready, just wondering if getting in forced mode should:

1. create all properties (i.e. as it was already working)
2. don't set any property, only for reading
3. set all nested properties but the last one (e.g. for a.b.c it would set a.b and not c)

I am leaning towards #2 - reading should not set anything.

[igr]

However, I see the tests are showing that option 1 is in place.
So it's either 1 or 3?

[tfabien]

I've been working around this last friday and I may add a bit more context about how I use this and why it bothered me.

I'm using jodd props to describe beans I want to create or populate, I found it a great fit with all the macros and profiles and so on...
So, for each defined property in my Props for the active profiles, I'm trying to see if the bean has such a property (eg: "declaredForcedSilent.hasProperty(propsKey)"), and, if it does, I'm getting the value to see if it has already been set (eg: "declaredForcedSilent.getProperty(propsKey) != null"), and finally, if it's null, I'm setting it to the value defined in my Props (eg: "declaredForcedSilent.setProperty(propsKey, props.getValue(propsKey))")

I encountered a series of hipcups doing so...
The first one is that "hasProperty" will return false if the property is nested and the parent bean is null (even with "forced" mode)
"getProperty" in forced mode could be used to overcome this by catching the exception, thus telling me there is no such property, but if the property does exist, getProperty() will affect it's value (and then the value would be considered as already defined...)

Maybe the real trouble here is that hasProperty should return true if the bean's class (and not the bean instance) has the required property, regardless of the bean instance having it's ancestors being set or not, whereas finding if the bean instance has this property set would be a "hasValue"...
eg:

  @Data
  public static class Foo {
    String foo;
    Bar bar;
  }

  @Data
  public static class Bar {
    String foo;
    MyEnum myEnum;
  }

  public static enum MyEnum {
    VAL1,VAL2,VAL3;
  }

  Foo bean = new Foo();

Would give:
hasProperty(bean, "bar.myEnum") -> false
getProperty(bean, "bar.myEnum") -> VAL1
hasProperty(bean, "bar.myEnum") -> true

Anyway, I think it's disturbing that a getter sets some values, be it the last one or any of it's ancestors.
The name doesn't make it quite clear that the bean may be altered... maybe we can make it more clear by having a "peekProperty()" method that does not affect the bean, and a "getProperty()" method (or any other name really... I don't know... getPropertyInstance()... you get the idea...) that mays affect the inspected bean on it's way and/or instanciate the last child (or not)

Let me know your thoughts on this, I found a workaround for my use case already so it's not a big problem right now

[igr]

Thank you for the detailed use case, now it's much clear to me what's going on.

You are right - hasProperty should not care about the instance. I guess the only reason why it works with instances is because of Map or List properties, where e.g. foo.map[something].bar depends on the actual value in the foo.map map.

However, has should work better. So my plan is:

• revisit how hasProperty works
• getProperty will not set any value (already have the patch for this).

Thank you again, I will build a snapshot one of these days and would kindly ask to review it if you don't mind :)))

[tfabien]

As always, I'm impressed with your reactivity and continued support on this framework, I'll gladly test the snapshot whenever it's ready

[igr]

@tfabien thanks for your kind words, I am really hoping this library helps ppl.

Sorry for the late response; work took my December away :(

I've just made a SNAPSHOT that should fix both issues: https://oss.sonatype.org/content/repositories/snapshots/org/jodd/jodd-util/6.0.1-SNAPSHOT/

If you have time, would you be able to try it and let me know if it works for you?