Change the way initialization is handled (a bit). It is now possible to create

a PersistenceContext and give it out to all of your repositories and delay
activating the context until you are ready for everyone to start talking to the
database. In this way you can enforce that no one does any database fooling
around before it's OK.

I also introduced DepotRepository.init() where database fooling around is OK.
The general idea is that you register schema migrations in your constructor and
then if you have data migrations to be done (or initialization that requires
reading things from the database), you do that in init().

src/java/com/samskivert/jdbc/depot/DepotRepository.java

@@ -29,6 +29,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -56,17 +57,10 @@ import com.samskivert.jdbc.depot.expression.ValueExp;
 public abstract class DepotRepository
 {
     /**
-     * Creates a repository with the supplied connection provider and its own private persistence
+     * Creates a repository with the supplied persistence context. Any schema migrations needed by
-     * context.
+     * this repository should be registered in its constructor. A repository should <em>not</em>
-     */
+     * perform any actual database operations in its constructor, only register schema
-    protected DepotRepository (ConnectionProvider conprov)
+     * migrations. Initialization related database operations should be performed in {@link #init}.
-    {
-        _ctx = new PersistenceContext(getClass().getName(), conprov);
-        _ctx.repositoryCreated(this);
-    }
-
-    /**
-     * Creates a repository with the supplied persistence context.
      */
     protected DepotRepository (PersistenceContext context)
     {
@@ -74,6 +68,35 @@ public abstract class DepotRepository
         _ctx.repositoryCreated(this);
     }
 
+    /**
+     * Creates a repository with the supplied connection provider and its own private persistence
+     * context. This should generally not be used for new systems, and is only included to
+     * facilitate the integration of small numbers of Depot-based repositories into systems using
+     * the older samskivert SimpleRepository system.
+     */
+    protected DepotRepository (ConnectionProvider conprov)
+    {
+        _ctx = new PersistenceContext();
+        _ctx.init(getClass().getName(), conprov, null);
+        _ctx.repositoryCreated(this);
+    }
+
+    /**
+     * Provides a place where a repository can perform any initialization that requires database
+     * operations. The default implementation resolves all records managed by this repository so
+     * that their schema migrations are run.
+     */
+    protected void init ()
+        throws DatabaseException
+    {
+        Set<Class<? extends PersistentRecord>> classes =
+            new HashSet<Class<? extends PersistentRecord>>();
+        getManagedRecords(classes);
+        for (Class<? extends PersistentRecord> rclass : classes) {
+            _ctx.getMarshaller(rclass);
+        }
+    }
+
     /**
      * Adds the persistent classes used by this repository to the supplied set.
      */

src/java/com/samskivert/jdbc/depot/PersistenceContext.java

@@ -20,8 +20,10 @@
 
 package com.samskivert.jdbc.depot;
 
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -114,33 +116,6 @@ public class PersistenceContext
         protected abstract boolean testForEviction (Serializable key, T record);
     }
 
-    /**
-     * Creates a persistence context that will use the supplied provider to obtain JDBC
-     * connections.
-     *
-     * @param ident the identifier to provide to the connection provider when requesting a
-     * connection.
-     */
-    public PersistenceContext (String ident, ConnectionProvider conprov)
-    {
-        this(ident, conprov, null);
-    }
-
-    /**
-     * Creates a persistence context that will use the supplied provider to obtain JDBC
-     * connections.
-     *
-     * @param ident the identifier to provide to the connection provider when requesting a
-     * connection.
-     */
-    public PersistenceContext (String ident, ConnectionProvider conprov, CacheAdapter adapter)
-    {
-        _ident = ident;
-        _conprov = conprov;
-        _liaison = LiaisonRegistry.getLiaison(conprov.getURL(ident));
-        _cache = adapter;
-    }
-
     /**
      * Returns the cache adapter used by this context or null if caching is disabled.
      */
@@ -149,6 +124,22 @@ public class PersistenceContext
         return _cache;
     }
 
