From 0e68c8a5995fc2b17edf255fb41b6cdc6f561022 Mon Sep 17 00:00:00 2001 From: Michael Bayne Date: Wed, 24 Sep 2008 18:33:25 +0000 Subject: [PATCH] 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(). --- .../jdbc/depot/DepotRepository.java | 45 +++++-- .../jdbc/depot/PersistenceContext.java | 115 ++++++++++-------- 2 files changed, 97 insertions(+), 63 deletions(-) diff --git a/src/java/com/samskivert/jdbc/depot/DepotRepository.java b/src/java/com/samskivert/jdbc/depot/DepotRepository.java index 45b73b1..7b9dad4 100644 --- a/src/java/com/samskivert/jdbc/depot/DepotRepository.java +++ b/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 - * context. - */ - protected DepotRepository (ConnectionProvider conprov) - { - _ctx = new PersistenceContext(getClass().getName(), conprov); - _ctx.repositoryCreated(this); - } - - /** - * Creates a repository with the supplied persistence context. + * Creates a repository with the supplied persistence context. Any schema migrations needed by + * this repository should be registered in its constructor. A repository should not + * perform any actual database operations in its constructor, only register schema + * migrations. Initialization related database operations should be performed in {@link #init}. */ 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> classes = + new HashSet>(); + getManagedRecords(classes); + for (Class rclass : classes) { + _ctx.getMarshaller(rclass); + } + } + /** * Adds the persistent classes used by this repository to the supplied set. */ diff --git a/src/java/com/samskivert/jdbc/depot/PersistenceContext.java b/src/java/com/samskivert/jdbc/depot/PersistenceContext.java index 9e77021..c74b81d 100644 --- a/src/java/com/samskivert/jdbc/depot/PersistenceContext.java +++ b/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 DepotMarshaller getMarshaller (Class type) throws DatabaseException { + checkAreInitialized(); // le check du sanity DepotMarshaller 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 - * their {@link DepotMarshaller}. This forces migrations to run and the database schema to be - * created. + * Initializes all repositories that have been created and registered with this persistence + * context. Any repositories that are constructed after this call will be immediately + * 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 - * called will result in a warning so that the application developer can restructure their code - * to ensure that those records are properly registered prior to this call. + * @param warnOnLazyInit if true, any repositories are constructed after this method is called + * will result in a warning so that the application developer can restructure their code to + * 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 rclass : _managedRecords) { - getMarshaller(rclass); + // initialize our repositories + 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 - * type. + * Looks up and creates, but does not initialize, the marshaller for the specified Entity type. */ protected DepotMarshaller getRawMarshaller (Class type) { @@ -490,13 +494,13 @@ public class PersistenceContext } /** - * Internal invoke method that takes care of transient retries - * for both queries and modifiers. + * Internal invoke method that takes care of transient retries for both queries and modifiers. */ - protected Object invoke ( - Query query, Modifier modifier, boolean retryOnTransientFailure) + protected Object invoke (Query 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>> _listenerSets = - new HashMap>>(); + /** Tracks repositories during the pre-initialization phase. */ + protected List _repositories = new ArrayList(); + /** A mapping from persistent record class to resolved marshaller. */ protected Map, DepotMarshaller> _marshallers = new HashMap, DepotMarshaller>(); - /** - * The set of persistent records for which this context is responsible. This data is used by - * {@link #initializeManagedRecords} to force migration/schema initialization. - */ - protected Set> _managedRecords = - new HashSet>(); + /** A mapping of cache listeners by cache id. */ + protected Map>> _listenerSets = + new HashMap>>(); }