Wednesday, 9 June 2021

CFML: messing around with mixins (part 2)

G'day:

In the previous article - CFML: messing around with mixins (part 1) - I had a look at some code to implement both compile-time and runtime mixins into target objects. Basically one takes a library object (a CFC just full of functions, no statefulness or anything), and stick references to them into a target object so that object can call those methods as if they were its own native method. It was all very proof-of-concept, and I would never recommend anyone actually using the code I wrote, and indeed would never recommend using mixins at all. They just seem like a hack to me: an anti-pattern and poor design.

One drawback of the approach I had taken thus far is that the way of injecting the functions was very naïve. It would only work as a tactic if the functions were very simple, and didn't refer to anything else within their library: no state (which isn't a problem in a library situation), but also no references to other private functions within the library. This is more of a problem. The reason it won't work is that a function statement doesn't bind any context to itself, it just adopts the context of wherever it's run from. Consider this:

// Library.cfc
component {
    function doSomething() {
        // some code
        
        someValue = runPrivateFunction()
        
        // more code
    }
    
    private function runPrivateFunction() {
        // code
    }
}

And using our injector we inject doSomething in to this:

// Object.cfc
component {


    myMethod() {
        // some code
        
        doSomething()
        
        // some more code
    }
}

This won't work. We only mixin a reference to doSomething, and when it's run in the context of Object, it will try to find runPrivateFunction in that object's context, and it won't be there.

We could inject all the private methods into the target as well: that's easy enough. But what if the object we're mixing-in from isn't a simple library, but it's a stateful object, or has dependencies injected into it that its methods then expect to be able to use… we'd have to chuck everything into the target. And by this point one should be questioning what the hell one is doing. It's just rubbish.

Instead of that, I'm just going to bind it to its original context instead. So we just inject the method, but it will still know about its context from the original object it came from. And it seems to be surprisingly easy.

More test cases…

It binds mixed-in methods to their original calling context when using a mixinMap

In this example we have a model class:

// MyModel.cfc
component {

    function getObjects(orderBy="id") {
        return getObjectsFromStorage(orderBy)
    }
}

And you can see it's calling a repository-ish method to getObjectsFromStorage. It just returns those in this example, but it's the sort of thing where you have a request to return some objects, the model tier calls a repository to get the objects, then the model performs business logic on the to round out the requisite business logic, and returns them to whatever asked for them - probably a controller.

You can also se that MyModel doesn't implement that behaviour, we need to inject it as a mixin. The repo is like this:

component {

    function init(MyDao dao) {
        variables.dao = dao
    }

    function getObjectsFromStorage(orderby="id") {
        orderBy = mapOrderBy(orderBy)
        return dao.getRecords(orderBy).reduce((array=[], record) => array.append(new Number(record.mi)))
    }

    private function mapOrderBy(property) {
        propertyColumnMap = {
            "id" = "id",
            "english" = "en",
            "maori" = "mi"
        }
        return propertyColumnMap.keyExists(property) ? propertyColumnMap[property] : "id"
    }
}

This is a proper dependency-injected repository. The repo method knows how to get the DAO to fetch the raw storage data, and it knows how to convert those records into domain objects. It simply does that, it doesn't perform any business logic; just conversion from storage data to domain data.

You can also see how our current mixin injector would break this code: it relies on other objects in its variables scope: the DAO, plus a private method to offload the task of mapping domain property names ot DB columns.

For completeness here's the DAO, although it's not relevant to this exercise other than needing to exist:

component {

    function getRecords(required orderBy) {
        records = queryNew("id,mi,en", "int,varchar,varchar", [
            [1, "tahi", "one"],
            [2, "rua", "two"],
            [3, "toru", "three"],
            [4, "wha", "four"]
        ])

        return queryExecute("SELECT * FROM records ORDER BY #orderBy#", {}, {dbtype="query"})
    }
}

And the test to make sure our mixing-in will all work is as follows:

it("binds mixed-in methods to their original calling context when using a mixinMap", () => {
    di = new advanced.DependencyInjectionImplementor()
    model = new advanced.MyModel()
    di.wireStuffIn(
        model,
        new advanced.MyRepository(new advanced.MyDao()),
        {
            getObjectsFromStorage : {}
        }
    )

    result = model.getObjects(orderBy="maori")

    expect(result).toBeArray()
    expect(result).toHaveLength(4)
    expect(result).toBe([
        new Number("rua"),
        new Number("tahi"),
        new Number("toru"),
        new Number("wha")
    ])
})

When I run the test, you see what I mean about all that context malarcky:


We've injected getObjectsFromStorage, but not the rest of the stuff it needs to work, and they're not magically there in the next context it's being run from. Solving this problem is super easy. Currently we're injecting the method like this (from DependencyInjectionImplementor.cfc):

