Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for different handling of nested null properties #11

Closed
siggemannen opened this issue Jan 26, 2022 · 17 comments
Closed

Support for different handling of nested null properties #11

siggemannen opened this issue Jan 26, 2022 · 17 comments
Assignees
Labels
enhancement New feature or request

Comments

@siggemannen
Copy link

Hi. And thanks for the nice lib.

I would like to be able to differentiate between accessing a nested property through null and an invalid nested property.
Consider following classes:

class X
{
 Y nested;
}

class Y
{
  int a;
}

BeanUtil.declared.getProperty(new X(), "nested.a")
vs
BeanUtil.declared.getProperty(new X(), "nested.b")

should give different exceptions in my opinion.

Since nested.b is a mistake in code, while nested.a is just cause nested is null.
Or if one wants keep same exception, perhaps a flag or a mode can be created to be able to differentiate these two cases.

@igr igr self-assigned this Jan 26, 2022
@igr igr added the enhancement New feature or request label Jan 26, 2022
@igr
Copy link
Member

igr commented Jan 27, 2022

👋

Make sense! When you say "different exception", do you mean A) a different exception type or B) simply a different message?

Somewhat related: note (in case you didnt know) there is a function that detects existence of a property:

System.out.println(BeanUtil.declared.hasProperty(new X(), "nested.a")); // true
System.out.println(BeanUtil.declared.hasProperty(new X(), "nested.b")); // false

Thank you for the feedback!

@siggemannen
Copy link
Author

Hi.
I was thinking different exception type. I'm trying to replace apache beanutils, and they throw a NestedNullException in case some of the nested params was null. But it would work with a flag or something in the BeanException, like: "boolean nestedNull", that says a property chain had a null value somewhere. Because otherwise all lib users need to add another checked exception.

What i'm looking for is to be able to just return null if any of the nested properties are null, but detect incorrect property names and throw exceptions.

Hmm, i tested hasProperty but both returned false!

X bean = new X();
System.out.println(BeanUtil.declared.hasProperty(bean, "nested.a")); // false
bean.nested = new Y(); // Nested must be inited first!
System.out.println(BeanUtil.declared.hasProperty(bean, "nested.a")); // true

@igr
Copy link
Member

igr commented Jan 27, 2022

Cool, I can refine the exception types - add different subtypes, including the new one for your case.

p.s. I will check the example with hasProperty, I am pretty sure it worked this morning :)

@igr
Copy link
Member

igr commented Feb 2, 2022

Under development, this week is just crazy :(

@siggemannen
Copy link
Author

It's cool, take your time! I will be happy to test it out =)

@igr
Copy link
Member

igr commented Feb 10, 2022

@siggemannen

The issue I am facing is this: in both cases, the nested of the new X() is null. BeanUtil simply stops there, not capable to continue any further. Since this is the same cause, the same exception would be thrown.

Otherwise, we would need to validate the whole path. This validation is not guaranteed, as it might depend on runtime (for lists, maps etc). Next, the whole path could be longer, e.g. nested.a.foo -> this one should throw the same exception as nested.b, right?

Note that the following works:

assertTrue(BeanUtil.declared.hasProperty(new X(), "nested.a"));
assertFalse(BeanUtil.declared.hasProperty(new X(), "nested.na"));

What I can do is to actually only cover the case of the immediate child: nested.foo when nested is null. So:

nested.a -> null exception
nested.b -> not found exception
nested.a.foo -> null exception (!!!)

wdyt?

@siggemannen
Copy link
Author

Hi. I think the behaviour in:

nested.a -> null exception
nested.b -> not found exception
nested.a.foo -> null exception (!!!)

is good enough!

@igr igr closed this as completed in 391dbf6 Feb 17, 2022
@igr
Copy link
Member

igr commented Feb 17, 2022

Here it is, the test is passing... I will release a snapshot for you to try it

@igr
Copy link
Member

igr commented Feb 18, 2022

The snapshot is published (hopfeully:)

@siggemannen
Copy link
Author

Hi,and thanks a lot!

I tested the snapshot and it worked fine, except for one test case i had.
Writing through a null property throws jodd.bean.exception.PropertyNotFoundBeanException.
I think for symmetry it should throw NullPropertyBeanException, what do you think?

My tests look something like this:

 //This passes
    @Test(expected=NullPropertyBeanException.class)
    public void test_utils_read_through_non_nulls_2()
    {
        X bean = new X();
        BeanUtils.getValue(bean, "nested.a");
    }
    
 //This fails with java.lang.Exception: Unexpected exception, expected<jodd.bean.exception.NullPropertyBeanException> but was<jodd.bean.exception.PropertyNotFoundBeanException>

@Test(expected=NullPropertyBeanException.class)
    public void test_utils_write_through_nulls()
    {
        X bean = new X();
        BeanUtils.setValue(bean, 5, "nested.a");
    }

@igr
Copy link
Member

igr commented Feb 19, 2022

Absolutely! Hold my beer... coding it :)

@igr
Copy link
Member

igr commented Feb 19, 2022

Done, releasing during the day (need to migrate from travis, damn :(

@igr
Copy link
Member

igr commented Feb 19, 2022

Snapshot published 🍭

@siggemannen
Copy link
Author

Awesome! It works for me now. Many thanks. If you have time to do a specific snap for the jodd.bean-package, it would be cool! But i can wait for proper release too

@igr
Copy link
Member

igr commented Feb 20, 2022

6.1.0 released :)

@siggemannen
Copy link
Author

Just a little followup. I've now been running BeanUtils and it works great =) Thanks a bunch

@igr
Copy link
Member

igr commented Feb 26, 2022

Glad to hear so!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

No branches or pull requests

2 participants