jira.codehaus.org

  • Log In Access more options
    • Online Help
    • Keyboard Shortcuts
    • About JIRA
    • JIRA Credits
    • What?s New
  • Dashboards Access more options (Alt+d)
  • Projects Access more options (Alt+p)
  • Issues Access more options (Alt+i)
Signup
GeoServer
  • GeoServer
  • GEOS-3806

Catalog+DAO split

  • Log In
  • Views
    • XML
    • Word
    • Printable

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: None
  • Fix Version/s: 2.1.x
  • Component/s: Global, Hibernate catalog
  • Labels:
    None
  • Patch Submitted:
    Yes
  • Number of attachments :
    6

Description

Here's a patch against the current trunk (14002) for the Catalog+DAO splitting (refer to to R'n'D page http://geoserver.org/pages/viewpage.action?pageId=22708796).

This is a step-zero refactoring, aimed at a first split from the Catalog logic and the real data handling.
Most of the code is still bound to the InMemoryCatalogDAO requirements.
The hibernate module is not aligned with these changes yet, so it won't work
until refactored itself. I think some other minor changes will be needed to the
CatalogImpl class in order to make also the hib module work.

The main refactoring has been made on CatalogImpl: it has been split in 2 classes + 1 interface:

  • main/src/main/java/org/geoserver/catalog/impl/CatalogImpl.java
  • main/src/main/java/org/geoserver/catalog/CatalogDAO.java
  • main/src/main/java/org/geoserver/catalog/impl/InMemoryCatalogDAO.java

Some other classes have been involved in the refactoring:

  • main/src/main/java/org/geoserver/config/util/XStreamPersister.java
    Handled some data that are no longer in CatalogImpl, but in InMemoryCatalogDAO.
  • main/src/main/java/org/geoserver/data/CatalogWriter.java
  • main/src/main/java/org/geoserver/catalog/util/LegacyCatalogReader.java
    Reads and writes data file also handled by XStreamPersister.
    While CatalogWriter writes data in the "new" format, LegacyCatalogReader may read both legacy and new format.
  • main/src/main/java/org/geoserver/catalog/impl/LayerGroupInfoImpl.java
    Added a workaround to be discussed about the resolve() call.
  • The current DAO exposes methods for every lookup/search loop in the original CatalogImpl. I
    think this is the least DAO interface we need for this step zero refactoring.
  • All of the listener logic has been left in the CatalogImpl, and the ModProxy stuff as
    well. The DAO's update() methods are aware about the proxying because the
    InMemoryCatalogDAO needs it to commit() the changes. This kind of coupling
    does not worry me, because having old and new values may be useful also for
    other checks at DAO level.
  • I split the saved() call in the 2-phase preSave() and postSave() to have a
    cleaner flow.
  • The default(Name|Work)space logic was a bit too tied with the way it was
    handled (null key into a Map). Now data and logic is more explicit in their usage.
  • resolve() has been moved into the MemoryDAO. Maybe it should not be its
    concern, but rather of the object that is creating the resource (the
    XStreamPersister?). This has to be discussed and cleaned up.

This patch compiles and passes all the tests with the web+restconfig profiles
activated.

Waiting for a review.

  • Options
    • Sort By Name
    • Sort By Date
    • Ascending
    • Descending
    • Download All

Attachments

  1. File
    20100219_catalog_dao_refactor.diff
    19/Feb/10 7:23 AM
    106 kB
    Emanuele Tajariol
  2. File
    20100225_catalog_dao_refactor.diff
    25/Feb/10 6:00 AM
    126 kB
    Emanuele Tajariol
  3. Text File
    GEOS-3806.patch
    22/Oct/10 2:27 PM
    230 kB
    Justin Deoliveira
  4. Text File
    GEOS-3806.patch
    22/Oct/10 10:55 AM
    229 kB
    Justin Deoliveira
  5. Text File
    GEOS-3806.patch
    01/Oct/10 5:20 PM
    183 kB
    Justin Deoliveira
  6. File
    gs_trunk_src.diff
    11/Feb/10 10:37 AM
    92 kB
    Emanuele Tajariol

Activity

Ascending order - Click to sort in descending order
  • All
  • Comments
  • Work Log
  • History
  • Activity
Hide
Permalink
Emanuele Tajariol added a comment - 19/Feb/10 7:23 AM

