Refactor ClassInfo using Natures

Latest cumulative patch including Natures. Please revert your project before applying this patch!

The last comment concerns the patch "patch.c2380.20080526.txt" (thought this would be added to the "Description" field...)

Some more comments:
This patch adds support for Natures, specifically:

• ClassInfo now implements NatureExtendable.
• A sample JDONature has been added.

Following points should be discussed:

• An inner class was added to JDONature to represent a column. Since; this a first shot at the JDONature and no discussion was held on whether we should drop FieldInfo classes all together (and simply add everything into the _natureProperties field of the ClassInfo class) or not, this seemed like a viable solution to start with.

• We have an ongoing debate on whether we should give external classes access to the ClassInfo's _natureProperties via a getNatureProperties() method or not. This would essentially eliminate the need of a _classInfo field within the JDONature; for, every JDONature-method which sets a property in the ClassInfo could be passed a ClassInfo parameter. However, the implementation of the _natureProperties' type should be hidden if we want to have the flexibility to exchange the current HashMap for some other type in the future. Further, we discussed whether it is a good idea to expose the _natureProperties to the "outside" world or not...

Cleaned up. Patch contains nature specific stuff only.

A few updates:
As discussed, we will NOT drop FieldInfos and I will try to come up with some implementation proposal by the end of this week.
Further we decided to add an interface PropertyHolder to adds getNatureProperty() and setNatureProperty(..) to the ClassInfo class (see my last comment from 27 May).
Next, we refactored the ClassInfo class to an interface which extends the PropertyHolder and NatureExtendable interfaces. The newly added BaseClassInfo realizes the ClassInfo interface.
The same could be done with the FieldInfo class with the only difference that the NatureExtendable interface will propably not be added; since, a FieldInfo won't have different Natures assigned to it than its ClassInfo container.

Another question which arose, was if it is necessary to refactor the JDONature class to JDOClassNature and JDOFieldNature to handle ClassInfos and FieldInfos, respectively, or if JDONature should handle both.

More Comments on the patch:
Right now I'm working on the implementation of a JDONature which would handle both ClassInfo and FieldInfo nature properties and JDOClassNature and JDOFieldNature which will handle them respectively. The refactoring as announced in my last comment does break a lot of existing code, most notably in castor.builder package which - on the other hand - isn't too much of a surprise
Added lots of documentation, so I won't repeat myself here. Feel free to post your suggestions/criticism/questions!

Please note that patch.c2380.20080602.txt patch CANNOT be applied correctly when using Eclipse! You must use another tool like TortoiseMerge (which ships with TortoiseSVN) in order to apply this patch correctly.

Notes on the contents of the patch:

• Refactoring done correctly this time
• Added various Nature-specific Exceptions.
• Changed interface hierarchy of ClassInfo which extends NatureExtendable which in turn extends PropertyHolder.
• Reworked and added JDOClassNatureTest and JDOFieldInfoTest (not fully implemented yet).
• Added code to JDOFieldNature (not fully implemented yet).

As to answer the question 'why their exist build errors' in advance, you may note that the ClassInfo and FieldInfo interfaces do not implement any of the XMLInfo methods which will be implemented using XML*Nature classes (not implemented at all... yet).

Following my last comment on the developement of a single JDONature class which contains both methods to handle ClassInfo and FieldInfo Nature properties I will first finish work on the JDOClassNature and JDOFieldNature classes and eventually merge them to a JDONature class, if feasible.

Following my last comment on the developement of a single JDONature class which contains both methods to handle ClassInfo and FieldInfo Nature properties I will first finish work on the JDOClassNature and JDOFieldNature classes and eventually merge them to a JDONature class, if feasible.

Which I think is the best of all approaches. Get it working with separated nature implementation(s) first, and then - after re-assessing your assumptions - refactor as needed.

This is the first implementation of the new architectural design including JDOClassNature and JDOFieldNature. ClassInfo and FieldInfo are both implementing NatureExtendable and PropertyHolder. The PropertyHolder and NatureExtendable interfaces hide implementation details. The Natures contain basic JDO attributes like table and column name and a few more. XMLNature will be our next task to be done. Further more we will enhance existing Natures as time goes by. As soon as we've implemented the XMLNature, we're going to introduce it to the other contributers.

XMLNature implementation finished. Patch also includes JDO Natures and TestCases. This patch does not affect Castor legacy code. ClassInfo and FieldInfo can be proceeded as usual, but internally use instances of XMLNature to access XML specific properties.

