Fixed a permenant routing failure after the first error. The folder router most likely did not experience this issue. But plugin routers would see this. A new router instance is now used for each message. Test added.

joehook@gmail.com · joehook@gmail.com · commit 25f9c787bc82 · 2017-05-30T22:47:37.000-07:00
diff --git a/csharp/gateway/smtpAgent/FolderRoute.cs b/csharp/gateway/smtpAgent/FolderRoute.cs
@@ -94,7 +94,8 @@ public FolderBalancer<ISmtpMessage> LoadBalancer
                 return m_loadBalancer;
             }
         }
-        
+
+        /// <inheritdoc />
         public override void Validate()
         {
             base.Validate();
@@ -104,11 +105,25 @@ public override void Validate()
             }
         }
 
+        /// <inheritdoc />
+        public override Route Clone()
+        {
+            var route = new FolderRoute();
+            route.CopyFolders = this.CopyFolders;
+            route.AddressType = this.AddressType;
+            route.CopyMessageHandler = this.CopyMessageHandler;
+            route.FailedDelivery = this.FailedDelivery;
+
+            return route;
+        }
+
+        /// <inheritdoc />
         public override void Init()
         {
             this.Validate();            
         }
 
+        /// <inheritdoc />
         public override bool Process(ISmtpMessage message)
         {
             return m_loadBalancer.Process(message);
diff --git a/csharp/gateway/smtpAgent/MessageRouter.cs b/csharp/gateway/smtpAgent/MessageRouter.cs
@@ -43,7 +43,13 @@ internal MessageRouter(AgentDiagnostics diagnostics)
             m_diagnostics = diagnostics;
             m_routes = new Dictionary<string, Route>(StringComparer.OrdinalIgnoreCase);
         }
-        
+
+        internal MessageRouter(AgentDiagnostics diagnostics, Dictionary<string, Route> routes)
+        {
+            m_diagnostics = diagnostics;
+            m_routes = routes;
+        }
+
         internal ILogger Logger
         {
             get
@@ -200,7 +206,25 @@ void Route(ISmtpMessage message, Route route)
                 m_diagnostics.Logger.Error("Routing Error {0}, {1}", route.AddressType, ex);
             }
         }
-        
+
+        /// <summary>
+        /// new copy of router instance
+        /// </summary>
+        /// <returns></returns>
+        public MessageRouter Clone()
+        {
+            Dictionary<string, Route> routes = new Dictionary<string, Route>(StringComparer.OrdinalIgnoreCase);
+
+            foreach (var mRoute in m_routes)
+            {
+                routes[mRoute.Key] = mRoute.Value.Clone();
+            }
+
+            var messageRouter = new MessageRouter(m_diagnostics, routes);
+
+            return messageRouter;
+        }
+
         public IEnumerator<Route> GetEnumerator()
         {
             return m_routes.Values.GetEnumerator();
diff --git a/csharp/gateway/smtpAgent/PluginRoute.cs b/csharp/gateway/smtpAgent/PluginRoute.cs
@@ -119,7 +119,7 @@ public bool HasReceivers
                 return (!this.Receivers.IsNullOrEmpty());
             }
         }
-
+        /// <inheritdoc />
         public override void Validate()
         {
             base.Validate();
@@ -136,7 +136,20 @@ public override void Validate()
                 }
             }            
         }
-        
+
+        /// <inheritdoc />
+        public override Route Clone()
+        {
+            var route = new PluginRoute();
+            route.AddressType = this.AddressType;
+            route.m_plugins = this.ReceiverDefinitions;
+            route.Receivers = this.Receivers;
+            route.FailedDelivery = this.FailedDelivery;
+            route.m_loadBalancer = this.m_loadBalancer;
+
+            return route;
+        }
+
         public override void Init()
         {
             m_loadBalancer = new LoadBalancer<ISmtpMessage>(this.Receivers);
diff --git a/csharp/gateway/smtpAgent/Route.cs b/csharp/gateway/smtpAgent/Route.cs
@@ -14,12 +14,6 @@ Neither the name of The Direct Project (directproject.org) nor the names of its
  
 */
 using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Text;
-using Health.Direct.Agent;
-using Health.Direct.Config.Store;
-using Health.Direct.Common.Extensions;
 using System.Xml.Serialization;
 
 namespace Health.Direct.SmtpAgent
@@ -33,10 +27,16 @@ public abstract class Route
     {
         string m_addressType;
 
+        /// <summary>
+        /// Constructor
+        /// </summary>
         public Route()
         {
         }
-
+        
+        /// <summary>
+        /// Marker to indicate which router can route an email address
+        /// </summary>
         [XmlElement]
         public string AddressType
         {
@@ -55,32 +55,62 @@ public string AddressType
             }
         }
 
+        /// <summary>
+        /// 
+        /// </summary>
         public event Action<Route, Exception> Error;
 
-        public abstract void Init();        
+        /// <summary>
+        /// Initialize router
+        /// </summary>
+        public abstract void Init();
+        
+        /// <summary>
+        /// Process message though route
+        /// </summary>
+        /// <param name="message"></param>
+        /// <returns></returns>
         public abstract bool Process(ISmtpMessage message);
+
+        /// <summary>
+        /// Validate route config settings.
+        /// </summary>
         public virtual void Validate()
         {
-            if (string.IsNullOrEmpty(this.AddressType))
+            if (string.IsNullOrEmpty(AddressType))
             {
                 throw new SmtpAgentException(SmtpAgentError.MissingAddressTypeInRoute);
             }
         }
                 
+        /// <summary>
+        /// 
+        /// </summary>
+        /// <param name="ex"></param>
         protected void NotifyError(Exception ex)
         {
-            if (this.Error != null)
+            if (Error != null)
             {
                 try
                 {
-                    this.Error(this, ex);
+                    Error(this, ex);
                 }
                 catch
                 {
+                    // ignored
                 }
             }
         }
 
+        /// <summary>
+        /// Mark route as failed
+        /// </summary>
         public bool FailedDelivery { get; set; }
+
+        /// <summary>
+        /// Copy route
+        /// </summary>
+        /// <returns></returns>
+        public abstract Route Clone();
     }
 }
