Issue Details (XML | Word | Printable)

Key: CASTOR-2604
Type: New Feature New Feature
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Werner Guttmann
Reporter: Herve Boutemy
Votes: 0
Watchers: 1
Operations

If you were logged in you would be able to see more operations.
castor

add AbstractJClass.addSourceCode(String) to allow addition of arbitrary source into the generated class

Created: 19/Dec/08 08:54 AM   Updated: 30/Jul/09 04:36 PM   Resolved: 31/Dec/08 03:49 AM
Return to search
Component/s: XML code generator
Affects Version/s: 1.3 rc1
Fix Version/s: 1.3

Time Tracking:
Original Estimate: Not Specified
Remaining Estimate: 0 minutes
Remaining Estimate - 0 minutes
Time Spent: 1 hour
Time Spent - 1 hour

File Attachments: 1. File CASTOR-2604.diff (1.0 kB)
2. Text File patch.c2604.20081230-002.txt (6 kB)
3. Text File patch.c2604.20081230.txt (4 kB)

Issue Links:
Related
 
dependent
 


 Description  « Hide

Modello contains an old fork of javasource package (done in march 2004).
Now I'l like to remove this fork and use Castor's codegen component: see MODELLO-102.
But there is one trivial feature missing: Modello's javasource has a JClass.addSourceCode(String) method to add an arbitrary to the generated source code.
This feature permits a user insert code in his Modello's model file (add a [[<codeSegment>]] in his [[model.mdo]] file).
For example, Maven's [[model.mdo]] uses this feature: you can see it online.

Inkow this feature is neither "clean" nor useful for Castor, but this represents few code and would permit to directly use the component from Modello (or from any other project need such things) instead of maintaining forked code



Herve Boutemy made changes - 19/Dec/08 08:58 AM
Field Original Value New Value
Link This issue is related to MODELLO-102 [ MODELLO-102 ]
Werner Guttmann made changes - 20/Dec/08 12:40 PM
Assignee Werner Guttmann [ wguttmn ]
Werner Guttmann added a comment - 20/Dec/08 12:46 PM

I don't see a reason why this should not be added. But let le ask you one question: if we add such a method to AbstractJClass, where in the generated source code should this arbitrary code fragment be added/inserted ? What does this method look like in your fork ?

PS Is this actually ''the'' fork many folks have mentioned to me over the past three years in terms of 'Maven forked Castor ...." ?


Werner Guttmann added a comment - 20/Dec/08 12:48 PM

Just looking at some of the Modello stuff you've referenced above, I wonder whether it would make more sense to provide you with additional hooks that do not rely on Strings, but on Field, Method, ..... ?


Herve Boutemy added a comment - 20/Dec/08 04:01 PM

Thanks for your help.

where in the generated source code should this arbitrary code fragment be added/inserted ? What does this method look like in your fork ?

Here is a small patch showing the change done in Modello's javasource: I finally wrote it in JClass.
The code is inserted at the end of the generated source, after inner classes. But there is no real point in having this code here: anywhere else would be the same: if you think another location is better, don't hesitate

PS Is this actually ''the'' fork many folks have mentioned to me over the past three years in terms of 'Maven forked Castor ...." ?

I suppose: Maven itself does not contain another javasource copy, Modello was written to do code generation for Maven's model
I'm working on this part of Modello only for a few months, and I needed some Googling to discover where this package came from

I wonder whether it would make more sense to provide you with additional hooks that do not rely on Strings, but on Field, Method, ..... ?

I need simple things that has a simple String as argument, since Modello user writes the code in a model.mdo XML file: I don't see how other things that Strings could be provided.
What could be generalized would be to allow custom free code to be inserted not only at class-source level, but in generated methods too. In therory, this makes sens, but I don't really see what a user would add at the end of a method (before return statement, of course). In a class, a user can add a lot of useful things: attributes, methods, even inner classes. But in methods...


Herve Boutemy made changes - 20/Dec/08 04:01 PM
Attachment CASTOR-2604.diff [ 38943 ]
Herve Boutemy made changes - 22/Dec/08 04:43 PM
Link This issue is depended upon by MODELLO-148 [ MODELLO-148 ]
Werner Guttmann added a comment - 30/Dec/08 01:29 PM

Herve, in your patch, you have added the new code to JClass whereas this issue's title talks about JAbstractClass. Is this change on intention ?