+    /**
+     * Initializes this context with its connection provider and cache adapter.
+     *
+     * @param ident the identifier to provide to the connection provider when requesting a
+     * connection.
+     * @param conprov provides JDBC {@link Connection} instances.
+     * @param adapter an optional adapter to a cache management system.
+     */
+    public void init (String ident, ConnectionProvider conprov, CacheAdapter adapter)
+    {
+        _ident = ident;
+        _conprov = conprov;
+        _liaison = LiaisonRegistry.getLiaison(conprov.getURL(ident));
+        _cache = adapter;
+    }
+
     /**
      * Shuts this persistence context down, shutting down any caching system in use and shutting
      * down the JDBC connection pool.
@@ -220,6 +211,7 @@ public class PersistenceContext
     public <T extends PersistentRecord> DepotMarshaller<T> getMarshaller (Class<T> type)
         throws DatabaseException
     {
+        checkAreInitialized(); // le check du sanity
         DepotMarshaller<T> marshaller = getRawMarshaller(type);
         try {
             if (!marshaller.isInitialized()) {
@@ -447,22 +439,28 @@ public class PersistenceContext
     }
 
     /**
-     * Iterates over all {@link PersistentRecord} classes managed by this context and initializes
+     * Initializes all repositories that have been created and registered with this persistence
-     * their {@link DepotMarshaller}. This forces migrations to run and the database schema to be
+     * context. Any repositories that are constructed after this call will be immediately
-     * created.
+     * initialized at the time they are constructed (which is probably undesirable). When a
+     * repository is initialized, schema migrations for all of its managed persistent records are
+     * run. It is best to do all schema migrations when the system initializes which is why lazy
+     * initialization of repositories is undesirable.
      *
-     * @param warnOnLazyInit if true, any persistent records that are resolved after this method is
+     * @param warnOnLazyInit if true, any repositories are constructed after this method is called
-     * called will result in a warning so that the application developer can restructure their code
+     * will result in a warning so that the application developer can restructure their code to
-     * to ensure that those records are properly registered prior to this call.
+     * ensure that those repositories are properly created prior to this call.
      */
-    public void initializeManagedRecords (boolean warnOnLazyInit)
+    public void initializeRepositories (boolean warnOnLazyInit)
         throws DatabaseException
     {
-        for (Class<? extends PersistentRecord> rclass : _managedRecords) {
+        // initialize our repositories
-            getMarshaller(rclass);
+        for (DepotRepository repo : _repositories) {
+            repo.init();
         }
+        // note that we've now been initialized
+        _repositories = null;
         // now issue a warning if we lazily initialize any other persistent record
-        _warnOnLazyInit = true;
+        _warnOnLazyInit = warnOnLazyInit;
     }
 
     /**
@@ -472,12 +470,18 @@ public class PersistenceContext
      */
     protected void repositoryCreated (DepotRepository repo)
     {
-        repo.getManagedRecords(_managedRecords);
+        if (_repositories == null) {
+            if (_warnOnLazyInit) {
+                log.warning("Repository created lazily: " + repo.getClass().getName());
+            }
+            repo.init();
+        } else {
+            _repositories.add(repo);
+        }
     }
 
     /**
-     * Looks up and creates, but does not initialize, the marshaller for the specified Entity
+     * Looks up and creates, but does not initialize, the marshaller for the specified Entity type.
-     * type.
      */
     protected <T extends PersistentRecord> DepotMarshaller<T> getRawMarshaller (Class<T> type)
     {
@@ -490,13 +494,13 @@ public class PersistenceContext
     }
 
     /**
-     * Internal invoke method that takes care of transient retries
+     * Internal invoke method that takes care of transient retries for both queries and modifiers.
-     * for both queries and modifiers.
      */
-    protected <T> Object invoke (
+    protected <T> Object invoke (Query<T> query, Modifier modifier, boolean retryOnTransientFailure)
-        Query<T> query, Modifier modifier, boolean retryOnTransientFailure)
         throws DatabaseException
     {
+        checkAreInitialized(); // le check du sanity
+
         boolean isReadOnlyQuery = (query != null);
         Connection conn;
         try {
@@ -556,6 +560,15 @@ public class PersistenceContext
         return invoke(query, modifier, false);
     }
 
+    protected void checkAreInitialized ()
+    {
+        if (_conprov == null) {
+            throw new IllegalStateException(
+                "This persistence context has not yet been initialized. You are probably " +
+                "doing something too early, like in a repository's constructor. Don't do that.");
+        }
+    }
+
     protected String _ident;
     protected ConnectionProvider _conprov;
     protected DatabaseLiaison _liaison;
@@ -564,16 +577,14 @@ public class PersistenceContext
     /** The object through which all our caching is relayed, or null, for no caching. */
     protected CacheAdapter _cache;
 
-    protected Map<String, Set<CacheListener<?>>> _listenerSets =
+    /** Tracks repositories during the pre-initialization phase. */
-        new HashMap<String, Set<CacheListener<?>>>();
+    protected List<DepotRepository> _repositories = new ArrayList<DepotRepository>();
 
+    /** A mapping from persistent record class to resolved marshaller. */
     protected Map<Class<?>, DepotMarshaller<?>> _marshallers =
         new HashMap<Class<?>, DepotMarshaller<?>>();
 
-    /**
+    /** A mapping of cache listeners by cache id. */
-     * The set of persistent records for which this context is responsible. This data is used by
+    protected Map<String, Set<CacheListener<?>>> _listenerSets =
-     * {@link #initializeManagedRecords} to force migration/schema initialization.
+        new HashMap<String, Set<CacheListener<?>>>();
-     */
-    protected Set<Class<? extends PersistentRecord>> _managedRecords =
-        new HashSet<Class<? extends PersistentRecord>>();
 }