Relax the POJO definition a bit to handle special cases of mocking needs

[mystarrocks]

We often deal with classes that are not POJOs in the strict sense of the word, but can very well be argued as ones.

Here's an example:

public class SpecialPojo {
  private static class Holder {
    private String holdingString;
    private long holdingLong;
  }

  private Holder myHolder;

  public SpecialPojo() {
    myHolder = new Holder();
  }

  public String getMyString() {
    return myHolder.holdingString;
  }

  public void setMyString(String myString) {
    myHolder.holdingString = myString;
  }

  public long getMyLong() {
    return myHolder.holdingLong;
  }

  public void setMyLong(long myLong) {
    myHolder.holdingLong = myLong;
  }
}

One would think this is one of the good, if not best, candidates PODAM could weave its magic on to create a mock object with arbitrary values. But unfortunately, PODAM won't fill this object with values since it doesn't strictly adhere to Java Bean standards. If PODAM was any less strict to not expect an explicit instance field matching the setter/getter, we'd have had an easier time mocking this type. These lines from the PodamUtils.java are what holding PODAM from being able to mock it:

public static ClassInfo getClassInfo(Class<?> clazz, 
    Set<Class<? extends Annotation>> excludeFieldAnnotations) {
  Set<String> classFields = getDeclaredInstanceFields(clazz, excludeFieldAnnotations);
  Set<Method> classSetters = getPojoSetters(clazz, classFields);
  return new ClassInfo(clazz, classFields, classSetters);
}

Can PODAM be enhanced to relax its definition of POJO a bit so it will be able to handle such special cases of types that need mocking?

[mtedone]

First comments that come to my mind. The ultimate goal of PODAM is to fill a POJO state. The strict adherence to JavaBean standards (although we've relaxed it quite a bit already by filling Immutable classes and, where possible, attributes without a setter) allows us to set only attributes. Even in the case above ultimately you want to fill Holder's attributes, only through outer setter methods. Now, one way that we could achieve that would be to invoke all setters in a POJO regardless. But here I think there might lie the problem. What would happen if someone wrote a setter that is not actually a setter and has some nasty side effects? Consider the following case:

public class Pojo {

private String string1;

private String string2;

public setString1(String string) {...}

public setString2(String string) {...}

public setInitialStatus(Bombshell bomb) {
bomb.ignite(); //This could have any sort of side effect
}

}

If we were to relax the Javabean rules, Podam would first set string1 and string2 and then ignite a bomb. One could ask: "Why would anyone write code like that?" and the answer is probably no one but that kind of proves my point. Podam is a framework and as such it doesn't have to know whether users write good or bad code. It needs to always work regardless of sensible or meaningless code. The adherence to Javabean standards is a contract between Podam and its users so that it can always fulfil its mission, which is to set a POJO's internal state.

How would you suggest Podam deals with the above situation?

[mystarrocks]

That's one of the two things that came to mind PODAM will have to deal with from its perspective.

• What if the setter wasn't a Java Bean style setter? Like what if accepted 2 or more parameters and returned something? I mean, although that's bad code, it's still writable, so PODAM will have to deal with it. I think PODAM deals with such cases well already - it ensures the setter signature matches that of the traditional setter before invoking, so there's no problem with this.
• What if the setter had invariants, especially dangerous ones at that throwing exceptions/errors if the parameter wasn't what the setter was expecting it to be (guess @PodamExclude or the DataProviderStrategy would help here) or bad code (like your bombshell example)? The one solution I can think of (if there wasn't an explicit matching instance field, of course) is extending @PodamExclude to methods so that the user can specify the annotation on setter they want PODAM to not mess with. My point is, such a not-so-sensible code could as well be written in a Java Bean with an explicitly standard instance field, so PODAM must already be running that risk and expecting user's intervention in such cases thru the @PodamExclude annotation or the DataProviderStrategy on the field that they want Podam to handle the way they expect it to.

But then if PODAM had to deviate too much from its principle of keeping it simple and expecting types to strictly adhere to the Java Bean standards, I guess it should be fine to not relax this.

[mtedone]

Maybe what we could do is to provide Podam with an additional list of
methods to invoke. We're already doing something similar by allowing users
to define, for example, a list of fields to exclude. So similarly we could
have an extendedMethods collection containing a list of additional methods
to execute and stick that to the Classinfo class.

