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

Incorrect current object in RandomizerContext #356

Closed
nrenzoni opened this issue May 23, 2019 · 5 comments
Closed

Incorrect current object in RandomizerContext #356

nrenzoni opened this issue May 23, 2019 · 5 comments
Labels
Milestone

Comments

@nrenzoni
Copy link

nrenzoni commented May 23, 2019

Feature Request (see comment below in getRandomValue()):

import org.jeasy.random.*;
import org.jeasy.random.api.*;
import org.junit.*;
import java.lang.annotation.*;
import static java.lang.annotation.ElementType.FIELD;
import static java.lang.annotation.RetentionPolicy.RUNTIME;

public class Tester {

    private static EasyRandom easyRandom;

    @Before
    public void setup() {
        EasyRandomParameters parameters = new EasyRandomParameters()
                .randomize(FieldPredicates.isAnnotatedWith(ExampleAnnotation.class), new CustomRandomizerContext());
        easyRandom = new EasyRandom(parameters);
    }

    @Test
    public void test() {
        A a = easyRandom.nextObject(A.class);
    }

    static class CustomRandomizerContext implements ContextAwareRandomizer<D> {

        RandomizerContext context;

        @Override
        public void setRandomizerContext(RandomizerContext context) {
            this.context = context;
        }

        @Override
        public D getRandomValue() {
            /*
             Problem: want to access "some value" (see below) of annotation here; need a reference to Field (like below) of the object, or to the enclosing object B b.
            */
            D d = new D();
            d.value = context.getField().getAnnotation(ExampleAnnotation.class).value();
            return d;
        }
    }

    static class A {
        private B b;
    }

    static class B {
        private C c;

        @ExampleAnnotation("some value")
        private D d;
    }

    static class C {
    }

    static class D {
        private String name;
    }

    @Target({FIELD})
    @Retention(RUNTIME)
    @Documented
    public @interface ExampleAnnotation {
        String value();
    }
}
@nrenzoni nrenzoni changed the title Access enclosing object in ContextAwareRandomizer Bug & Feature in ContextAwareRandomizer class May 23, 2019
@nrenzoni
Copy link
Author

Bug found in RandomizerContext.getCurrentObject(), where not always referencing the current object.

  • Since getCurrentObject() implementation in RandomizationContext returns randomizedObject, and this variable is only assigned in setRandomizedObject(), which is only called from doPopulateBean(), but not in all cases. So the correct way to do implement getCurrentObject() would be to use stack in RandomizationContext, and to retrieve the last element from the stack in the case where there are elements, or if the stack is empty, to return the root element.

@nrenzoni
Copy link
Author

I pushed 2 commits to address these 2 respective issues: branch in my fork.

@fmbenhassine
Copy link
Member

Could you please create separate issues (one for the bug and another one for the feature)? From your fork, you can create PRs and I will review them.

Thank you upfront!

@fmbenhassine
Copy link
Member

In regards to the bug

I was able to reproduce the issue. Indeed, RandomizationContext#getCurrentObject is incorrect in the above case, it returns an instance of C where it should return an instance of the enclosing B.

@nrenzoni Your fix LGTM so I applied it. The credit goes to you so thank you! I added you to the contributors list. The fix is deployed in 4.1.0-SNAPSHOT if you want to give it a try already. I will release 4.1.0 in the next few days.

In regards to the requested feature

The requested feature asks to expose the current Field in the context. I believe this is not required now that the bug is fixed and we can correctly access to the enclosing object.

need a reference to Field (like below) of the object, or to the enclosing object B b.

The combination of the full path to the current field (in dotted notation) + the enclosing object is enough to get the field. An example based on the requirement above can be found here. It requires to extract the field name from the full path but that is not an issue IMO.

@fmbenhassine fmbenhassine added this to the 4.1.0 milestone Nov 3, 2019
@fmbenhassine fmbenhassine changed the title Bug & Feature in ContextAwareRandomizer class Incorrect current object in RandomizerContext Nov 3, 2019
@nrenzoni
Copy link
Author

Thank you for taking care of this @benas

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants