ReflectionObjectFactory is too magical, and it's causing unit tests to
fail.

Specifically, I ran across this "joy" recently, and took me until now to
diagnose: The DbMetal_test.dll tests run without error if you load only
DbMetal_test.dll into NUnit.

If, however, you instead use the DbLinq.nunit project file (which lists
all NUnit test assemblies), then the GetWordsTest_MyTableName,
GetWordsTest_MyTableName2, and InvalidCharctersLanguage2Test tests
within NameFormatterTest (in DbMetal_test.dll) all fail with a
NullReferenceException.  Oops.

Why?  Because within DbLinq.nunit the DbMetal_test.dll assembly is
listed last.  Consequently, it (apparently) isn't loaded within the
AppDomain when ReflectionObjectFactory.ParseAppDomain() is executed, and
thus important types located within DbMetal.exe (such as
DbMetal.Language.EnglishWords) aren't found.

Furthermore, it is an ordering issue -- if I alter DbLinq.nunit so that
DbMetal_test.dll is loaded first, then all the NameFormatterTest tests
pass.

So, there's an obvious (in hindsight) ordering dependency here, which
raises the question: what should be done about it?

The simple solution is to say this is By Design, and change DbLinq.nunit
so that DbMetal_test.dll is done first.

I don't like this idea as it's borderline evil.  (It also means assembly
names can't be sorted, making it harder for me to make sure that all
test assemblies are actually listed within DbLinq.nunit.)

What I don't know about is a "proper" solution.  Previously I've
mentioned using assembly-level attributes (as they're faster), but that
won't work here as the assembly needs to be loaded in order for
attributes to be a viable solution, and in this case the assembly isn't
loaded.

My current idea, which I'd appreciate feedback on, is to modify the
DbLinq.Factory.IObjectFactory interface to add Register() and
Unregister() methods [0]:

        interface IObjectFactory {
            void Register(Type implementationType);
            void Unregister(Type implementationType);
            // ...
        }

These methods would allow NameFormatterTest to ensure that the
appropriate types could be found, e.g. within GetWordsTest_MyTableName:

        try {
            ObjectFactory.Current.Register(typeof(EnglishWords));
            // ...
        }
        finally {
            ObjectFactory.Current.Unregister(typeof(EnglishWords));
        }

Attached is a patch which implements this idea, and allows the
DbMetal_test.dll unit tests to work without ordering dependencies.  (The
patch also implements the ideas in [0].)

Permission to commit?

Thanks,

 - Jon

[0] I'm not entirely fond of the IObjectFactory interface, because it
has duplication, e.g. Get<T>() and GetInstance() do the same thing, and
Create<T>() and GetInstance() also do the same thing.  Furthermore,
using a bool to choose between Get and Create is counter to FxDG, as it
makes the callsite unobvious (does true mean get or create semantics?).

Consequently, in the attached patch I altered IObjectFactory to be:

        interface IObjectFactory {
            void Register(Type implementationType);
            void Unregister(Type implementationType);
            object Create(Type interfaceType);
            object Get(Type interfaceType);
            IEnumerable<Type> GetImplementations(Type interfaceType);
        }

To preserve the generic niceties of the original Get<T>() and related
methods, I've added an ObjectFactoryExtensions class which provides
extension methods to implement Create<T>() and Get<T>() in terms of
IObjectFactory.

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"DbLinq" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to 
[email protected]
For more options, visit this group at 
http://groups.google.com/group/dblinq?hl=en
-~----------~----~----~----~------~----~------~--~---

Index: src/DbMetal/Test/NameFormatterTest.cs
===================================================================
--- src/DbMetal/Test/NameFormatterTest.cs	(revision 1028)
+++ src/DbMetal/Test/NameFormatterTest.cs	(working copy)
@@ -26,10 +26,12 @@
 
 using System.Globalization;
 using DbLinq;
+using DbLinq.Factory;
 using DbLinq.Schema;
 using DbLinq.Schema.Implementation;
 using DbLinq.Util;
 using DbMetal;
+using DbMetal.Language;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using NUnit.Framework;
 using Assert = NUnit.Framework.Assert;
@@ -84,27 +86,51 @@
         [Test]
         public void InvalidCharactersLanguage2Test()
         {
-            var nf = new NameFormatter();
-            var tn = nf.GetTableName("Test#?", WordsExtraction.FromDictionary, EnglishNameFormat);
-            Assert.AreEqual("Test__", tn.ClassName);
+            try
+            {
+                ObjectFactory.Current.Register(typeof(EnglishWords));
+                var nf = new NameFormatter();
+                var tn = nf.GetTableName("Test#?", WordsExtraction.FromDictionary, EnglishNameFormat);
+                Assert.AreEqual("Test__", tn.ClassName);
+            }
+            finally
+            {
+                ObjectFactory.Current.Unregister(typeof(EnglishWords));
+            }
         }
 
         [TestMethod]
         [Test]
         public void GetWordsTest_MyTableName()
         {
-            var nf = new NameFormatter();
-            var tn = nf.GetTableName("MY_TABLE_NAME_", WordsExtraction.FromDictionary, EnglishNameFormat);
-            Assert.AreEqual("MyTableName", tn.ClassName);
+            try
+            {
+                ObjectFactory.Current.Register(typeof(EnglishWords));
+                var nf = new NameFormatter();
+                var tn = nf.GetTableName("MY_TABLE_NAME_", WordsExtraction.FromDictionary, EnglishNameFormat);
+                Assert.AreEqual("MyTableName", tn.ClassName);
+            }
+            finally
+            {
+                ObjectFactory.Current.Unregister(typeof(EnglishWords));
+            }
         }
 
         [TestMethod]
         [Test]
         public void GetWordsTest_MyTableName2()
         {
-            var nf = new NameFormatter();
-            var tn = nf.GetTableName("_MY_TABLE__NAME", WordsExtraction.FromDictionary, EnglishNameFormat);
-            Assert.AreEqual("MyTableName", tn.ClassName);
+            try
+            {
+                ObjectFactory.Current.Register(typeof(EnglishWords));
+                var nf = new NameFormatter();
+                var tn = nf.GetTableName("_MY_TABLE__NAME", WordsExtraction.FromDictionary, EnglishNameFormat);
+                Assert.AreEqual("MyTableName", tn.ClassName);
+            }
+            finally
+            {
+                ObjectFactory.Current.Unregister(typeof(EnglishWords));
+            }
         }
 
     }
