Friday, 12 July 2013

Stand-alone repro case for that weird interface bug

G'day:
This is a follow-up to the article from yesterday detailing some weirdness I was getting when using Mockbox to mock methods in CFCs which implemented interfaces.

I have no factored-out Mockbox as a factor in this issue. The tangentially-related issues in Mockbox still stand, but I wanted to have a repro case that did not have any third party code in it. And now I have one.

Here's the code. I.cfc is just a very simple interface, with no surprises:

// I.cfc
interface {

    public string function functionToMock(required string s);

}

This in turn is implemented by Impl.cfc:

// Impl.cfc
component implements="I" {

    public string function functionToMock(required string s){
        return true;
    }

    public string function otherFunction(required string s){
        return true;
    }

}

Note that as well as implementing functionToMock(), it also has an unrelated method, otherFunction(). This will be relevant later.

Next I bring all this together in a test rig:

// repro.cfm
mockedImpl     = new Impl();
writeOutput("Before Mocking<br>");
writeOutput('isInstanceOf(mockedImpl, "I"): ' & isInstanceOf(mockedImpl, "I") & "<br>");
writeOutput('isInstanceOf(mockedImpl, "Impl"): ' & isInstanceOf(mockedImpl, "Impl") & "<br>");
writeOutput("<hr>");

structClear(mockedImpl);
public string function functionToMock(/*required string s*/){    // does not fulfill interface
    return "";
}
mockedImpl.functionToMock = functionToMock;

writeOutput("After Mocking<br>");
writeOutput('isInstanceOf(mockedImpl, "I"): ' & isInstanceOf(mockedImpl, "I") & "<br>");
writeOutput('isInstanceOf(mockedImpl, "Impl"): ' & isInstanceOf(mockedImpl, "Impl") & "<br>");
writeOutput("<hr>");

try {
    writeOutput("Run test<br>");
    function testFunctionToMock(required I impl){
         writeOutput("In testFunctionToMock()<br>");
         impl.functionToMock(s=""); // THIS IS LINE 22
    }
    testFunctionToMock(impl=mockedImpl);

    writeOutput("Mocked object worked<br>");
}
catch (any e){
    writeOutput("Mocked object failed:<br>Exception type: #e.type#<br>Exception message: #e.message#<br>Exception detail: #e.detail#<br>Exception occurred at: #e.tagcontext[1].raw_trace#<br>");
}

So what this does is:
  • create an Impl object;
  • clears all its methods out;
  • injects a new functionToMock, but one that doesn't fulfill the interface requirements;
  • passes the Impl object into a test which requires it to be an implementation of I;
  • and calls the mocked method.
I now start ColdFusion for the first time, and run repro.cfm, and I get this:

Before Mocking
isInstanceOf(mockedImpl, "I"): YES
isInstanceOf(mockedImpl, "Impl"): YES

After Mocking isInstanceOf(mockedImpl, "I"): NO isInstanceOf(mockedImpl, "Impl"): YES
Run test In testFunctionToMock() Mocked object failed: Exception type: Application Exception message: Function argument mismatch. Exception detail: The functionToMock function does not specify the same arguments or arguments in the same order in the shared.git.blogExamples.interfaces.typeCheckBugRepro.Impl ColdFusion component and the shared.git.blogExamples.interfaces.typeCheckBugRepro.I ColdFusion interface. Exception occurred at: at cfreproWithoutTesterMoreTelemetry2ecfm123831794$funcTESTFUNCTIONTOMOCK.runFunction(D:\websites\www.scribble.local\shared\git\blogExamples\interfaces\typeCheckBugRepro\reproWithoutTesterMoreTelemetry.cfm:22)

OK, so reading through this:
  • after creation, the Impl object is an I, and is also an Impl. Correct.
  • After I clear out the methods and inject an incorrect method, it is not an I any more. Yep, fair enough.
  • However I can still pass it into a function which requires it to be an I, and this works fine. So is it an I, or not? It wasn't on the preceding line, but now apparently it is.
  • And once inside the function, I call the method, and nuh-uh, it's not legit again.
Make your frickin' mind up, ColdFusion!