Werner Guttmann added a comment - 30/Dec/08 01:30 PM

In addition to adding new code to JClass.print(), our Velocity templates need to be changed as well.


Werner Guttmann added a comment - 30/Dec/08 01:33 PM

Just for reference, that would be a change to /castor2607/codegen/src/main/resources/org/exolab/castor/builder/printing/templates/main.vm et alias.


Werner Guttmann added a comment - 30/Dec/08 02:16 PM

Initial patch for review.


Werner Guttmann made changes - 30/Dec/08 02:16 PM
Attachment patch.c2604.20081230.txt [ 39084 ]
Herve Boutemy added a comment - 30/Dec/08 02:29 PM

Herve, in your patch, you have added the new code to JClass whereas this issue's title talks about JAbstractClass. Is this change on intention ?

Yes, I initially thought JAbstractClass would be more general. But I didn't find what could be done somewhere else from JClass: then I chose to add the code only where useful.
If you think you can do something better with the code in JAbstractClass, no problem for me

Initial patch for review.

Ok for me


Werner Guttmann added a comment - 30/Dec/08 02:39 PM

You are definitely right in that context. But with regards to code coherence, I decided to move things up the hierarchy, I will commit the code as is (most likely) , and deal with the Velocity changes through a sub-task or similar.


Werner Guttmann added a comment - 30/Dec/08 02:40 PM

Can I actually take it that you are not using the Velocity mode of the XML code generator ?


Werner Guttmann added a comment - 30/Dec/08 03:51 PM

Updatd patch, incl. changes to the existing Velocity templates.


Werner Guttmann made changes - 30/Dec/08 03:51 PM
Attachment patch.c2604.20081230-002.txt [ 39086 ]
Herve Boutemy added a comment - 30/Dec/08 05:07 PM

Can I actually take it that you are not using the Velocity mode of the XML code generator ?

exactly
the fork for Modello was done at a time when this mode didn't exist
and for the moment, I don't intend to change that, unless you tell me this mode is largely superior and could simplify the existing (working) code

the latest patch is ok for me

thank you


Werner Guttmann added a comment - 31/Dec/08 03:26 AM

... unless you tell me this mode is largely superior and could simplify the existing (working) code

No, it does not, to my knowledge. But with release 1.3 we'll start deprecating the old code, as we'd like to achieve a few things:

a) Not maintain two code bases for class generation from JClass instances.
b) Drop all those JStructure.print(...) methods with all the mess coded in there.

Given your 'sophisticated' usage of the XML code generator, I wonder whether you could briefly switch to the new Velocity mode and report back any findings you have.

the latest patch is ok for me

great. I'll commit the patch later on today and deploy a (hopefully) last snapshot release of 1.3 within a day or so.


Werner Guttmann logged work - 31/Dec/08 03:49 AM
Time Worked: 1 hour
Code integration & testing.
Werner Guttmann made changes - 31/Dec/08 03:49 AM
Resolution Fixed [ 1 ]
Status Open [ 1 ] Resolved [ 5 ]
Werner Guttmann made changes - 31/Dec/08 03:49 AM
Time Spent 1 hour [ 3600 ]
Remaining Estimate 0 minutes [ 0 ]
Ralf Joachim made changes - 22/Jan/09 11:04 AM
Fix Version/s 1.3.1 [ 14004 ]
Fix Version/s 1.3 [ 14917 ]
Ralf Joachim made changes - 24/Jan/09 11:12 AM
Status Resolved [ 5 ] Closed [ 6 ]
Ludovic Maître added a comment - 23/Jul/09 02:01 PM

Hi,

I think we should add this to JStructure instead of JAbstractClass, Modello use it on a JInterface when it generate code.
Best regards,


Ludovic Maître added a comment - 23/Jul/09 02:37 PM

Hi,

I have done a patch, which move sourcecode entries from AbstractJClass to JInterface, with it Modello "modello-plugin-java" work with Castor (because in modello we can add sourcecode to the interface too). The unit tests still works for Castor codegen component, and the unit test named org.codehaus.modello.plugin.java.javasource.JavaSourceTest in Modello succeed. More details regarding Modello are or will be on MODELLO-148 .

