Scope Concept

[sialcasa]

Currently we rethink the behavior of scopes and we tend to adopt the DI-Mechanism from Angular2 for our Scopes.

The mechanism injects same instances of a type into the views that are in a hierarchy.

Example

Given the a Hierarchie of Views:

ViewHierarchie
ParentView
-ViewA
-ViewC
--ViewD
-ViewB
--ViewC
--ViewD

Szenario 1:

ViewModelHierarchie
ParentViewModel
-ViewAViewModel - @InjectScope MyScope scope; (ObjectID = 1)
--ViewCViewModel - @InjectScope MyScope scope; (ObjectID = 1)
--ViewDViewModel - @InjectScope MyScope scope; (ObjectID = 1)
-ViewBViewModel - @InjectScope MyScope scope; (ObjectID = 2)
--ViewCViewModel - @InjectScope MyScope scope; (ObjectID = 2)
--ViewDViewModel - @InjectScope MyScope scope; (ObjectID = 2)

Szenario 2:

ViewModelHierarchie
ParentViewModel - @InjectScope MyScope scope; (ObjectID = 1)
-ViewAViewModel - @InjectScope MyScope scope; (ObjectID = 1)
--ViewCViewModel - @InjectScope MyScope scope; (ObjectID = 1)
--ViewDViewModel - @InjectScope MyScope scope; (ObjectID = 1)
-ViewBViewModel - @InjectScope MyScope scope; (ObjectID = 1)
--ViewCViewModel - @InjectScope MyScope scope; (ObjectID = 1)
--ViewDViewModel - @InjectScope MyScope scope; (ObjectID = 1)

The principle is, that the highest Injection point decides for the subviews which scope instance is used.

[sialcasa]

Currently there is a prototype implementation on the branch 383_scope_concept

My first impression is, that the mechanism works quite good but there are some things to optimize:

1. Current implementation needs a context that has to get delegated by a parent view that creates a child view using the ViewLoader
2. The descision, where a new scope gets created is currently to implicit and should be more explicit. For example with an Annotation on a ViewModel Class @ScopeProvider(Scope1Type.class,Scope2Type.class) This should ensure, that this View and the Lower Views gets the newly created Scope.

Szenarios for point 2:

Szenario 1:

ViewModelHierarchie
ParentViewModel
-ViewAViewModel - @ScopeProvider(MyScope.class) + (optional) @InjectScope MyScope scope; (ObjectID = 1)
--ViewCViewModel - @InjectScope MyScope scope; (ObjectID = 1)
--ViewDViewModel - @InjectScope MyScope scope; (ObjectID = 1)
-ViewBViewModel - @ScopeProvider(MyScope.class) + (optional) @InjectScope MyScope scope; (ObjectID = 2)
--ViewCViewModel - @InjectScope MyScope scope; (ObjectID = 2)
--ViewDViewModel - @InjectScope MyScope scope; (ObjectID = 2)

Szenario 2

ViewModelHierarchie
ParentViewModel - @ScopeProvider(MyScope.class)
-ViewAViewModel - @InjectScope MyScope scope; (ObjectID = 1)
--ViewCViewModel - @ScopeProvider(MyScope.class) @InjectScope MyScope scope; (ObjectID = 2)
--ViewDViewModel - @InjectScope MyScope scope; (ObjectID = 2)
-ViewBViewModel - @InjectScope MyScope scope; (ObjectID = 1)
--ViewCViewModel - @InjectScope MyScope scope; (ObjectID = 1)
--ViewDViewModel - @InjectScope MyScope scope; (ObjectID = 1)

[gtnarg]

I noticed that there is still no lifecycle for the ScopeStore itself - it's still a singleton. If you open two of these hierarchies side by side - won't they share the same scope instances between them?

[sialcasa]

In this approach (and in the prototype) there is no ScopeStore anymore. Can you check it and tell me what you think?

[gtnarg]

Sorry - I neglected to switch branches - I will definitely check it out and let you know :)

[gtnarg]

I have refactored my code to use the new approach and it works well. I reviewed the code and your implementation is really well done :)