Daniil what do you think?

On Saturday, April 11, 2015, mystarrocks notifications@github.com wrote:

That's one of the two things that came to mind PODAM will have to deal
with from its perspective.

What if the setter wasn't a Java Bean style setter? Like what if
accepted 2 or more parameters and returned something? I mean, although
that's bad code, it's still writable, so PODAM will have to deal with
it. I think PODAM deals with such cases well already - it ensures the
setter signature matches that of the traditional setter before invoking, so
there's no problem with this.

What if the setter had invariants, especially dangerous ones at that
throwing exceptions/errors if the parameter wasn't what the setter was
expecting it to be (guess @PodamExclude or the DataProviderStrategy
would help here) or bad code (like your bombshell example)? The one
solution I can think of (if there wasn't an explicit matching instance
field, of course) is extending @PodamExclude to methods so that the
user can specify the annotation on setter they want PODAM to not mess
with. My point is, such a not-so-sensible code could as well be written in
a Java Bean with an explicitly standard instance field, so PODAM must
already be running that risk and expecting user's intervention in such
cases thru the @PodamExclude annotation or the DataProviderStrategy on
the field that they want Podam to handle the way they expect it to.

But then if PODAM had to deviate too much from its principle of keeping
it simple and expecting types to strictly adhere to the Java Bean
standards, I guess it should be fine to not relax this.

—
Reply to this email directly or view it on GitHub
#86 (comment).

[daivanov]

I will speak about Podam 5.2.1-SNAPSHOT version. In 5.2.0 this is also possible, but code will be slightly different.

So what is between you and the success is AbstractClassInfoStrategy.approve() method, part of ClassInfoStrategy, which implements POJO introspection

public boolean approve(ClassAttribute attribute) {
    return (attribute.getAttribute() != null);
}

It skips all setters, which has no field, because Java contains lots of classes with setters, which breaks PODAM production.

So what you need to do is to use custom ClassInfoStrategy. For example, like this:

private static ClassInfoStrategy classInfoStrategy
        = new AbstractClassInfoStrategy() {

    @Override
    public boolean approve(ClassAttribute attribute) {

        /* EJB attributes: field and setter */
        if (attribute.getAttribute() != null) {
            return true;
        }

        /* Exclusion for SpecialPojo class */
        for (Method setter : attribute.getSetters()) {
            if (SpecialPojo.class.equals(setter.getDeclaringClass())) {
                return true;
            }
        }
        return false;
    }
};

Use this strategy with Podam factory

PodamFactory podam = new PodamFactoryImpl().setClassStrategy(classInfoStrategy);

And then attributes get filled:

SpecialPojo pojo = podam.manufacturePojo(SpecialPojo.class);
Assert.assertNotNull("Construction failed", pojo);
Assert.assertNotNull("Attr should not be empty", pojo.getMyString());

I hope this helps.

[mtedone]

Daniil, I think what we need is an extraMethods concept where users can add the extra methods they want to execute. These might not only be setters but any method really. I’ve created a local branch issue-86 and I’m trying to implement this concept there.

Regards,

On 12 Apr 2015, at 14:18, Daniil Ivanov notifications@github.com wrote:

I will speak about Podam 5.2.1-SNAPSHOT version. In 5.2.0 this is also possible, but code will be slightly different.

So what is between you and the success is AbstractClassInfoStrategy.approve() method, part of ClassInfoStrategy, which implements POJO introspection

public boolean approve(ClassAttribute attribute) {
return (attribute.getAttribute() != null);
}
It skips all setters, which has no field, because Java contains lots of classes with setters, which breaks PODAM production.

So what you need to do is to use custom ClassInfoStrategy. For example, like this:

private static ClassInfoStrategy classInfoStrategy
= new AbstractClassInfoStrategy() {

@Override
public boolean approve(ClassAttribute attribute) {

    /* EJB attributes: field and setter */
    if (attribute.getAttribute() != null) {
        return true;
    }

    /* Exclusion for NonEJBPojo class */
    for (Method setter : attribute.getSetters()) {
        if (NonEJBPojo.class.equals(setter.getDeclaringClass())) {
            return true;
        }
    }
    return false;
}

};
Use this strategy with Podam factory

PodamFactory podam = new PodamFactoryImpl().setClassStrategy(classInfoStrategy);
And then attributes get filled:

