Follow up to bug: line 902 DefaultXmlBeanFactory

[Chuck Savage]

Here's the fix, you can read the full thread that was discussed at the
Reactor forums. I started the thread there since it was
reactorFactory.cfc that was throwing the initial error. I reported
this on another thread here that hasn't shown up... If it doesn't,
the error is that the init() method in the array md.functions isn't
guaranteed to be called first, and therefore the call to setBeanFactory
() in the beanInstance may rely on data that would be called in the
init() and therefore, as reactorFactory.cfc does indeed require init()
to be called first, throws an error.

http://groups.google.com/group/reactor-users/browse_frm/thread/50770e8ea6b88762

The fix is,

I added this function to DefaultXmlBeanFactory

<cffunction name="pushInit" access="private" output="false"
returntype="array" hint="We parse array, and if 'init' exists, we push
it to front of array">
<cfargument name="functions" type="array" required="yes" hint="we
are the array to parse">
<cfloop from="1" to="#arraylen(functions)#" index="functionIndex">
<!--- check for method init in array --->
<cfif functions[functionIndex].name eq "init">
<!--- found, if first no sorting required and just return --->
<cfif 1 eq functionIndex><cfreturn functions></cfif>
<cfset arraySwap(functions, 1, functionIndex)>
<cfreturn functions>
</cfif>
</cfloop>
<!--- init doesn't exist --->
<cfreturn functions>
</cffunction>

This is inserted at line 867 right after the line, <cfif
structKeyExists(md, "functions")>
<!--- sort array to be sure init method is always first checked (if it
exists) --->
<cfset md.functions = pushInit(md.functions)>

[Sean Corfield]

On Fri, Dec 4, 2009 at 11:13 AM, Chuck Savage <ch...@searisen.com> wrote:
> Here's the fix, you can read the full thread that was discussed at the
> Reactor forums.  I started the thread there since it was
> reactorFactory.cfc that was throwing the initial error.

I reported this bug a while back and my fix was to simply loop over
md.functions twice, looking for init() the first time and the rest of
the logic the second time. Modifying the metadata seems pretty suspect
to me (and returning an array causes it to be copied so that isn't
very efficient). I don't know whether that was ever incorporated into
the core.
--
Sean A Corfield -- (904) 302-SEAN
Railo Technologies US -- http://getrailo.com/
An Architect's View -- http://corfield.org/

"If you're not annoying somebody, you're not really alive."
-- Margaret Atwood

[Brian Kotek]

I'm not either but I'll look. And if it isn't incorporated, it should be. I'll check with Mark as well.

Thanks,

Brian

--

You received this message because you are subscribed to the Google Groups "ColdSpring-Users" group.
To post to this group, send email to coldspri...@googlegroups.com.
To unsubscribe from this group, send email to coldspring-use...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/coldspring-users?hl=en.

[Mark Mandel]

Don't look at me - I've not really touched the CS 1.x core, except to add annotation based pointcuts.

Mark

--
E: mark....@gmail.com
T: http://www.twitter.com/neurotic
W: www.compoundtheory.com

[Chuck Savage]

What I did as a separate function to shift the init method to the front could pass the array as a reference, so it wouldn't copy the array as Sean said.

I knew what I did, wasn't necessarily the most efficient.  It could as well be done inline aswell.

The reason for the pushInit function in my eyes, is the code will get messier the more functions that are added that need to be looped over.

The code I believe Sean and I are both talking about is from 868-905.

[Sean Corfield]

On Mon, Dec 7, 2009 at 9:04 AM, Chuck Savage <ch...@searisen.com> wrote:
> What I did as a separate function to shift the init method to the front
> could pass the array as a reference, so it wouldn't copy the array as Sean
> said.

Functions copy arrays when you pass them as arguments (and when you
assign results to a variable).

> The reason for the pushInit function in my eyes, is the code will get
> messier the more functions that are added that need to be looped over.

Arguably true. It's a trade off of performance vs maintainability and
I'm not sure which way I'd lean on this (given that ColdSpring is not
intended for transients and thus it only runs this code once per
singleton object anyway).

[Chuck Savage]

Well I was curious about passing arrays by reference and came across this article by Ben Nadel.  It may not be practical for an engine like ColdSpring.  Its not in what you pass to a function, but in how the array is defined.  Since arrays are actually java lists.  For more info, read...

http://www.bennadel.com/blog/275-Passing-Arrays-By-Reference-In-ColdFusion-SWEEET-.htm

I like his argument, that ArrayAppend() doesn't return a value, so since ArrayAppend() is actually passing the array by reference, we should be able to do it too.

I haven't played with it, so who knows.  But thought you might like to know if you ever wanted to handle an array by reference.

[Dennis Clark]

Just in case anyone here thought two implementations to fix the same ColdSpring bug was not enough, I'm contributing my own patch to the fray.

I like to think of this as the fix that Sean would have submitted if he hadn't submitted the other fix first :-) . It uses a couple of local boolean variables to to remember the presence of the init and setBeanFactory methods in the loop and shifts the invocation of those methods to after the end of the loop.

IMNSHO my implementation not only avoids multiple loops and juggling of array elements, it also improves the readability of the original code.

Cheers,

Dennis

[Chuck Savage]

sort of off topic, but how do you make a patch?

[Sean Corfield]