Index: src/DbMetal/Generator/Implementation/Processor.cs
===================================================================
--- src/DbMetal/Generator/Implementation/Processor.cs	(revision 1028)
+++ src/DbMetal/Generator/Implementation/Processor.cs	(working copy)
@@ -165,7 +165,7 @@
         {
             foreach (var codeGeneratorType in ObjectFactory.Current.GetImplementations(typeof(ICodeGenerator)))
             {
-                yield return (ICodeGenerator)ObjectFactory.Current.GetInstance(codeGeneratorType, false);
+                yield return (ICodeGenerator)ObjectFactory.Current.Get(codeGeneratorType);
             }
         }
 
Index: src/DbLinq/Language/Implementation/Languages.cs
===================================================================
--- src/DbLinq/Language/Implementation/Languages.cs	(revision 1028)
+++ src/DbLinq/Language/Implementation/Languages.cs	(working copy)
@@ -47,7 +47,7 @@
             {
                 // checks for instance, but doesn't load it here
                 // (loading is slow)
-                var language = (ILanguageWords)objectFactory.GetInstance(languageType, false);
+                var language = (ILanguageWords)objectFactory.Get(languageType);
                 if (language.Supports(cultureInfo))
                 {
                     // if we have the right language, we load it
Index: src/DbLinq/Factory/Implementation/ReflectionObjectFactory.cs
===================================================================
--- src/DbLinq/Factory/Implementation/ReflectionObjectFactory.cs	(revision 1028)
+++ src/DbLinq/Factory/Implementation/ReflectionObjectFactory.cs	(working copy)
@@ -36,7 +36,7 @@
     /// Object factory. Main objects (most of them are stateless) are created with this class
     /// This may allow later to inject dependencies with a third party injector (I'm a Spring.NET big fan)
     /// </summary>
-    internal class ReflectionObjectFactory : AbstractObjectFactory
+    internal class ReflectionObjectFactory : IObjectFactory
     {
         private IDictionary<Type, IList<Type>> implementations;
         /// <summary>
@@ -106,18 +106,7 @@
                 var assemblyTypes = assembly.GetTypes();
                 foreach (Type type in assemblyTypes)
                 {
-                    if (type.IsAbstract)
-                        continue;
-                    foreach (Type i in type.GetInterfaces())
-                    {
-                        if (i.Assembly.GetCustomAttributes(typeof(DbLinqAttribute), false).Length > 0)
-                        {
-                            IList<Type> types;
-                            if (!interfaceImplementations.TryGetValue(i, out types))
-                                interfaceImplementations[i] = types = new List<Type>();
-                            types.Add(type);
-                        }
-                    }
+                    Register(type, interfaceImplementations);
                 }
             }
             catch (ReflectionTypeLoadException)
@@ -125,16 +114,43 @@
             }
         }
 
+        private void Register(Type type, IDictionary<Type, IList<Type>> interfaceImplementations)
+        {
+            if (type.IsAbstract)
+                return;
+            foreach (Type i in type.GetInterfaces())
+            {
+                if (i.Assembly.GetCustomAttributes(typeof(DbLinqAttribute), false).Length > 0)
+                {
+                    IList<Type> types;
+                    if (!interfaceImplementations.TryGetValue(i, out types))
+                        interfaceImplementations[i] = types = new List<Type>();
+                    types.Add(type);
+                }
+            }
+        }
+
+        public void Register(Type implementationType)
+        {
+            Register(implementationType, Implementations);
+        }
+
+        public void Unregister(Type implementationType)
+        {
+            foreach (var entry in Implementations)
+                entry.Value.Remove(implementationType);
+        }
+
         /// <summary>
         /// Gets the singleton.
         /// </summary>
         /// <param name="t">The t.</param>
         /// <returns></returns>