SpecialPojo pojo = podam.manufacturePojo(SpecialPojo.class);
Assert.assertNotNull("Construction failed", pojo);
Assert.assertNotNull("Attr should not be empty", pojo.getMyString());

I hope this helps.

—
Reply to this email directly or view it on GitHub #86 (comment).

[daivanov]

Well, PODAM already can do it for such simple case. But I see your proposal extremely valuable for another type of beans, those, which have post construction initialization, for example, like this:

package uk.co.jemos.podam.test.dto;

import java.net.MalformedURLException;
import java.net.URL;

import javax.annotation.PostConstruct;

/**
 * POJO , with post construction initialization
 */
public class PostConstructPojo {

    private URL url;

    private String value;

    public String getValue() {
        return value;
    }

    public void setValue(String value) {
        this.value = value;
    }

    @PostConstruct
    public void init() throws MalformedURLException {
        url = new URL(value);
    }

    @Override
    public String toString() {
        return url.toString();
    }
}

[mystarrocks]

True, my example was maybe too simple to demonstrate the exact need and PODAM is definitely capable of handling such cases already. My point was to be able to specify ad-hoc methods to be executed that might affect the state in a desired yet subtle way, not necessarily just the setters like Marco pointed out.

[mtedone]

This has now been implemented in 5.3.1-SNAPSHOT, however Daniil is going to improve my initial commit. Once completed, we’ll be releasing 5.3.1.RELEASE and it’ll be available on Maven central.

Regards,

On 12 Apr 2015, at 18:28, mystarrocks notifications@github.com wrote:

True, my example was maybe too simple to demonstrate the exact need and PODAM is definitely capable of handling such cases already. My point was to be able to specify ad-hoc methods to be executed that might affect the state in a desired yet subtle way, not necessarily just the setters like Marco pointed out.

—
Reply to this email directly or view it on GitHub #86 (comment).

[mystarrocks]

Cheers guys, that was quick!

[mtedone]

Fixed in 5.3.0-SNAPSHOT. Will be released in 5.3.0.RELEASE

[daivanov]

Something like this
daivanov@e9ab2c5

PoDaM user will need to have only these two lines in her/his code:

classInfoStrategy.addExtraMethod(ExtraMethodsPojo.class, "setMyString", String.class);
classInfoStrategy.addExtraMethod(ExtraMethodsPojo.class, "setMyLong", Long.class);

[mtedone]

Perfect! Could you please update also the website docs?

On Tuesday, April 14, 2015, Daniil Ivanov notifications@github.com wrote:

Something like this
daivanov/joinmo@e9ab2c5
daivanov@e9ab2c5

PoDaM user will need to have only these two lines in her/his code:

classInfoStrategy.addExtraMethod(ExtraMethodsPojo.class, "setMyString", String.class);
classInfoStrategy.addExtraMethod(ExtraMethodsPojo.class, "setMyLong", Long.class);

—
Reply to this email directly or view it on GitHub
#86 (comment).

[daivanov]

Sure!

[mtedone]

Does this solution accept more than one argument?

On Tuesday, April 14, 2015, Daniil Ivanov notifications@github.com wrote:

Something like this
daivanov/joinmo@e9ab2c5
daivanov@e9ab2c5

PoDaM user will need to have only these two lines in her/his code:

classInfoStrategy.addExtraMethod(ExtraMethodsPojo.class, "setMyString", String.class);
classInfoStrategy.addExtraMethod(ExtraMethodsPojo.class, "setMyLong", Long.class);

—
Reply to this email directly or view it on GitHub
#86 (comment).

[daivanov]

It's variable length arguments list. What worries me, now ExtraMethodExecutorData encapsulates only java.reflect.Method and nothing else. It's good for extension, but from lean development point of view this is waste.

[mtedone]

We can remove it and use simply Method?

On Tuesday, April 14, 2015, Daniil Ivanov notifications@github.com wrote:

It's variable length arguments list. What worries me, now
ExtraMethodExecutorData encapsulates only java.reflect.Method and nothing
else. It's good for extension, but from lean development point of view this
is waste.

—
Reply to this email directly or view it on GitHub
#86 (comment).

[daivanov]

Now it's released. I noticed one more thing: extra methods are places into HashSet, thus, their execution order is not guaranteed. I believe some users might find it useful as List, when they can be sure in which order extra methods will be invoked.