Issue Details (XML | Word | Printable)

Key: GRAILS-814
Type: Task Task
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Marc Palmer
Reporter: Marc Palmer
Votes: 0
Watchers: 0
Operations

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

Refactor GrailsApplication and DefaultGrailsApplication to abstract artefact management

Created: 22/Feb/07 03:38 AM   Updated: 25/Nov/08 07:04 AM   Resolved: 25/Nov/08 07:04 AM
Return to search
Component/s: Commons
Affects Version/s: 0.4.1
Fix Version/s: 1.1-beta1

Time Tracking:
Issue & Sub-Tasks
Issue Only
Not Specified

  • Sub-Tasks:
  • All
  • Open

 Description  « Hide

I notice something that feels bad in GrailsApplication - basically all the methods relating to artefacts that are in many ways duplicated across artefact types and make it difficult/impossible for plugins to add "true" artefacts:

public GrailsControllerClass[] getControllers();
public GrailsControllerClass getController(String fullname);
public GrailsControllerClass getControllerByURI(String uri);
public GrailsTaskClass[] getGrailsTasksClasses();
public GrailsTaskClass getGrailsTaskClass( String fullname );
public GrailsDomainClass[] getGrailsDomainClasses();
public GrailsDomainClass[] getDomainClasses();
public GrailsCodecClass[] getCodecClasses();
public boolean isGrailsDomainClass(Class domainClass);
public GrailsDomainClass getGrailsDomainClass(String name);
public GrailsDomainClass getDomainClass( String name );
public GrailsDataSource getGrailsDataSource();
public GroovyClassLoader getClassLoader();
public GrailsServiceClass[] getGrailsServiceClasses();
public GrailsServiceClass[] getServices();
public GrailsServiceClass getGrailsServiceClass(String name);
public GrailsServiceClass getService(String name);
public GrailsBootstrapClass[] getGrailsBootstrapClasses();
public GrailsBootstrapClass[] getBootstraps();
public GrailsTagLibClass[] getGrailsTabLibClasses();
public GrailsTagLibClass[] getTagLibs();
public GrailsTagLibClass getGrailsTagLibClass(String tagLibName);
public GrailsCodecClass getGrailsCodecClass(String codecName);
public GrailsCodecClass addCodecClass(Class codecClass);
public GrailsTagLibClass getTagLib(String name);
public GrailsTagLibClass getTagLibClassForTag(String tagName);
GrailsControllerClass addControllerClass(Class controllerClass);

...it goes on. This smells bad to me!

Imagine for example we decide the "log" property should only be added to the metaclass of all artifacts, not all classes. How to implement this currently?

I propose we refactor GrailsApplication completely for 0.5 to instead use this standard set of methods:

public GrailsArtefactClass getArtefact(String artefactType, String name);
public GrailsArtefactClass getArtefactClass(String artefactType, String name);
public GrailsArtefactClass[] getArtefacts(String artefactType);
public GrailsArtefactClass[] getArtefactClasses(String artefactType);
// This is next call is equiv to getControllerByURI
public GrailsArtefactClass getArtefactForFeature(String artefactType, String tagName);
// This is next call is equiv to getTagLibClassForTag
public GrailsArtefactClass getArtefactClassForFeature(String artefactType, String tagName);
public GrailsArtefactClass addArtefactClass(String artefactType, Class artefactClass);
public boolean isArtefactClass(String artefactType, Class artefactClass);

That concise API would translate into calls like this to effect the existing methods we have:

// public GrailsControllerClass[] getControllers();
getArtefacts("Controller")

// public GrailsControllerClass getController(String fullname);
getArtefact("Controller", fullname)

// public GrailsControllerClass getControllerByURI(String uri);
getArtefactForFeature("Controller", uri)

// public GrailsTaskClass[] getGrailsTasksClasses();
getArtefactClasses("Task")

// public GrailsTaskClass getGrailsTaskClass( String fullname );
getArtefactClass("Task", fullName)

// public GrailsDomainClass[] getGrailsDomainClasses();
getArtefactClasses("Domain")

// public GrailsCodecClass[] getCodecClasses();
getArtefactClasses("Codec")

// public boolean isGrailsDomainClass(Class domainClass);
isArtefactClass("Domain", domainClass)

...and so on.

I know this would be a breaking API change. Is this a big problem? We could deprecate all the existing methods, add the new ones, make the existing ones call the new ones as above, and in 0.6 pull them out.



Marc Palmer made changes - 22/Feb/07 03:39 AM
Field Original Value New Value
Status Open [ 1 ] In Progress [ 3 ]
Marc Palmer made changes - 27/Feb/07 09:50 AM
Status In Progress [ 3 ] Open [ 1 ]
Graeme Rocher made changes - 20/Mar/07 12:14 PM
Fix Version/s 0.6 [ 12699 ]
Fix Version/s 0.5 [ 12698 ]
Graeme Rocher made changes - 10/Jul/07 05:20 AM
Fix Version/s 0.6 [ 12699 ]
Fix Version/s 1.0-RC1 [ 13341 ]
Graeme Rocher made changes - 05/Sep/07 04:57 PM
Fix Version/s 1.1 [ 13674 ]
Fix Version/s 1.0-RC1 [ 13341 ]
Graeme Rocher added a comment - 25/Nov/08 07:04 AM

This is done now


Graeme Rocher made changes - 25/Nov/08 07:04 AM
Resolution Fixed [ 1 ]
Status Open [ 1 ] Closed [ 6 ]