On Tue, Dec 8, 2009 at 9:38 PM, Dennis Clark <boom...@gmail.com> wrote:
> I like to think of this as the fix that Sean would have submitted if he
> hadn't submitted the other fix first :-) . It uses a couple of local boolean
> variables to to remember the presence of the init and setBeanFactory methods
> in the loop and shifts the invocation of those methods to after the end of
> the loop.

Yeah, that was the solution I started thinking about after posting my
hack to the list... Thank you!

[Dennis Clark]

A patch file is also known as a diff. The Wikipedia entry for diff explains the diff program and the various diff formats, and includes links to tools that support the creation of diff files.

Diffs/patches against a CVS or SVN repository are usually created using their corresponding repository client tools. Repository managers tend to prefer diffs created this way because they usually include the revision numbers of the original files.

The 1.x branch of ColdSpring is in CVS and I had it checked out as a project in Eclipse with the CVS client plugin, so I was easily able to create the diff from Eclipse via its Team/Create Patch context menu.

On Wed, Dec 9, 2009 at 4:42 PM, Chuck Savage <ch...@searisen.com> wrote:

sort of off topic, but how do you make a patch?

[Gavin Baumanis]

Hi Chuck,

Basically a PATCH is a file that contains the difference between an original file and the state you want it to be in after the Patch is applied.

I.e. (very simply) - lets assume you wan to add a simple copyright notice to the beginning of a file;

Then your patch file would simply contain a few "control" lines (The file path/name, and the line numbers effected by the change) and then the change.

That is - your PATCH file will NOT contain the whole file - Just the information required to get it into the state you want it to be in after the operation completes.

Here is an example that simply changes the year of the copyright notice, by deleting a single line and replacing it with a new one.

The actual Application.cfm file in this instance is about 60 lines in length - but the patch file simply contains information about the lines to be changed (removed / added).

Index: trunk/nz/nsh/Application.cfm

===================================================================

--- trunk/nz/nsh/Application.cfm (revision 8328)

+++ trunk/nz/nsh/Application.cfm (revision 8339)

@@ -7,1 +7,1 @@

-Copyright (C) 2007. PalCare Pty. Ltd.

+Copyright (C) 2009. PalCare Pty. Ltd.

It also shows the revision numbers for this file that this change was effected at too.

By using a versioning control system (Subversion in my case) I could even UNDO the change if it was deemed to be incorrect.

The human benefit of this format is that it is relatively simple to see what the actual change was - as opposed to having to review an entire file.

Hope this helps

On 10/12/2009, at 08:42 , Chuck Savage wrote:

sort of off topic, but how do you make a patch?

--

You received this message because you are subscribed to the Google Groups "ColdSpring-Users" group.
To post to this group, send email to coldspri...@googlegroups.com.
To unsubscribe from this group, send email to coldspring-use...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/coldspring-users?hl=en.

As always, please contact me if I can be of any further assistance.

Gavin "Beau" Baumanis
Senior Application Developer
PalCare P/L

657 Nicholson Street
Carlton North
Victoria, Australia, 3054

E: be...@palcare.com.au
P: +61 -3 9380 3513
M: +61 -438 545 586
W: http://www.palcare.com.au

[Chuck Savage]

Since this apparently hasn't been patched yet, another person on the
MG groups just posted the same error. I felt I should revise my based
on the discussions on these two threads, and the possibility I may end
up sharing it with them.

I removed the extra function that passed the array by value and call
it now inline just before the loop.

I'm having problems accessing the cvs here: http://cvs.coldspringframework.org/coldspring/

Do I have it wrong? I'm using Tortoise svn. I'd like to patch vs
the current cvs.

Here's the code anyway, insert at line 868

<!--- sort array to be sure init method is always first (if it exists)
--->
<cfloop from="1" to="#arraylen(md.functions)#" index="functionIndex">

<!--- check for method init in array --->

<cfif md.functions[functionIndex].name eq "init">
<!--- found, if first no sorting required, else swap and break --->
<cfif 1 neq functionIndex>
<cfset arraySwap(md.functions, 1, functionIndex)>
</cfif>
<!--- else break, cause its first already --->
<cfbreak>
</cfif>
</cfloop>

[Gavin Baumanis]

Hi Chuck,

I could of course be completely wrong...
but I am "petty" sure that you're going to need tortiseCVS and not TortoiseSVN to access / use a CVS repository as opposed to a Subversion one.

Gavin.

> --
>
> You received this message because you are subscribed to the Google Groups "ColdSpring-Users" group.
> To post to this group, send email to coldspri...@googlegroups.com.
> To unsubscribe from this group, send email to coldspring-use...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/coldspring-users?hl=en.

As always, please contact me if I can be of any further assistance.

[Dennis Clark]

I found a link from the ColdSpring RIAForge page to a JIRA issue tracker site, so I used it to file an issue:

http://code.coldspringframework.org/browse/CSP-109

Does anyone here know if this issue tracker is actively being used by the ColdSpring team? It shows only one other ColdSpring issue as having been filed in 2009. It's easy to miss the link to the issue tracker, so if we want the community to actively use it I suggest someone include a link from the main ColdSpring site.

I already have one enhancement patch for ColdSpring 1.2 ready and am in the process of creating another one to support some upcoming work on the Model-Glue framework. It'd be nice to have someone or someplace to send my patches.

Cheers,

Dennis