diff --git a/csharp/gateway/smtpAgent/SmtpAgent.cs b/csharp/gateway/smtpAgent/SmtpAgent.cs
@@ -745,12 +745,12 @@ void PostProcessIncoming(ISmtpMessage message, IncomingMessage envelope)
             
             if (envelope.HasDomainRecipients)
             {
-                DirectAddressCollection routedRecipients = new DirectAddressCollection();                
-                m_router.Route(message, envelope, routedRecipients); 
+                var routedRecipients = new DirectAddressCollection();
+                var messageRoute = m_router.Clone();
+                messageRoute.Route(message, envelope, routedRecipients);
                 
-                this.SendNotifications(envelope, routedRecipients);
-
-                SendDeliveryStatus(m_router, envelope, routedRecipients);
+                SendNotifications(envelope, routedRecipients);
+                SendDeliveryStatus(messageRoute, envelope, routedRecipients);
             }
             //
             // Any recipients that were handled by routes are no longer in the DomainRecipients collection (removed)
diff --git a/csharp/unittests/smtpAgent/SmtpAgentTestFiles/TestReceiverPlugin.xml b/csharp/unittests/smtpAgent/SmtpAgentTestFiles/TestReceiverPlugin.xml
@@ -0,0 +1,61 @@
+﻿<?xml version="1.0" encoding="utf-8" ?>
+<SmtpAgentConfig>
+  <Domain>nhind.hsgincubator.com</Domain>
+  <Log>
+    <DirectoryPath>C:\inetpub\logs</DirectoryPath>
+    <NamePrefix>testSmtpAgent</NamePrefix>
+    <RolloverFrequency>Day</RolloverFrequency>
+    <Level>Trace</Level>
+  </Log>
+  <Notifications>
+    <AutoResponse>false</AutoResponse>
+    <AlwaysAck>false</AlwaysAck>
+    <Text>Message Delivery Notification</Text>
+  </Notifications>
+  <PrivateCerts>
+    <MachineResolver>
+      <Name>NHINDPrivate</Name>
+    </MachineResolver>
+  </PrivateCerts>
+  <PublicCerts>
+    <MachineResolver>
+      <Name>NHINDExternal</Name>
+    </MachineResolver>
+  </PublicCerts>
+  <Anchors>
+    <MachineResolver>
+      <Incoming>
+        <Name>NHINDAnchors</Name>
+      </Incoming>
+      <Outgoing>
+        <Name>NHINDAnchors</Name>
+      </Outgoing>
+    </MachineResolver>
+  </Anchors>
+  <IncomingRoutes>
+   <PluginRoute>
+      <AddressType>STUB</AddressType>
+      <Receiver>
+        <TypeName>Health.Direct.SmtpAgent.Tests.StubPluginReciver, Health.Direct.SmtpAgent.Tests</TypeName>
+        <Settings>
+          <CopyFolder>C:\inetpub\mailroot\Incoming</CopyFolder>
+        </Settings>
+      </Receiver>
+    </PluginRoute>
+    <Route>
+      <AddressType>Undeliverable</AddressType>
+      <CopyFolder>C:\inetpub\mailroot\Gateway\Secure\Unknown</CopyFolder>
+    </Route>
+    <Route>
+      <AddressType>NormalCopy</AddressType>
+      <CopyFolder>C:\inetpub\mailroot\Incoming</CopyFolder>
+    </Route>
+  </IncomingRoutes>
+  <Container>
+    <Component>
+      <Service>Health.Direct.Common.Diagnostics.IAuditor, Health.Direct.Common</Service>
+      <Type>Health.Direct.SmtpAgent.Tests.DummyAuditor, Health.Direct.SmtpAgent.Tests</Type>
+      <Scope>Singleton</Scope>
+    </Component>
+  </Container>
+</SmtpAgentConfig>
diff --git a/csharp/unittests/smtpAgent/SmtpAgentTester.cs b/csharp/unittests/smtpAgent/SmtpAgentTester.cs
@@ -22,6 +22,7 @@ Neither the name of The Direct Project (directproject.org) nor the names of its
 using System.Net.Mime;
 using Health.Direct.Agent;
 using Health.Direct.Common.Cryptography;
