Dominic Jones reported a problem with SideEffects being exectued more than once. Test and fix for that.
Project: http://git-wip-us.apache.org/repos/asf/zest-qi4j/repo Commit: http://git-wip-us.apache.org/repos/asf/zest-qi4j/commit/116fa089 Tree: http://git-wip-us.apache.org/repos/asf/zest-qi4j/tree/116fa089 Diff: http://git-wip-us.apache.org/repos/asf/zest-qi4j/diff/116fa089 Branch: refs/heads/master Commit: 116fa089c55f3e45cd03244337344133f58a994c Parents: b49eca6 Author: Niclas Hedhman <[email protected]> Authored: Sat May 24 23:47:30 2014 +0200 Committer: Niclas Hedhman <[email protected]> Committed: Sat May 24 23:47:30 2014 +0200 ---------------------------------------------------------------------- .../bootstrap/CompositeAssemblyImpl.java | 79 +++++++++--------- .../sideeffects/SampleTransientTest.java | 84 ++++++++++++++++++++ .../sideeffects/SpecificSideEffectTest.java | 2 +- .../src/test/java/org/qi4j/test/ASMTest.java | 2 +- 4 files changed, 123 insertions(+), 44 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zest-qi4j/blob/116fa089/core/runtime/src/main/java/org/qi4j/runtime/bootstrap/CompositeAssemblyImpl.java ---------------------------------------------------------------------- diff --git a/core/runtime/src/main/java/org/qi4j/runtime/bootstrap/CompositeAssemblyImpl.java b/core/runtime/src/main/java/org/qi4j/runtime/bootstrap/CompositeAssemblyImpl.java index 61eb547..7441390 100644 --- a/core/runtime/src/main/java/org/qi4j/runtime/bootstrap/CompositeAssemblyImpl.java +++ b/core/runtime/src/main/java/org/qi4j/runtime/bootstrap/CompositeAssemblyImpl.java @@ -173,9 +173,10 @@ public abstract class CompositeAssemblyImpl compositeMethodsModel = new CompositeMethodsModel( mixinsModel ); // Implement composite methods - Iterable<Class<? extends Constraint<?, ?>>> constraintClasses = constraintDeclarations( this.types ); - Iterable<Class<?>> concernClasses = flatten( concerns, concernDeclarations( this.types ) ); - Iterable<Class<?>> sideEffectClasses = flatten( sideEffects, sideEffectDeclarations( this.types ) ); + ArrayList<Type> allTypes = getTypes( this.types ); + Iterable<Class<? extends Constraint<?, ?>>> constraintClasses = constraintDeclarations( getTypes( this.types ) ); + Iterable<Class<?>> concernClasses = flatten( concerns, concernDeclarations( allTypes ) ); + Iterable<Class<?>> sideEffectClasses = flatten( sideEffects, sideEffectDeclarations( allTypes ) ); Iterable<Class<?>> mixinClasses = flatten( mixins, mixinDeclarations( this.types ) ); implementMixinType( types, constraintClasses, concernClasses, sideEffectClasses, mixinClasses ); @@ -683,20 +684,12 @@ public abstract class CompositeAssemblyImpl @SuppressWarnings( "unchecked" ) private Iterable<Class<? extends Constraint<?, ?>>> constraintDeclarations( Class<?> type ) { - Iterable<? extends Class<?>> iterable = iterable( type ); - return constraintDeclarations( iterable ); + ArrayList<Type> allTypes = getTypes( type ); + return constraintDeclarations( allTypes ); } - private Iterable<Class<? extends Constraint<?, ?>>> constraintDeclarations( Iterable<? extends Class<?>> typess ) + private Iterable<Class<? extends Constraint<?, ?>>> constraintDeclarations( ArrayList<Type> allTypes ) { - // Find constraint declarations - List<Type> allTypes = new ArrayList<>(); - for( Class<?> type : typess ) - { - Iterable<Type> types = typesOf( type ); - addAll( allTypes, types ); - } - // Find all constraints and flatten them into an iterable Function<Type, Iterable<Class<? extends Constraint<?, ?>>>> function = new Function<Type, Iterable<Class<? extends Constraint<?, ?>>>>() { @@ -722,27 +715,11 @@ public abstract class CompositeAssemblyImpl private Iterable<Class<?>> concernDeclarations( Class<?> type ) { Iterable<? extends Class<?>> iterable = iterable( type ); - return concernDeclarations( iterable ); + return concernDeclarations( getTypes( iterable ) ); } - private Iterable<Class<?>> concernDeclarations( Iterable<? extends Class<?>> typess ) + private Iterable<Class<?>> concernDeclarations( ArrayList<Type> allTypes ) { - // Find concern declarations - ArrayList<Type> allTypes = new ArrayList<>(); - for( Class<?> type : typess ) - { - Iterable<Type> types; - if( type.isInterface() ) - { - types = typesOf( type ); - } - else - { - types = cast( classHierarchy( type ) ); - } - addAll( allTypes, types ); - } - // Find all concerns and flattern them into an iterable Function<Type, Iterable<Class<?>>> function = new Function<Type, Iterable<Class<?>>>() { @@ -768,19 +745,11 @@ public abstract class CompositeAssemblyImpl protected Iterable<Class<?>> sideEffectDeclarations( Class<?> type ) { Iterable<? extends Class<?>> iterable = iterable( type ); - return sideEffectDeclarations( iterable ); + return sideEffectDeclarations( getTypes( iterable ) ); } - protected Iterable<Class<?>> sideEffectDeclarations( Iterable<? extends Class<?>> typess ) + protected Iterable<Class<?>> sideEffectDeclarations( ArrayList<Type> allTypes ) { - // Find side-effect declarations - ArrayList<Type> allTypes = new ArrayList<>(); - for( Class<?> type : typess ) - { - Iterable<Type> types = typesOf( type ); - addAll( allTypes, types ); - } - // Find all side-effects and flattern them into an iterable Function<Type, Iterable<Class<?>>> function = new Function<Type, Iterable<Class<?>>>() { @@ -802,6 +771,32 @@ public abstract class CompositeAssemblyImpl return toList( flatten ); } + private ArrayList<Type> getTypes( Class<?> type ) + { + Iterable<? extends Class<?>> iterable = iterable( type ); + return getTypes( iterable ); + } + + private ArrayList<Type> getTypes( Iterable<? extends Class<?>> typess ) + { + // Find side-effect declarations + ArrayList<Type> allTypes = new ArrayList<>(); + for( Class<?> type : typess ) + { + Iterable<Type> types; + if( type.isInterface() ) + { + types = typesOf( type ); + } + else + { + types = cast( classHierarchy( type ) ); + } + addAll( allTypes, types ); + } + return allTypes; + } + @SuppressWarnings( "unchecked" ) protected Iterable<Class<?>> mixinDeclarations( Class<?> type ) { http://git-wip-us.apache.org/repos/asf/zest-qi4j/blob/116fa089/core/runtime/src/test/java/org/qi4j/runtime/sideeffects/SampleTransientTest.java ---------------------------------------------------------------------- diff --git a/core/runtime/src/test/java/org/qi4j/runtime/sideeffects/SampleTransientTest.java b/core/runtime/src/test/java/org/qi4j/runtime/sideeffects/SampleTransientTest.java new file mode 100644 index 0000000..6a0d4f6 --- /dev/null +++ b/core/runtime/src/test/java/org/qi4j/runtime/sideeffects/SampleTransientTest.java @@ -0,0 +1,84 @@ +/* + * Copyright (c) 2014, Niclas Hedhman. All Rights Reserved. + * Copyright (c) 2014, Dominic Jones. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.qi4j.runtime.sideeffects; + +import org.junit.Test; +import org.qi4j.api.common.UseDefaults; +import org.qi4j.api.composite.TransientBuilderFactory; +import org.qi4j.api.injection.scope.Structure; +import org.qi4j.api.mixin.Mixins; +import org.qi4j.api.property.Property; +import org.qi4j.api.sideeffect.SideEffectOf; +import org.qi4j.api.sideeffect.SideEffects; +import org.qi4j.bootstrap.AssemblyException; +import org.qi4j.bootstrap.ModuleAssembly; +import org.qi4j.test.AbstractQi4jTest; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.junit.Assert.assertThat; + +public class SampleTransientTest extends AbstractQi4jTest +{ + + @Structure + TransientBuilderFactory transientBuilderFactory; + + @Override + public void assemble( ModuleAssembly module ) + throws AssemblyException + { + module.transients( SampleTransient.class ); + } + + @Test + public void givenTransientWithSideEffectsWhenInvokingMethodShouldExecuteSideEffectOnlyOnce() + { + SampleTransient sample = transientBuilderFactory.newTransient( SampleTransient.class ); + sample.execute(); + assertThat( sample.count().get(), equalTo(1)); + } + + @SideEffects( SampleSideEffect.class ) + @Mixins( SampleTransientMixin.class ) + public static interface SampleTransient + { + void execute(); + + @UseDefaults + Property<Integer> count(); + } + + public abstract static class SampleTransientMixin + implements SampleTransient + { + @Override + public void execute() + { + System.out.println( "Invocation of Transient" ); + } + } + + public abstract static class SampleSideEffect extends SideEffectOf<SampleTransient> + implements SampleTransient + { + @Override + public void execute() + { + System.out.println( "Invocation of SideEffect" ); + count().set( count().get() + 1 ); + } + } +} http://git-wip-us.apache.org/repos/asf/zest-qi4j/blob/116fa089/core/runtime/src/test/java/org/qi4j/runtime/sideeffects/SpecificSideEffectTest.java ---------------------------------------------------------------------- diff --git a/core/runtime/src/test/java/org/qi4j/runtime/sideeffects/SpecificSideEffectTest.java b/core/runtime/src/test/java/org/qi4j/runtime/sideeffects/SpecificSideEffectTest.java index 7bbbd73..21f8c4e 100644 --- a/core/runtime/src/test/java/org/qi4j/runtime/sideeffects/SpecificSideEffectTest.java +++ b/core/runtime/src/test/java/org/qi4j/runtime/sideeffects/SpecificSideEffectTest.java @@ -50,7 +50,7 @@ public class SpecificSideEffectTest Property<Integer> count = some.count(); assertThat( "count is zero", count.get(), equalTo( 0 ) ); some.doStuff(); - assertThat( "count is not zero", count.get(), not( equalTo( 0 ) ) ); + assertThat( "count is not zero", count.get(), equalTo( 1 ) ); } @SideEffects( CounterSideEffect.class ) http://git-wip-us.apache.org/repos/asf/zest-qi4j/blob/116fa089/core/runtime/src/test/java/org/qi4j/test/ASMTest.java ---------------------------------------------------------------------- diff --git a/core/runtime/src/test/java/org/qi4j/test/ASMTest.java b/core/runtime/src/test/java/org/qi4j/test/ASMTest.java index f0cbc6c..797fc0a 100644 --- a/core/runtime/src/test/java/org/qi4j/test/ASMTest.java +++ b/core/runtime/src/test/java/org/qi4j/test/ASMTest.java @@ -143,7 +143,7 @@ public class ASMTest } @Test - @Ignore( "This was dead commented code, ~70% of this source file ... What should we do about this!?!" ) + @Ignore( "Convenience to look at what code is generated in the Fragment Classloader, and is not really a test case." ) public void fragmentClassLoaderGenerateClassTest() throws Exception {