In my use case I needed to do the following:

MyScope myScope = new MyScope();

Context context = new Context();
Map<Class<? extends Scope>, Object> scopeBottich = context.getScopeBottich();
scopeBottich.put( MyScope.class, myScope );

ViewTuple<MyView,MyViewModel> viewTuple = 
    FluentViewLoader
        .javaView(MyView.class)
        .context( context )
        .load();


ViewTuple<MyView2,MyViewModel2> viewTuple2 =
    FluentViewLoader
        .javaView(MyView2.class)
        .context( context )
        .load();

It would be nice to be able to:

Context context = new Context().addScope( myScope );

BTW, Bottich is something that doesn't have English meaning (AFAIK). Other alternatives might be "scopeHolder" or "scopeMap".

[gtnarg]

Re: @ScopeProvider

Enhancing @InjectScope might be more intuitive:

@InjectScope( inheritFromParent="false" )

[sialcasa]

Current WIP:

@ScopeProvider does not work due to technical restrictions (I removed it)

.providedScopes(…) in ViewLoader you can explicit add Scopes for the subview

@InjectContext Resolves the Context of the parent (this is now a Markerinterface - you should not create instances by your own)

.context(context) in ViewLoader provides mvvmFX the Context of the parent. this is nescessary to bypass already existing ApplicationFramework State (for example existing Scopes) from the parent to the child.

[gtnarg]

Tested the updated implementation and it works well.

I agree with not exposing Context creation - providedScopes(...) works well for my use case :)

Re: @ScopeProvider - I think that the default behaviour should be that the framework creates a new instance when it doesn't already exist in the Context. I think this is intuitive default behaviour (i.e. the framework stays out of your way). I think, like originally proposed, @ScopeProvider should only be needed for cases where the default behaviour needs to be overridden.

[sialcasa]

@lestard if we have the scenario that somebody creates the ViewModel by himself to provide the given instance to the ViewLoader, the necessary things like Scopes won't get injected and can't be accessed.

Fixed it here: 39a766d

Now we know what the line we've deleted was for :-)

[manuel-mauky]

I've pushed some tests to reproduce still open problems.

Problem 1

Scope isolation between FXML branches doesn't work
See commit 74567b5. In code this is "Example2".

The example's fxml files have this structure:

        A
      /   \
    B      C

There is one scope type that is injected in B and C.
These scenarios are possible:
a)

• no ScopeProvider is defined.
• expected behaviour: Exception due to missing ScopeProvider
• current behaviour: as expected

b)

• A is a ScopeProvider
• expected behaviour: Both B and C get the same scope instance.
• current behaviour: as expected

c)

• only B is a ScopeProvider
• expected behaviour: B would get the scope injected. But for C there is no ScopeProvider found so an exception with this message should be thrown.
• current behaviour: B and C get the same scope injected. No exception is thrown.

d)

• only C is a ScopeProvider
• expected behaviour: Similar to c) but with an exception thrown for B this time.
• current behaviour: as expected.

The reason for the wrong behaviour of c) is that FXML files are loaded from "left to right" (depth-first search) so that in this case B is loaded before C and because B is a ScopeProvider it is used for C too. While loading we have no clue that B is not above C in the hierarchy.

The following fxml structure would look the same from the point of view of the FXMLLoader:

        A
      /  
    B 
  /
C

The only difference is the order of the initialize calls of the Views. But we have no way of getting to know when these initialize methods are called.

So we have 2 specific problems here:

• the developer don't get a hint when a ScopeProvider is missing in some use cases
• the scope can be accesses outside of the FXML subtree that has the ScopeProvider defined. This means that there is no real isolation of Scopes at the moment.

Problem 2

When a ScopeProvider viewmodel isn't injected into it's view, it is ignored by the ViewLoader.
See commit 89f2762

Take the example from above:

        A
      /   \
    B      C

When A is a ScopeProvider but the AView class doesn't @InjectViewModel the AViewModel
then the ScopeProvider annotation from AViewModel isn't used.
Instead an exception is thrown.

