Collaborating assemblies broken if hard-wired, must use TyphoonCollaboratingAssemblyProxy

[ratkins]

If I create a TyphoonBlockComponentFactory like so:

    TyphoonComponentFactory *typhoonComponentFactory = [[TyphoonBlockComponentFactory alloc] initWithAssemblies:@[[FooAssembly assembly], [BarAssembly assembly], [BazAssembly assembly]]];

... and BarAssembly contains a definition that refers to a definition in BazAssembly:

- (id)blah {
    return [TyphoonDefinition withClass:[Blah class] properties:^(TyphoonDefinition *definition) {
        [definition injectProperty:@selector(thingy) withDefinition:[[BazAssembly assembly] thingy]];
    }];
}

... Typhoon silently injects nil into my Blah object's thingy property. Changing the order of the assemblies in the component factory definition fixes it.

[rhgills]

The problem might be [definition injectProperty:@selector(thingy) withDefinition:[[BazAssembly assembly] thingy]];

Specifically, the direct call to [BazAssembly assembly] in the definition.

Try adding a property for thingyProvider, and implementing:

- (void)resolveCollaboratingAssemblies
{
    [self setThingyProvider:[BazAssembly assembly];
}

on BarAssembly.

And change the definition to:

[definition injectProperty:@selector(thingy) withDefinition:[[self thingyProvider] thingy]];

[jasperblues]

That's correct @rhgills - I didn't intend the use-case above, you're supposed to resolveCollaboratingAssemblies either to a stand-in or the real thing first. . . . I think we can throw an error you don't do this.

Today, I'm updating the sample app - its gotten rusty, after which I'll have a look at this.

[ratkins]

Oh, I see.

The resolveCollaboratingAssemblies dance is quite a bit of boilerplate though.

[ratkins]

I've done this and it's not working. All the injected properties set with definitions resolved from other files are coming out nil.

[rhgills]

Sorry for taking so long to get back to you on this.

Looks like Typhoon has issues with resolving collaborating assemblies when they are specified 'out-of-order'. So, say you have two assemblies: MainAssembly and AddonAssembly. AddonAssembly contains a definition that refers to a definition in MainAssembly.

This works:

TyphoonComponentFactory* factory = [[TyphoonBlockComponentFactory alloc] initWithAssemblies:@[
        [MainAssembly assembly],
        [AddonAssembly assembly],
]];

This doesn't:

 TyphoonComponentFactory* factory = [[TyphoonBlockComponentFactory alloc] initWithAssemblies:@[
        [AddonAssembly assembly],
        [MainAssembly assembly],
]];

There is a unit test for this, which fails when run alone but incorrectly succeeds when run alongside other unit tests. This occurs because the swizzling performed by the BlockComponentFactory when initializing assemblies persists across test invocations.

@jasperblues:
I'm making the test manually fail for now. I refactored the BlockComponentFactory and Assembly a few days ago to make it easier to undo this swizzling. I'll add in the fix for the tests ASAP, then look into fixing this issue.

@ratkins:
In the meantime, try passing the BazAssembly before the BarAssembly which references it.

[rhgills]

@jasperblues

I captured your idea about throwing an error if resolveCollaboratingAssemblies isn't called in #117.

[jasperblues]

@rhgills Thanks - I also changed this from 'enhancement' to 'bug'. . .

Also thanks for taking the lead on this issue.

[jasperblues]

If you use [TyphoonCollaboratingAssemblyProxy] instead of the real assembly, and then specify the concrete assembly at runtime with [BlockComponentFactory initWithAssemblies:@[a, b, c]] then it will work.

We might want to only support this, and raise meaningful errors in the other cases.

I think the boilerplate is not too bad here, especially if we had a little IDE support. . There's also the option, to detect properties of type TyphoonAssembly, and make them a [TyphoonCollaboratingAssemblyProxy] automatically.

[rhgills]

Just a quick update: I plan to look into this tonight.

[jasperblues]

👍

[rhgills]

Fixed the leaky unit tests in 5a6bc7e.

I think it is a good idea to only support the use of a CollaboratingAssemblyProxy (and to automatically detect TyphoonAssembly properties and assign them a proxy automatically).

If you use a real assembly, we should treat that as an error. It might work if the definitions happen to be in-order, but you shouldn't have to worry about that.

[jasperblues]

👍

Yeah, I agree on all three points:

• Only support collaborating assembly as proxy - provide the real thing at runtime
• Magically wire it in. . . (we don't have to deprecate the old way, because this feature has only been on the main branch for a few weeks).
• Error on attempt to make it something other than a stand-in for runtime supplied assembly.

[rhgills]

Implemented an exception when providing a real assembly in a definition instead of using a collaborating assembly proxy in the commits leading up to a0c01d9.

On a tangential note, what are your feelings on squashing commits? I don't have a habit of doing so, but I was just thinking that it would be nice to have a single commit to go along with the single logical change I just made here, instead of many little commits—which were useful as I was going along so I could back out a change if it proved to be too much, but are less useful now that I'm sure it works okay.

[rhgills]

Next up is magic wiring!

[jasperblues]

I've always done loads of micro commits like yourself. . Jose (one of the early contributors) pointed out a great tool for commit squashing, but we haven't adopted it yet.

If I'm working on a particularly risky feature I usually do this: (works for me)

• git remote set-url origin git@tinsy.blues:Typhoon.git (my local server)
• push push push . . when happy
• git remote set-url origin git@github. . . etc

Thanks for finishing off this feature - it badly needed doing. I was planning on looking at it next. Will you be able to update the sample app as well?

[jgongo]

Hi! Although I haven't been able to contribute any more due to other responsibilities I keep an eye on the project ;o)

@jasperblues I guess the tool you mention is git flow. Here you can have an article from the creator of the tool where he explains the branching model he implemented. The basic idea is that you have per-feature private branches where you can do "micro" commits and then merge that branch into the public development branch, keeping all the branch history so you can easily see the commits belonging to each feature.

[jasperblues]

Thanks Jose! Now's probably a good time to start using this.

[rhgills]

Sounds good to me.

[rhgills]

Implemented magic wiring in the commits leading up to dec03f2.

One interesting aspect that came up was how to determine which properties on a TyphoonAssembly are stand-ins for collaborating assemblies and thus should have a TyphoonCollaboratingAssemblyProxy set upon them as part of the magic wiring process. The solution I went with is to simply to assume that all properties on a TyphoonAssembly subclass are collaborating assemblies.

This seems like a good solution, until if and when we receive some requests to allow user defined properties on a TyphoonAssembly subclass, especially as it is non-trivial to determine which properties are collaborating assemblies. We can't determine the type of the property (including any protocol it is supposed to conform to). We could determine the name of the backing instance variable, to, say, require that it has an 'assembly' in the name and otherwise leave it alone; but this seems less than ideal.

[jasperblues]

Great work Robert.

The objective-C runtime often doesn't supply any type information, however in the case of declared properties it does. Therefore, how about we check the return type of a property is a sub-class of TyphoonAssembly ?

@interface NSObject (TyphoonIntrospectionUtils) <TyphoonIntrospectiveNSObject>

/**
* Returns a Class object or `TyphoonTypeDescriptor` in the case of a primitive type.
*/
- (TyphoonTypeDescriptor*)typeForPropertyWithName:(NSString*)propertyName;

@end

[rhgills]

Thanks!

I looked into the docs for property introspection and got the impression that this was not possible (that all object properties reported that they were of type id, not of type TyphoonAssembly*, for example). But some googling tells me this might not be the case. I'll look into that!

[jasperblues]

Indeed it is actually possible. . . we use it for the following:

• Inject candidate by type (in xml or block assembly) rather than explicit matching
• Typhoon Autowire "annotation" (aka Macro)
• Converting a text property (eg Configuration.properties file) from a text value to the target value

. . this only works for properties unfortunately. . . ivars, initializers, etc don't provide this meta.

[rhgills]

Yup! Implemented in 58e5852.

The documentation carefully tiptoes around this. It has an extensive list of examples, none of which are of non-id type... and mentions that the type information provided is equivalent to that of @encode, which I believe is more limited. Awesome though! And in hindsight, of course it's possible, otherwise auto wiring wouldn't be able to work.

[rhgills]

I'll update the example app when I have a chance (and push out a new minor version).

[jasperblues]

👍 great work

[jasperblues]

@rhgills Great work on the assembly validation. . . for the collaborating proxy definition, I had something a little different in mind. . please hold off on pushes related to this - I'm working in that area. . should be done in a few hours.

[jasperblues]

Fixed.