Fix NPE issue in PackageRelationshipCollection

openxml4Net/OPC/PackageRelationshipCollection.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Text;
 using System.Xml;
 using System.Xml.XPath;
@@ -27,10 +28,6 @@ public class PackageRelationshipCollection : IEnumerator<PackageRelationship>
         private SortedList<String, PackageRelationship> relationshipsByID;
 
         /**
-         * Package relationships ordered by type.
-         */
-        private SortedList<String, PackageRelationship> relationshipsByType;
-        /**
  * A lookup of internal relationships to avoid
  */
         private SortedList<String, PackageRelationship> internalRelationshipsByTargetName;
@@ -65,7 +62,6 @@ public class PackageRelationshipCollection : IEnumerator<PackageRelationship>
         public PackageRelationshipCollection()
         {
             relationshipsByID = new SortedList<String, PackageRelationship>();
-            relationshipsByType = new SortedList<String, PackageRelationship>(new DuplicateComparer());
             internalRelationshipsByTargetName = new SortedList<string, PackageRelationship>();
         }
         class DuplicateComparer : IComparer<string>
@@ -205,7 +201,6 @@ public void AddRelationship(PackageRelationship relPart)
                 throw new ArgumentException("invalid relationship part/id");
             }
             relationshipsByID[relPart.Id] = relPart;
-            relationshipsByType[relPart.RelationshipType] = relPart;
         }
 
         /**
@@ -225,9 +220,9 @@ public void AddRelationship(PackageRelationship relPart)
         public PackageRelationship AddRelationship(Uri targetUri,
                 TargetMode targetMode, String relationshipType, String id)
         {
-            if (id == null)
+            if (string.IsNullOrEmpty(id))
             {
-                // Generate a unique ID is id parameter is null.
+                // Generate a unique ID if id parameter is null.
                 if (nextRelationshipId == -1)
                 {
                     nextRelationshipId = Size + 1;
@@ -243,7 +238,6 @@ public PackageRelationship AddRelationship(Uri targetUri,
             PackageRelationship rel = new PackageRelationship(container,
                     sourcePart, targetUri, targetMode, relationshipType, id);
             relationshipsByID[rel.Id] = rel;
-            relationshipsByType[rel.RelationshipType] = rel;
             if (targetMode == TargetMode.Internal
                 && !internalRelationshipsByTargetName.ContainsKey(targetUri.OriginalString))
             {
@@ -260,20 +254,15 @@ public PackageRelationship AddRelationship(Uri targetUri,
          */
         public void RemoveRelationship(String id)
         {
-            if (relationshipsByID != null && relationshipsByType != null)
+            if(relationshipsByID == null)
             {
-                PackageRelationship rel = relationshipsByID[id];
-                if (rel != null)
-                {
-                    relationshipsByID.Remove(rel.Id);
-                    for (int i = 0; i < relationshipsByType.Count; i++)
-                    {
-                        if (relationshipsByType.Values[i] == rel)
-                            relationshipsByType.RemoveAt(i);
-                    }
-
-                    internalRelationshipsByTargetName.RemoveAt(internalRelationshipsByTargetName.IndexOfValue(rel));
-                }
+                return;
+            }
+            PackageRelationship rel = relationshipsByID[id];
+            if (rel != null)
+            {
+                relationshipsByID.Remove(rel.Id);                    
+                internalRelationshipsByTargetName.RemoveAt(internalRelationshipsByTargetName.IndexOfValue(rel));
             }
         }
 
@@ -289,7 +278,6 @@ public void RemoveRelationship(PackageRelationship rel)
                 throw new ArgumentException("rel");
 
             relationshipsByID.Values.Remove(rel);
-            relationshipsByType.Values.Remove(rel);
         }
 
         /**
@@ -321,6 +309,11 @@ public PackageRelationship GetRelationship(int index)
          */
         public PackageRelationship GetRelationshipByID(String id)
         {
+            if(id==null)
+            {
+                throw new ArgumentException("Cannot read relationship, provided ID is empty: " + id +
+                    ", having relationships: " + relationshipsByID.Keys.Select(key=>string.Join(",", key)));
+            }
             if (!relationshipsByID.ContainsKey(id))
                 return null;
             return relationshipsByID[id];
@@ -466,7 +459,6 @@ public IEnumerator<PackageRelationship> Iterator(String typeFilter)
         public void Clear()
         {
             relationshipsByID.Clear();
-            relationshipsByType.Clear();
             internalRelationshipsByTargetName.Clear();
         }
         public PackageRelationship FindExistingInternalRelation(PackagePart packagePart)
@@ -529,8 +521,7 @@ PackageRelationship IEnumerator<PackageRelationship>.Current
 
         void IDisposable.Dispose()
         {
-            //relationshipsByID=null;
-            //relationshipsByType = null;
+
         }
 
         #endregion