mixinMap.each((sourceMethod, mapping) => {
    targetMethod = mapping.keyExists("target") ? mapping.target : sourceMethod
    requestedAccess = mapping.keyExists("access") ? mapping.access : "private"

    targetAccess = requestedAccess == "public" ? "public" : "private"

    scopes[targetAccess][targetMethod] = someMixin[sourceMethod]
})

We just need to change it to this:

mixinMap.each((sourceMethod, mapping) => {
    targetMethod = mapping.keyExists("target") ? mapping.target : sourceMethod
    requestedAccess = mapping.keyExists("access") ? mapping.access : "private"

    targetAccess = requestedAccess == "public" ? "public" : "private"

    proxy = () => someMixin[sourceMethod](argumentCollection=arguments)

    scopes[targetAccess][targetMethod] = proxy
})

We wrap the call to the original method in a proxy function, and inject the proxy. This means the method is being called in the context of its original object!

Now the test passes.

It binds mixed-in methods to their original calling context by default

It stands to reason that this is the default behaviour to use, so we'll cross-implement that back into the rest of the injection code, even if we're not using the mixinMap:

it("binds mixed-in methods to their original calling context by default", () => {
    di = new advanced.DependencyInjectionImplementor()
    model = new advanced.MyModel()
    di.wireStuffIn(
        model,
        new advanced.MyRepository(new advanced.MyDao())
    )

    result = model.getObjects(orderBy="english")

    expect(result).toBeArray()
    expect(result).toHaveLength(4)
    expect(result).toBe([
        new Number("wha"),
        new Number("tahi"),
        new Number("toru"),
        new Number("rua")
    ])
})

And the implementation:

private function mixinUsingMixin(someObject, someMixin) {
    someObject.__putVariable = putIntoVariables
    structKeyArray(someMixin).each((methodName) => {
    
        proxy = () => someMixin[methodName](argumentCollection=arguments)

        someObject.__putVariable(someMixin[methodName]proxy, methodName)
    })
    structDelete(someObject, "__putVariable")
}

It doesn't bind mixed-in function to their original calling context if bind option is false

One last thing. Maybe one might want to still use the old-school approach without the proxy for some reason. In this super contrived example we actually want the mixed-in method to bind to the target location's context. Here's a model:

component {

    function init() {
        variables.status = true
    }
}

And the method we want to mix-in:

component {

    function getStatus() {
        return variables.status
    }
}

It's just providing a way to return the status of the target object to calling code. Daft example, but you get the idea. here's the test:

it("doesn't bind mixed-in function to their original calling context if bind option is false", () => {
    di = new advanced.DependencyInjectionImplementor()
    model = new advanced.MyModel()
    di.wireStuffIn(
        model,
        new advanced.MyStatusLib(),
        {getStatus = {access="public", bind=false}}
    )

    result = model.getStatus()

    expect(result).toBeTrue()
})

And the implementation:

private function mixinUsingMap(someObject, someMixin, mixinMap) {
    someObject.__getVariables = getVariables
    scopes = {
        public = someObject,
        private = someObject.__getVariables()
    }
    structDelete(someObject, "__getVariables")

    mixinMap.each((sourceMethod, mapping) => {
        targetMethod = mapping.keyExists("target") ? mapping.target : sourceMethod
        requestedAccess = mapping.keyExists("access") ? mapping.access : "private"

        targetAccess = requestedAccess == "public" ? "public" : "private"
        
        bind = mapping.keyExists("bind") && isBoolean(mapping.bind) ? mapping.bind : true

        proxy = bind
            ? () => someMixin[sourceMethod](argumentCollection=arguments)
            : someMixin[sourceMethod]

        scopes[targetAccess][targetMethod] = proxy
    })
}

(Note that this code does not work on ColdFusion, but works fine on Lucee. I have not worked out what's up with it yet, and will report back once I've looked at it. ColdFusion's parser is getting confused and it gives a compile error. The code is syntactically correct though).

That deals with it, and that's it for this exercise. As I said before don't actually do this. It'll make your code impenetrable to follow, with everyone wondering how you're calling methods that aren't defined in the class yer looking at. It's a shit way of going about things. I'm thinking of a third part to this when the mixing-in is at runtime still, but it's handled in a more explicit fashion in the target class, so the code stays clear. I've not worked out how to do this yet, but I have some ideas and some embryonic code somewhere.

TDD is your friend

I've been adding these test cases iteratively, and making sure all the previous test cases still pass with every update I make to the implementation. This directs how I design the code, and makes sure I take small steps. This is gold. And you are - IMO - a fool if it's not how you work on a daily basis.

 

Righto.

--
Adam

PS: I just noticed I missed a test case in all this. It's related to the last test case I created. Can you spot what I've missed? Hint: it's not some "nice to have" sort of test case; I've actually written some code that I'm not testing, and was not actually related to the case I was trying to fulfil.