The patch is here because i cannot attach a file nor reopen this issue (it also fix CASTOR-2769 sorry for the mess , i attach it as a file to 2769 if it's more easy for you ):

Index: src/main/java/org/exolab/javasource/AbstractJClass.java
===================================================================
— src/main/java/org/exolab/javasource/AbstractJClass.java (revision 8289)
+++ src/main/java/org/exolab/javasource/AbstractJClass.java (working copy)
@@ -49,17 +49,7 @@
/** A collection of inner classes for this JClass. */
private Vector<JClass> _innerClasses;

  • private Vector<String> _sourceCodeEntries = new Vector<String>();
    -
    /**
  • * Returns a collection of (complete) source code fragments.
  • * @return A collection of (complete) source code fragments.
  • */
  • public String[] getSourceCodeEntries() { - return _sourceCodeEntries.toArray(new String[_sourceCodeEntries.size()]); - }
    -
  • /**
  • Creates a new AbstractJClass with the given name.
    *
  • @param name The name of the AbstractJClass to create.
    @@ -731,21 +721,6 @@
    jsw.writeln();
    }
    }
  • protected final void printSourceCodeFragments(final JSourceWriter sourceWriter) {
  • if (!_sourceCodeEntries.isEmpty()) { - sourceWriter.writeln(); - sourceWriter.writeln(" //----------------------------------/"); - sourceWriter.writeln(" //- Injected source code fragments -/"); - sourceWriter.writeln("//----------------------------------/"); - sourceWriter.writeln(); - }
  • for (String sourceCode : _sourceCodeEntries) { - sourceWriter.writeln(sourceCode); - sourceWriter.writeln(); - }
  • }

/**

  • Writes to the JSourceWriter all inner classes belonging to this class.
    @@ -767,17 +742,5 @@
    }
    }
    }
    -
  • /**
  • * Adds a complete source code fragment (method) to this class.
  • * @param sourceCode The complete source code fragment to be added.
  • */
  • public void addSourceCode(final String sourceCode) { - _sourceCodeEntries.add(sourceCode); - }
    -
  • public final int getSourceCodeEntryCount() { - return _sourceCodeEntries.size(); - }

}
Index: src/main/java/org/exolab/javasource/JInterface.java
===================================================================
— src/main/java/org/exolab/javasource/JInterface.java (revision 8289)
+++ src/main/java/org/exolab/javasource/JInterface.java (working copy)
@@ -83,7 +83,7 @@
public JInterface(final String name) { super(name); - _fields = null; + //_fields = null; _methods = new Vector<JMethodSignature>(); //-- initialize default Java doc Index: src/main/java/org/exolab/javasource/JStructure.java =================================================================== --- src/main/java/org/exolab/javasource/JStructure.java (revision 8289) +++ src/main/java/org/exolab/javasource/JStructure.java (working copy) @@ -76,6 +76,8 @@ /** A standard complaint for a bad parameter. */ private static final String JSW_SHOULD_NOT_BE_NULL = "argument 'jsw' should not be null."; + private Vector<String> _sourceCodeEntries = new Vector<String>(); + //-------------------------------------------------------------------------- /** The source header. */ @@ -591,5 +593,39 @@ return getName(); }

+ /**
+ * Returns a collection of (complete) source code fragments.
+ * @return A collection of (complete) source code fragments.
+ */
+ public String[] getSourceCodeEntries() { + return _sourceCodeEntries.toArray(new String[_sourceCodeEntries.size()]); + }
+
+ protected final void printSourceCodeFragments(final JSourceWriter sourceWriter) {
+ if (!_sourceCodeEntries.isEmpty()) { + sourceWriter.writeln(); + sourceWriter.writeln(" //----------------------------------/"); + sourceWriter.writeln(" //- Injected source code fragments -/"); + sourceWriter.writeln("//----------------------------------/"); + sourceWriter.writeln(); + }
+ for (String sourceCode : _sourceCodeEntries) { + sourceWriter.writeln(sourceCode); + sourceWriter.writeln(); + }
+
+ }
+
+ /**
+ * Adds a complete source code fragment (method) to this class.
+ * @param sourceCode The complete source code fragment to be added.
+ */
+ public void addSourceCode(final String sourceCode) { + _sourceCodeEntries.add(sourceCode); + }
+
+ public final int getSourceCodeEntryCount() { + return _sourceCodeEntries.size(); + }
//--------------------------------------------------------------------------
}


Werner Guttmann added a comment - 30/Jul/09 04:36 PM

Ludovic, mind creating a completely new Jira issue and attach the patch to it. I will take care of this as soon as possible.