Attaching new patch, including modifications suggested in Justin review and some other considerations in IRC.

  • classes and methods renaming, to respect existing naming conventions.
  • Resolving instances: Added CatalogDAO.preAdd(T) for every existing add(T). This method will prepare the data for addition into the Catalog. The MemoryCatalogDAO will forward the call to resolve(T).
  • XStreamPersister is only used for loading data, and not the whole catalog. Changes have been committed so that Catalog cannot be (de)persisted any longer. Test case has been commented out.
  • Catalog.resolve() has been commented out as well, since it was only invoked from within XStreamPersister.
  • LayerGroupInfoImpl changes have been reverted, since they were related to an XStreamPersister issue.
  • GeoServerLoader has been modified in order to deal with data loading in generic case.
  • CatalogSync (a class to copy catalog contents) has been sketched; it needs a proper implementation.

This patch completely replaces previous one.

Show
Emanuele Tajariol added a comment - 19/Feb/10 7:23 AM Attaching new patch, including modifications suggested in Justin review and some other considerations in IRC. classes and methods renaming, to respect existing naming conventions. Resolving instances: Added CatalogDAO.preAdd(T) for every existing add(T). This method will prepare the data for addition into the Catalog. The MemoryCatalogDAO will forward the call to resolve(T). XStreamPersister is only used for loading data, and not the whole catalog. Changes have been committed so that Catalog cannot be (de)persisted any longer. Test case has been commented out. Catalog.resolve() has been commented out as well, since it was only invoked from within XStreamPersister. LayerGroupInfoImpl changes have been reverted, since they were related to an XStreamPersister issue. GeoServerLoader has been modified in order to deal with data loading in generic case. CatalogSync (a class to copy catalog contents) has been sketched; it needs a proper implementation. This patch completely replaces previous one.
Hide
Permalink
Emanuele Tajariol added a comment - 25/Feb/10 6:00 AM

This new patch fixes some misbehaviours and makes the (WIP) hibernate community module work.

  • Simplified some method names in CatalogDAO;
  • Added a CatalogFactoryFactory in order to have specific catalog beans when needed (there are two of them now - LayerInfoImplHb and LayerGroupInfoImplHb - which should be harmonized with the "standard" ones, but this factory will ease fixes for any other glitch);
  • ModificationProxy won't cope well with Hibernate collections, so a sort-of workaround was needed. To be discussed.

This patch replaces the previous ones.

Show
Emanuele Tajariol added a comment - 25/Feb/10 6:00 AM This new patch fixes some misbehaviours and makes the (WIP) hibernate community module work. Simplified some method names in CatalogDAO; Added a CatalogFactoryFactory in order to have specific catalog beans when needed (there are two of them now - LayerInfoImplHb and LayerGroupInfoImplHb - which should be harmonized with the "standard" ones, but this factory will ease fixes for any other glitch); ModificationProxy won't cope well with Hibernate collections, so a sort-of workaround was needed. To be discussed. This patch replaces the previous ones.
Hide
Permalink
Justin Deoliveira added a comment - 01/Oct/10 5:20 PM

Here is an updated patch for the dao split.

Show
Justin Deoliveira added a comment - 01/Oct/10 5:20 PM Here is an updated patch for the dao split.
Hide
Permalink
Justin Deoliveira added a comment - 22/Oct/10 10:55 AM

An updated patch with some javadocs for the CatalogDAO interface

Show
Justin Deoliveira added a comment - 22/Oct/10 10:55 AM An updated patch with some javadocs for the CatalogDAO interface
Hide
Permalink
Justin Deoliveira added a comment - 22/Oct/10 2:27 PM

Updated back with Rename of DAO to Facade

Show
Justin Deoliveira added a comment - 22/Oct/10 2:27 PM Updated back with Rename of DAO to Facade
Hide
Permalink
Andrea Aime added a comment - 06/Mar/11 11:52 AM

Mass closing all issues that have been in the resolved state for the last month without anyone commenting or reopening them

Show
Andrea Aime added a comment - 06/Mar/11 11:52 AM Mass closing all issues that have been in the resolved state for the last month without anyone commenting or reopening them

People

  • Assignee:
    Justin Deoliveira
    Reporter:
    Emanuele Tajariol
Vote (0)
Watch (2)

Dates

  • Created:
    11/Feb/10 10:37 AM
    Updated:
    06/Mar/11 11:52 AM
    Resolved:
    25/Oct/10 2:50 PM
  • Atlassian JIRA (v5.2.7#850-sha1:b2af0c8)
  • Report a problem
  • Powered by a free Atlassian JIRA open source license for Codehaus. Try JIRA - bug tracking software for your team.