-        private object GetSingleton(Type t)
+        public object Get(Type t)
         {
             object r;
             if (!Singletons.TryGetValue(t, out r))
-                Singletons[t] = r = GetNewInstance(t);
+                Singletons[t] = r = Create(t);
             return r;
         }
 
@@ -143,7 +159,7 @@
         /// </summary>
         /// <param name="t">The t.</param>
         /// <returns></returns>
-        private object GetNewInstance(Type t)
+        public object Create(Type t)
         {
             //warning - the Activator.CreateInstance below was throwing unerported exceptions (as of 2008June).
             //So - let's add two future rules:
@@ -162,24 +178,11 @@
         }
 
         /// <summary>
-        /// Underlying method for Get&lt;T&gt; and Create&lt;T&gt;
-        /// </summary>
-        /// <param name="t"></param>
-        /// <param name="newInstanceRequired"></param>
-        /// <returns></returns>
-        public override object GetInstance(Type t, bool newInstanceRequired)
-        {
-            if (newInstanceRequired)
-                return GetNewInstance(t);
-            return GetSingleton(t);
-        }
-
-        /// <summary>
         /// Returns a list of types implementing the required interface
         /// </summary>
         /// <param name="interfaceType"></param>
         /// <returns></returns>
-        public override IEnumerable<Type> GetImplementations(Type interfaceType)
+        public IEnumerable<Type> GetImplementations(Type interfaceType)
         {
             return Implementations[interfaceType];
         }
Index: src/DbLinq/Factory/IObjectFactory.cs
===================================================================
--- src/DbLinq/Factory/IObjectFactory.cs	(revision 1028)
+++ src/DbLinq/Factory/IObjectFactory.cs	(working copy)
@@ -44,22 +44,17 @@
         /// </summary>
         /// <typeparam name="T">class or interface</typeparam>
         /// <returns></returns>
-        T Get<T>();
+        object Get(Type interfaceType);
 
         /// <summary>
         /// Returns a new instance of the specified class (can not be a singleton)
         /// </summary>
         /// <typeparam name="T">class or interface</typeparam>
         /// <returns></returns>
-        T Create<T>();
+        object Create(Type interfaceType);
 
-        /// <summary>
-        /// Underlying method for Get&lt;T> and Create&lt;T>
-        /// </summary>
-        /// <param name="t"></param>
-        /// <param name="newInstanceRequired"></param>
-        /// <returns></returns>
-        object GetInstance(Type t, bool newInstanceRequired);
+        void Register(Type implementationType);
+        void Unregister(Type implementationType);
 
         /// <summary>
         /// Returns a list of types implementing the required interface
@@ -68,4 +63,20 @@
         /// <returns></returns>
         IEnumerable<Type> GetImplementations(Type interfaceType);
     }
+
+#if !MONO_STRICT
+    public
+#endif
+    static class ObjectFactoryExtensions
+    {
+        public static T Create<T>(this IObjectFactory self)
+        {
+            return (T) self.Create(typeof(T));
+        }
+
+        public static T Get<T>(this IObjectFactory self)
+        {
+            return (T) self.Get(typeof(T));
+        }
+    }
 }
Index: src/DbLinq/DbLinq.csproj
===================================================================
--- src/DbLinq/DbLinq.csproj	(revision 1028)
+++ src/DbLinq/DbLinq.csproj	(working copy)
@@ -202,7 +202,6 @@
     <Compile Include="Data\Linq\Table.Extended.cs" />
     <Compile Include="DbLinqToDoAttribute.cs" />
     <Compile Include="Factory\DbLinqAttribute.cs" />
-    <Compile Include="Factory\Implementation\AbstractObjectFactory.cs" />
     <Compile Include="Factory\Implementation\ReflectionObjectFactory.cs" />
     <Compile Include="Factory\IObjectFactory.cs" />
     <Compile Include="Factory\ObjectFactory.cs" />
Index: src/DbLinq/System.Data.Linq.csproj
===================================================================
--- src/DbLinq/System.Data.Linq.csproj	(revision 1030)
+++ src/DbLinq/System.Data.Linq.csproj	(working copy)
@@ -232,7 +232,6 @@
     <Compile Include="Data\Linq\Table.cs" />
     <Compile Include="DbLinqToDoAttribute.cs" />
     <Compile Include="Factory\DbLinqAttribute.cs" />
-    <Compile Include="Factory\Implementation\AbstractObjectFactory.cs" />
     <Compile Include="Factory\Implementation\ReflectionObjectFactory.cs" />
     <Compile Include="Factory\IObjectFactory.cs" />
     <Compile Include="Factory\ObjectFactory.cs" />

Reply via email to