There's a few things wrong here:
  1. It doesn't fulfill the interface contract after I mock it. So isInstanceOf() is correct.
  2. When I specify I need an I (in the method signature), CF isn't actually checking if the passed-in method fulfills the contract or not. However I know for a fact that CF does do this, so it must be referring to out-of-date metadata to do this check.
  3. It's not until the method is called - and the call to it is fine, and would work - that CF then does a proper check and throws its toys. Note the error crops up on line 22, which is where I call the mocked function.
But there's another thing wrong. When I reload that file, I get this:

Before Mocking
isInstanceOf(mockedImpl, "I"): YES
isInstanceOf(mockedImpl, "Impl"): YES

After Mocking isInstanceOf(mockedImpl, "I"): NO isInstanceOf(mockedImpl, "Impl"): YES
Run test In testFunctionToMock() Mocked object worked

Note: I've not changed any code, I've simply reloaded that page. And now everything is, apparently, fine.

I still haven't managed to work out what it is Coldfusion is looking at when it makes these decisions. If I restart CF and reload: error. Run again: pass. Run again: pass. Etc. Well: I say "pass", but it should actually be erroring, because the Impl instance isn't implementing I properly.

So there's a coupla things to fix here:
  1. ColdFusion is not correctly checking whether an object correctly implements an interface when required: ie: when calling a method requiring an argument to be of a type of interface;
  2. having done that, it should not be checking when the method is called;
  3. isInstanceOf() and the function / method type checking is clearly calling different code for the type check. That's just bloody stupid. Refactor it so it's calling the same code, so that ColdFusion can answer simple questions coherently and accurately.

Do you know what though? I ain't finished yet. You know how I had that otherFunction() in my Impl.cfc? Well I changed the repro.cfm code to mock and call that method instead:

mockedImpl     = new Impl();
structClear(mockedImpl);
public string function otherFunction(){
    return "";
}
mockedImpl.otherFunction = otherFunction;
// so functionToMock() is missing from mockedImpl now

writeOutput('isInstanceOf(mockedImpl, "I"): ' & isInstanceOf(mockedImpl, "I") & "<br>");
writeOutput('isInstanceOf(mockedImpl, "Impl"): ' & isInstanceOf(mockedImpl, "Impl") & "<br>");

try {
    function testOtherFunction(required I impl){
         impl.otherFunction(s=""); // THIS IS LINE 15
    }
    testOtherFunction(impl=mockedImpl);
    writeOutput("Mocked object worked<br>");
}
catch (any e){
    writeOutput("Mocked object failed:<br>#e.type#<br>#e.message#<br>#e.detail#<br>#e.tagcontext[1].raw_trace#<br>");
}

And running this after restarting CF yields this:

isInstanceOf(mockedImpl, "I"): NO
isInstanceOf(mockedImpl, "Impl"): YES
Mocked object failed:
Application
CFC shared.git.blogExamples.interfaces.typeCheckBugRepro.Impl does not implement the interface shared.git.blogExamples.interfaces.typeCheckBugRepro.I.
The functionToMock method is not implemented by the component or it is declared as private.
at cfreproHittingNonInterfaceMethodWithoutTester2ecfm1640607630$funcTESTOTHERFUNCTION.runFunction
(D:\websites\www.scribble.local\shared\git\blogExamples\interfaces\typeCheckBugRepro\reproHittingNonInterfaceMethodWithoutTester.cfm:15)

And similarly, the code gets past the type check on the method signature, but errors when the method is called. But... um... I'm not even calling the method that's wrong! That's just really dumb.

TBH, I'm beginning to think that whoever designed and implemented ColdFusion's interface implementation was quite possibly on crack. Or perhaps had a history of crack use and were going cold turkey whilst doing this work. But - really - there's so many things wrong / substandard in how this work has been done, I am really beginning to conclude (as suggsted by Sean), that they're just something to be steered clear of, in CF.

In closing, I ran this code on ColdFusion 9.0.1 and 10.0.11. And Railo 4.1. Railo did not report any errors, so at least it's uniform, but I think it is also wrong here: it should have been reporting errors.

Believe it or not, I have another article to write on interfaces after this one. It's end-of-sprint-not-much-work-to-do day today, so I shall probably crack on with that after lunch.

Righto.

--
Adam