+using Health.Direct.Common.Extensions;
 using Health.Direct.Common.Mail;
 using Health.Direct.Common.Mail.DSN;
 using Health.Direct.Common.Mail.Notifications;
@@ -45,6 +46,17 @@ public class SmtpAgentTester
 MIME-Version: 1.0
 Content-Type: text/plain
 
+Yo. Wassup?";
+
+        public static string TestMessageLoad =
+            @"From: <toby@redmond.hsgincubator.com>
+To: <jshook@nhind.hsgincubator.com>
+Subject: {1}
+Message-ID: {0}
+Date: Tue, 30 May 2017 22:10:27 -0700
+MIME-Version: 1.0
+Content-Type: text/plain
+
 Yo. Wassup?";
 
         public static string TestMessageTimelyAndReliable =
@@ -193,7 +205,8 @@ private void CleanMessages(string[] path)
         }
         private void CleanMessages(MessageProcessingSettings settings)
         {
-            if (settings == null) return;
+            if (settings == null || settings.CopyFolder.IsNullOrWhiteSpace()) return;
+
             CleanMessages(settings.CopyFolder);
         }
 
diff --git a/csharp/unittests/smtpAgent/TestRouter.cs b/csharp/unittests/smtpAgent/TestRouter.cs
diff --git a/csharp/unittests/smtpAgent/smtpagent.tests.csproj b/csharp/unittests/smtpAgent/smtpagent.tests.csproj

Original file line number	Diff line number	Diff line change
`@@ -94,7 +94,8 @@ public FolderBalancer<ISmtpMessage> LoadBalancer`
`94`	`94`	`return m_loadBalancer;`
`95`	`95`	`}`
`96`	`96`	`}`
`97`		`-`
	`97`	`+`
	`98`	`+ /// <inheritdoc />`
`98`	`99`	`public override void Validate()`
`99`	`100`	`{`
`100`	`101`	`base.Validate();`
`@@ -104,11 +105,25 @@ public override void Validate()`
`104`	`105`	`}`
`105`	`106`	`}`
`106`	`107`
	`108`	`+ /// <inheritdoc />`
	`109`	`+ public override Route Clone()`
	`110`	`+ {`
	`111`	`+ var route = new FolderRoute();`
	`112`	`+ route.CopyFolders = this.CopyFolders;`
	`113`	`+ route.AddressType = this.AddressType;`
	`114`	`+ route.CopyMessageHandler = this.CopyMessageHandler;`
	`115`	`+ route.FailedDelivery = this.FailedDelivery;`
	`116`	`+`
	`117`	`+ return route;`
	`118`	`+ }`
	`119`	`+`
	`120`	`+ /// <inheritdoc />`
`107`	`121`	`public override void Init()`
`108`	`122`	`{`
`109`	`123`	`this.Validate();`
`110`	`124`	`}`
`111`	`125`
	`126`	`+ /// <inheritdoc />`
`112`	`127`	`public override bool Process(ISmtpMessage message)`
`113`	`128`	`{`
`114`	`129`	`return m_loadBalancer.Process(message);`
Original file line number	Diff line number	Diff line change
`@@ -119,7 +119,7 @@ public bool HasReceivers`
`119`	`119`	`return (!this.Receivers.IsNullOrEmpty());`
`120`	`120`	`}`
`121`	`121`	`}`
`122`		`-`
	`122`	`+ /// <inheritdoc />`
`123`	`123`	`public override void Validate()`
`124`	`124`	`{`
`125`	`125`	`base.Validate();`
`@@ -136,7 +136,20 @@ public override void Validate()`
`136`	`136`	`}`
`137`	`137`	`}`
`138`	`138`	`}`
`139`		`-`
	`139`	`+`
	`140`	`+ /// <inheritdoc />`
	`141`	`+ public override Route Clone()`
	`142`	`+ {`
	`143`	`+ var route = new PluginRoute();`
	`144`	`+ route.AddressType = this.AddressType;`
	`145`	`+ route.m_plugins = this.ReceiverDefinitions;`
	`146`	`+ route.Receivers = this.Receivers;`
	`147`	`+ route.FailedDelivery = this.FailedDelivery;`
	`148`	`+ route.m_loadBalancer = this.m_loadBalancer;`
	`149`	`+`
	`150`	`+ return route;`
	`151`	`+ }`
	`152`	`+`