Hello,

I'm very interested in this patch... but I'm not able to apply that patch and I don't have any of those tools available on my platform...
could you please also attach the new ClassInfo and FieldInfo sources - those are the only two which aren't useable after applying the patch.

Thx

Joachim

We had various problems applying patches provoked by the * $Id: ... $ tags in header comments due to handling differences of various svn software products which I will discuss with Werner today. I think this is clear, but: you must also revert these files before applying the patch and update your working copy to the latest revision. If you got these problems concerning the $id thing you can also do a short workaround and modify the patch changing the $id line to what it says in your local file. Regards, Lukas

This patch contains the ongoing work on XML and JDO Natures. It also includes a bugfix concerning incorrect behaviour of properties not set before, as you will find it in the former XMLInfo's boolean properties. In case you are not able to apply this patch to your working directory, I will provide ClassInfo, XMLInfo and FieldInfo as a zipped file in the next attachment.

ClassInfo.java, FieldInfo.java, XMLInfo.java - as promised as a zipfile containing relative paths.

Lukas, can I safely assume that the CTF finished without any problems ?

Yesterday, Tobias and I wrote an implementation of the newly proposed Nature architecture, which is similar to the latest version, but containing a few improvements as discussed. We faced various details being not as slightly, as we thought. First, we needed to have PropertyHolder extending NatureExtendable to do the nature health check in the constructor of the BaseNature (BaseNature(holder : PropertyHolder)) and to get/set properties. Then we figured out, that by again creating a Nature interface consisting of one single getID() : String method, which is not accessible unless you have an instance of a - let's say - JDOClassNature, there is no way to do:
<code>
ClassInfo ci = ...
if (!ci.hasNature(JDOClassNature.getId())) {
ci.addNature(JDOClassNature.getId());
JDOClassNature jdo = ...
}
//do some JDOClassNature specific stuff
</code>
Due to the health check in the constructor JDOClassNature(holder : PropertyHolder) it is not possible to instantiate a Nature unless a NatureExtendable "has" this Nature. We see no way to enforce a static method/public constant or something like that. Any ideas?

I remember having the same problem with the first implementation of Natures. After playing around a bit I found two possible solutions:

1. We introduce an empty Nature constructor, e.g.

   ClassInfo cInfo = ...;
   JDOClassNature cNature = new JDOClassNature();
   cInfo.addNature(cNature.getId());
   

   and add a setInfo(PropertyHolder ph) to the Natures and do the health check therein.

2. Or we somehow use another class to keep track of the available Nature IDs which should be declared as private static final String ID = "..."; within the respective nature class. The class that would be predestined to handle this functionality (i.e. providing the Nature IDs) - imho - is a NatureRegistry, similar to the class we discussed in our initial architectural design to allow for dependency injection of possible natures.

However, this still doesn't solve the fact that we are not (fully) able to enforce our intentions of the usage of these nature classes. In case of my second proposal from above (introducing a NatureRegistry) we could still define the getId() method in the Nature Interface and let the NatureRegistry handle the instantiation of the Nature upon adding it to the registry, basically reading out its ID and dropping it thereafter.

Concerning the change of the PropertyHolder interface now extending the NatureExtendable interface I was thinking about doing this the other way around, so NatureExtendable extends the PropertyHolder interface which is - imho - the more general of the two interfaces. However, this is more of a semantic issue...

Any ideas?

Yes, as already discussed on Friday, in addition to the Nature interface and its getId:String method, we said we gonna have a public static constant ini each nature named ID.

In addition- as suggested by Steven on Friday - we could move towards using fully qualified class names rather than IDs. This would reduce complexity altogether.

Implementation of the revised Nature architecture. For compatibility reason XML/Class/FieldInfo.java will be attachted as a zip file in the next attachment.

Archive contains XMLInfo.java ClassInfo.java FieldInfo.java including relative paths for those who can't patch these files.

The current state does not cover the getFields() method on the JDOClassNature returning a List of JDOFieldNatures. Will be added soon.

Filip, I did not add the hasPrimaryKey(..) method in JDOClassNature as suggested, because I wanted to keep the Natures strictly delegating. Though it is boilerplate code you have to write, it is neither jdo nor Nature specific I think.

Added a getPropertyAsList method and corresponding testcases.