I think this problem isn't too hard to solve but we need to refactor the loading process. At the moment we are based on existing fields in the View class. If no such field for the ViewModel exists we doen't even create a ViewModel instance.

To fix this we have to check if the View has a ViewModel field. If not, we need to check if the generic type of the View (the viewModel type) has a ScopeProvider annotation.

Problem 3

If the developer uses a fx:root approach, it's not possible to use Contexts.
See this example:

public class MyView extends VBox implements FxmlView<MyViewModel> {
    @InjectContext
    private Context context;

    public MyView() {
        FluentViewLoader.fxmlView(MyView.class)
            .codeBehind(this)
            .root(this)
            .context(context)  // not possible because context == null at this point in time
            .load();
    }
}

[gtnarg]

Make ScopeProvider implicit ( first time it's asked for it gets created ).

If you don't want to inherit a scope then @InjectScope( inheritFromParent="false")

You don't have to worry about ScopeProvider exceptions, order of loading etc.

[manuel-mauky]

@gtnarg
Making the ScopeProvider implicit would be an option but the downside of this approach is that it's harder to reason about when a scope is created and where the same scope instance is visible.
With the ScopeProvider annotation this is explicit.

About your "inheritFromParent=false" proposal:
As a developer at the place where you inject the scope you don't want to decide how the scope is created and where the scope instance comes from. Otherwise you would introduce new coupling.

In a component you only want to say "I need an instance of Scope XY". This way you can reuse the component in different places by managing the scope that is used by the component from outside.

In the above examples of "Problem A" there is a scenario possible:
Both B and C are defining their own ScopeProvider. This way the both get an independent instance of the Scope and everything works fine.
The problem is that if the developer forgets one annotation it doesn't work as expected anymore and in some use cases (like c) the developer doesn't even get to now about the false behaviour because we can't find out at runtime. This makes the whole Scopes feature brittle to use.

[sialcasa]

Solution for problem 1 would be:

Remove / delete the scope instance in the context when the initialize method of the view with the ScopPprovider annotated ViewModel. This is possible due to the loading / initialize order of the view hierarchy.

Therefore we would need a callback of the FXMLLoader when a initialize method of a controller/code behind was called.

Possible solutions:

1. Bytecode manipulation to get a pre hook on the initialize method
2. Forked fxml loader to implement a pre hook on the initialize method

[gtnarg]

"As a developer at the place where you inject the scope you don't want to decide how the scope is created and where the scope instance comes from."

Everywhere @ScopeProvider is declared in the test cases, a matching @InjectScope exists - breaking the rule above.

@InjectScope( inheritFromParent="false" ) == @ScopeProvider + @InjectScope

One is needed occasionally - the other is needed every time (as I understand it) - convention over configuration should cover the common case (making reasoning easier). Perhaps the failing test cases (and possible workarounds) are the consequence of a design that is hard to reason about.

Decent logging should cover "doesn't work as expected".

[sialcasa]

@gtnarg
A
/
B C
/ \ /
D E F G

D + E does have a shared @InjectScope MyScope scope;
F + G does have a shared @InjectScope MyScope scope;

With @ScopeProvider(scopes={MyScope.class}) you can annotate B and C, with the inheritFromParent you would have to inject a Scope into B and C to create new Scope instances.

[gtnarg]

@sialcasa - the design makes more sense now - thanks for clarifying.

[sialcasa]

You're welcome. Any ideas for the last technical problem? :-)

[manuel-mauky]

We will release the current scopes implementation with the next version 1.5.0 and mark it as Alpha state which means that the API will probably change in the future.
This way we can test the existing API and search for solutions for the open problems.

I will let this issue open to keep the discussion in a single place

[manuel-mauky]

Hi everyone,
I have two informations regarding this issue:

1. At Saxonia Systems AG we have a student who is working on his bachelor thesis with the topic of finding errors with AOP when using mvvm and JavaFX. As a part of this he is working on a way to find out misconfigurations with scopes. Using a full AOP implementation like AspectJ maybe leads to performance issues at runtime but we are thinking about a utility that only runs during development and not in production. This way the developer can find and fix problems before shipping the product.
   However, our main concerns with this approach are incompatibility problems with other third-party libraries and an error-prone configuration.
2. I'm working on a prototype to analyze the structure of the FXML graph with recursive resolution of included FXML files. This way we can also check if the configuration of scopes is correct or not.
   There are multiple possible ways of using such a tool.
   1. We could include it in the FluentViewLoader so that it scans the graph before loading it. However this would also result in a performance reduction. For this reason I doubt this is a suitable solution. But maybe this could also use some sort of "dev-mode" switch so that the validation only happens during development.
   2. We can create a test helper that the developer can use to create a simple one-liner unit test for the root view. This way the developer get's notified by the build server when someone introduces a wrong scope configuration by accident.
   3. I can imagine to build a graphical tool that developers can use to analyze their application. In such a tool we could also integrate other interesting informations about the application structure.

What do you think?

[sideisra]

I am currently trying to use the new scope feature in our project and got a strange behaviour.

I created a new scope and passed it to the FluentViewLoader via providedScopes(). I expected the newly created scope to be injected into my view models but got a new one instatiated by the di container. The reason for this behaviour was the definition of @ScopeProvider on the top level view model.

I think it would be better to get the provided scope injected no matter if a @ScopeProvider has been set or not.

What do you think?

[manuel-mauky]

Hi denny,
yes this sounds reasonable. The providedScopes should have a higher priority then the ScopeProvider.

On the other hand there might be examples where this is confusing. See this example:

       A
     /   \
    B     C

Both B and C have a @ScopeProvider annotation. When loading A you provide an existing Scope. This would mean that both B and C are getting the same instance of the scope. This would violate the scope isolation doesn't it? B and C are designed with the expectation to get a fresh isolated scope instance which is expressed by the @ScopeProvider annotation.

I tend to still think that the providedScope should have a higher priority (like what you requested) but I'd like to discuss such corner cases that might speak against this.

[sideisra]

Interesting point but the scenario sounds strange. When B and C need a scope of the same type but different instances then A should not use a scope of this type. It would be very confusing no matter what mechanism mvvmfx provides. With this in mind i think that the FluentViewLoader should throw an exception in this situation to avoid a confusing mixture of scope instances.

[sideisra]

Another point: i am still refactoring our app to use the new scope mechanism and have some trouble with testing of view models. Due to the injection of the scope via @InjectScope i cannot fully set up the class under test via the constructor. I have to inject the scope manually (via reflection) and call initialize afterwards to correctly initialize the class under test.

I thought about another possibility. Would it be possible to allow the @InjectScope annotation on a method? an initialiez method could be annotated with @InjectScope and take the dessired scopes as parameter. This approach has to advantages:

• the initialization of the instance could be finalized in this method
• when testing the class, the test code could instantiate the class and inject the scopes via method call

[manuel-mauky]

1. The scopes feature was inspired by how angular DI works and as far as I understand it in angular a provider in a component has a higher priority then a provided service when the root component is loaded. This means that angular works like mvvmFX in the current version. For me this is an indicator that the question of what the "right" behavior is, is not that obvious and we should discuss this a little longer before we change the bahavior that might introduce other issues.

2. Regarding @InjectScope: I can understand your request but using another initialize method wouldn't be idiomatic for mvvmFX and for JavaFX in general. In JavaFX controllers all ui components are injected with @FXML and then are initialized in the initialize method. This is also the approach we are using for all other resources that are provided by mvvmFX. Introducing another approach only for Scopes doesn't sound like a good idea to me.

When it comes to testing I would make the field of the Scope protected. This way you can set the field in your test case and then invoke the initialize method by hand. I don't see any problems with this approach.

It's actually a common discussion when it comes to dependency injection in general on wether it's better to use constructor injection, field injection or method injection.

[sideisra]

1. when its angular standard it would be ok for me to keep that behaviour.

2. ok i understand the motivation to use the same principle as JavaFX. Personally i don't like to use protected fields. Such an enhanced initialize method could be provided as additional possibility to initialize.