`140`	`153`	`public override void Init()`
`141`	`154`	`{`
`142`	`155`	`m_loadBalancer = new LoadBalancer<ISmtpMessage>(this.Receivers);`
Original file line number	Diff line number	Diff line change
`@@ -14,12 +14,6 @@ Neither the name of The Direct Project (directproject.org) nor the names of its`
`14`	`14`
`15`	`15`	`*/`
`16`	`16`	`using System;`
`17`		`-using System.Collections.Generic;`
`18`		`-using System.Linq;`
`19`		`-using System.Text;`
`20`		`-using Health.Direct.Agent;`
`21`		`-using Health.Direct.Config.Store;`
`22`		`-using Health.Direct.Common.Extensions;`
`23`	`17`	`using System.Xml.Serialization;`
`24`	`18`
`25`	`19`	`namespace Health.Direct.SmtpAgent`
`@@ -33,10 +27,16 @@ public abstract class Route`
`33`	`27`	`{`
`34`	`28`	`string m_addressType;`
`35`	`29`
	`30`	`+ /// <summary>`
	`31`	`+ /// Constructor`
	`32`	`+ /// </summary>`
`36`	`33`	`public Route()`
`37`	`34`	`{`
`38`	`35`	`}`
`39`		`-`
	`36`	`+`
	`37`	`+ /// <summary>`
	`38`	`+ /// Marker to indicate which router can route an email address`
	`39`	`+ /// </summary>`
`40`	`40`	`[XmlElement]`
`41`	`41`	`public string AddressType`
`42`	`42`	`{`
`@@ -55,32 +55,62 @@ public string AddressType`
`55`	`55`	`}`
`56`	`56`	`}`
`57`	`57`
	`58`	`+ /// <summary>`
	`59`	`+ ///`
	`60`	`+ /// </summary>`
`58`	`61`	`public event Action<Route, Exception> Error;`
`59`	`62`
`60`		`- public abstract void Init();`
	`63`	`+ /// <summary>`
	`64`	`+ /// Initialize router`
	`65`	`+ /// </summary>`
	`66`	`+ public abstract void Init();`
	`67`	`+`
	`68`	`+ /// <summary>`
	`69`	`+ /// Process message though route`
	`70`	`+ /// </summary>`
	`71`	`+ /// <param name="message"></param>`
	`72`	`+ /// <returns></returns>`
`61`	`73`	`public abstract bool Process(ISmtpMessage message);`
	`74`	`+`
	`75`	`+ /// <summary>`
	`76`	`+ /// Validate route config settings.`
	`77`	`+ /// </summary>`
`62`	`78`	`public virtual void Validate()`
`63`	`79`	`{`
`64`		`- if (string.IsNullOrEmpty(this.AddressType))`
	`80`	`+ if (string.IsNullOrEmpty(AddressType))`
`65`	`81`	`{`
`66`	`82`	`throw new SmtpAgentException(SmtpAgentError.MissingAddressTypeInRoute);`
`67`	`83`	`}`
`68`	`84`	`}`
`69`	`85`
	`86`	`+ /// <summary>`
	`87`	`+ ///`
	`88`	`+ /// </summary>`
	`89`	`+ /// <param name="ex"></param>`
`70`	`90`	`protected void NotifyError(Exception ex)`
`71`	`91`	`{`
`72`		`- if (this.Error != null)`
	`92`	`+ if (Error != null)`
`73`	`93`	`{`
`74`	`94`	`try`
`75`	`95`	`{`
`76`		`- this.Error(this, ex);`
	`96`	`+ Error(this, ex);`
`77`	`97`	`}`
`78`	`98`	`catch`
`79`	`99`	`{`
	`100`	`+ // ignored`
`80`	`101`	`}`
`81`	`102`	`}`
`82`	`103`	`}`
`83`	`104`
	`105`	`+ /// <summary>`
	`106`	`+ /// Mark route as failed`
	`107`	`+ /// </summary>`
`84`	`108`	`public bool FailedDelivery { get; set; }`
	`109`	`+`
	`110`	`+ /// <summary>`
	`111`	`+ /// Copy route`
	`112`	`+ /// </summary>`
	`113`	`+ /// <returns></returns>`
	`114`	`+ public abstract Route Clone();`
`85`	`115`	`}`
`86`	`116`	`}`