Andrea Aime wrote: > Justin Deoliveira ha scritto: >> Looked a bit into this as well, and I agree it is tricky. One thing i am >> not crazy about with the proposed solution is putting the logic directly >> into the ComponentInfo (PageInfo) class. This class is designed to be >> relatively dumb, just metadata about the page. > > I have no troubles with moving behavior outside the class. > Yet, having all these pure data holders around makes me wonder, > the issue of pure data holders is usually described as > "anemic domain model": > http://en.wikipedia.org/wiki/Anemic_Domain_Model > http://martinfowler.com/bliki/AnemicDomainModel.html
Hmmm... guess I am a bad programmer :) The reason I try to stick to this separation can be summed up by one class: org.vfny.geoserver.config.FeatureTypeInfo. Which was a mix of data/java bean and logic. Again, maybe i don't know what I am doing so take this for what its worth, but I felt that this class was a nightmare to maintain. And in porting to the new configuration and catalog, believe me when I say that ensuring that all the logic and assumptions that this class encapsulated was no simple task. That said, I know I push this anti-pattern quite extensively in my designs and implementation, so if others feel that this does more bad than good I will gladly change my practices. > >> As a middle ground i would say perhaps keep mountPath on PageInfo, but >> move all the logic for figuring out what a sensible default is into >> GeoServerApplication. > > Works for me > >> In terms of url mappings what about the slightly less verbose: >> >> LayerListPage -> .../layer/list >> LayerNewPage -> .../layer/new >> LayerEditPage -> .../layer/edit > > I guess we can, how would the general name transformation rule > look like? Would we have to check if the page name starts with the > same name as the containing package? Well... here is what i see. Let's call the "prefix" in this case "layer". And the "operation" one of (list,new,edit) To get the prefix: 1. Strip off org.geoserver.web from the package 2. Take the last component of the package remaining (ie, "data.layer" -> "layer") And to get the operation check for the classname ending in one of ListPage,EditPage,NewPage. > >> Also, for the edit case, don't we have to encode some state in the url? >> The id of the object being edited? > > Yes, we would, thought I did not check how the resulting URL would look > like. > >> I took the opportunity to play around expanding on the patch and here is >> what I came up with: >> >> 1) Add PageInfo same as the patch >> >> 2) Add PageInfo instances to the app context for all pages, not just >> ones linked from the main menu > > Yep. At least for all pages that we want to make bookmarkable > >> 3) In GeoServerApplication.init() do the dance of stripping off the >> package name, and inspect the class name to figure if it is a "list", >> "edit" , or "new" case. And mount the page class appropriately. > > Roger > >> The "edit" case is a bit tricky because it has state which needs to be >> represented in the url. Something like: >> >> http://.../layer/edit?id=<layerid> or: >> http://.../layer/edit/<layerid> >> >> Now at the end of the day all the edit pages basically hold onto the id >> of the object they are editing (I think some hold onto the actual object >> but I think there is a jira open to fix this, and make the behavior the >> same). Those pages that hold onto the id usually do so with a >> LoadableDetachable model. There is WorkspaceDetachableModel, >> LayerDetachableModel, etc... >> >> All those loadable detachable models could extend a single base class, >> something like "CatalogDetachableModel", which would look something like: >> >> class CatalogDetachableModel implements LoadableDetachableModel { >> >> String id; >> >> protected CatalogDetachableModel(String id) { >> this.id = id; >> } >> >> String getID() { >> return id; >> } >> >> final Object load() { >> return loadFromId(id); >> } >> >> protected abstract Object loadFromId(String id); >> >> } >> >> Now, in step 3 above, when GeoServerApplication is mounting all the >> classes from PageInfo, and it detects the edit case it would mount a >> specific implementation of IRequestTargetUrlCodingStrategy which in its >> encode() method calls page.getModel(), checks it for being an instanceof >> CatalogDetachableModel, and gets the id from it. > > At that point the page instances are still not created, and the model > not set. How would this work? Well, generally in the edit page class our application creates the page directly in order to set the state, rather than just supply the class of page and letting wicket create it. Correct? When this happens the IRequestTragetUrlCodingStrategy will be asked to encode an instnace of IPageRequestTarget, which has the method getPage(). So to pull this off we would need to: 1) ensure we stick to instantiating the page before hand, although the coding strategy could pretty easily handle the case of a page class with some page parameters. 2) Ensure that in the constructor of an edit page that the model is set properly. Just an idea... perhaps not a good one. Open to alternatives here. > > Cheers > Andrea > -- Justin Deoliveira OpenGeo - http://opengeo.org Enterprise support for open source geospatial. ------------------------------------------------------------------------------ Enter the BlackBerry Developer Challenge This is your chance to win up to $100,000 in prizes! For a limited time, vendors submitting new applications to BlackBerry App World(TM) will have the opportunity to enter the BlackBerry Developer Challenge. See full prize details at: http://p.sf.net/sfu/Challenge _______________________________________________ Geoserver-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geoserver-devel
