Better logging for DllPackageFragmentInstaller; IDataItemTreeAttachme…

…ntPoint - improved XML comments;
Orckestra · Nov 2, 2021 · 1331e98 · 1331e98
1 parent c65ac0f
commit 1331e98
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 20 deletions.
diff --git a/Composite/Core/PackageSystem/PackageFragmentInstallers/DllPackageFragmentInstaller.cs b/Composite/Core/PackageSystem/PackageFragmentInstallers/DllPackageFragmentInstaller.cs
@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
@@ -23,7 +23,7 @@ public sealed class DllPackageFragmentInstaller : BasePackageFragmentInstaller
     {
         private List<FileToCopy> _filesToCopy;
 
-        private static readonly string LogTitle = typeof (DllPackageFragmentInstaller).Name;
+        private static readonly string LogTitle = nameof(DllPackageFragmentInstaller);
 
         /// <exclude />
         public override IEnumerable<PackageFragmentValidationResult> Validate()
@@ -197,10 +197,11 @@ public override IEnumerable<XElement> Install()
                 this.InstallerContext.ZipFileSystem.WriteFileToDisk(fileToCopy.SourceFilename, tempFileName);
 
                 // Checking for dll version here:
-                var sourceAssemblyName = AssemblyName.GetAssemblyName(tempFileName);
+                var sourceAssemblyName = GetAssemblyNameWithErrorText(tempFileName, () => $"Source file name: '{fileToCopy.SourceFilename}'");
+
                 var sourceAssemblyVersion = sourceAssemblyName.Version;
                 var sourceFileVersion = GetDllFileVersion(tempFileName);
-                
+
                 string targetDirectory = Path.GetDirectoryName(fileToCopy.TargetFilePath);
                 if (!Directory.Exists(targetDirectory))
                 {
@@ -213,10 +214,11 @@ public override IEnumerable<XElement> Install()
 
                 if (C1File.Exists(fileToCopy.TargetFilePath) && fileToCopy.Overwrite)
                 {
-                    var existingAssemblyVersion = AssemblyName.GetAssemblyName(fileToCopy.TargetFilePath).Version;
+                    var existingAssemblyName = GetAssemblyNameWithErrorText(fileToCopy.TargetFilePath, () => $"TargetFilePath: '{fileToCopy.TargetFilePath}' ");
+                    var existingAssemblyVersion = existingAssemblyName.Version;
                     var existingFileVersion = GetDllFileVersion(fileToCopy.TargetFilePath);
 
-                    if (existingAssemblyVersion == sourceAssemblyVersion 
+                    if (existingAssemblyVersion == sourceAssemblyVersion
                         && existingFileVersion >= sourceFileVersion)
                     {
                         Log.LogInformation(LogTitle,
@@ -254,12 +256,11 @@ public override IEnumerable<XElement> Install()
                     Log.LogInformation(LogTitle, "Overwriting existing file '{0}' version '{2}', new version is '{1}'",
                         fileToCopy.TargetRelativeFilePath, sourceFileVersion, existingFileVersion);
                 }
-                
+
                 if (addAssemblyBinding)
                 {
                     asmBindingsToAdd.Add(sourceAssemblyName);
                 }
-
 
                 File.Delete(fileToCopy.TargetFilePath);
                 File.Move(tempFileName, fileToCopy.TargetFilePath);
@@ -281,6 +282,18 @@ public override IEnumerable<XElement> Install()
             yield return new XElement("Files", fileElements);
         }
 
+        private AssemblyName GetAssemblyNameWithErrorText(string filePath, Func<string> getErrorText)
+        {
+            try
+            {
+                return AssemblyName.GetAssemblyName(filePath);
+            }
+            catch (Exception ex)
+            {
+                throw new InvalidOperationException("Failed to read AssemblyName from a DLL. " + getErrorText(), ex);
+            }
+        }
+
         private Version GetDllFileVersion(string dllFilePath)
         {
             var fileVersionInfo = FileVersionInfo.GetVersionInfo(dllFilePath);
@@ -416,8 +429,8 @@ private string GetPublicKeyToken(AssemblyName assemblyName)
             {
                 byte[] publicKeyTokenBytes = assemblyName.GetPublicKeyToken();
 
-                return publicKeyTokenBytes == null || publicKeyTokenBytes.Length == 0 
-                    ? "null" 
+                return publicKeyTokenBytes == null || publicKeyTokenBytes.Length == 0
+                    ? "null"
                     : string.Join("", publicKeyTokenBytes.Select(b => $"{b:x2}"));
             }
 
@@ -474,6 +487,6 @@ public string OldVersion
                 }
             }
         }
-    
+
     }
 }
diff --git a/Composite/Data/Types/IDataItemTreeAttachmentPoint.cs b/Composite/Data/Types/IDataItemTreeAttachmentPoint.cs
@@ -1,47 +1,60 @@
-using System;
+using System;
+using Composite.C1Console.Elements.Plugins.ElementAttachingProvider;
 using Composite.Data.Hierarchy;
 using Composite.Data.Hierarchy.DataAncestorProviders;
 
 
 namespace Composite.Data.Types
 {
-    /// <summary>    
+    /// <summary>
+    /// Represents a link between a tree definition and a data item.
+    /// F.e. a tree that shows navigation elements, attached to a specific C1 page.
     /// </summary>
     /// <exclude />
     [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)] 
     [AutoUpdateble]
     [ImmutableTypeId("{24CDC117-4510-41C4-8A73-D1B4CD85FE2A}")]
-    [KeyPropertyName("Id")]
+    [KeyPropertyName(nameof(Id))]
     [DataAncestorProvider(typeof(NoAncestorDataAncestorProvider))]
     [DataScope(DataScopeIdentifier.PublicName)]
-    [CachingAttribute(CachingType.Full)]
+    [Caching(CachingType.Full)]
     public interface IDataItemTreeAttachmentPoint : IData
     {
-        /// <exclude />
+        /// <summary>
+        /// The Id value for the attachment point.
+        /// </summary>
         [StoreFieldType(PhysicalStoreFieldType.Guid)]
         [ImmutableFieldId("{BEB44A8E-37FD-4FA6-A420-B252B8590AD6}")]
         Guid Id { get; set; }
 
 
-        /// <exclude />
+        /// <summary>
+        /// The Id of the tree that is attached.
+        /// </summary>
         [StoreFieldType(PhysicalStoreFieldType.LargeString)]
         [ImmutableFieldId("{FA9A44E0-9D41-491A-86C0-BC5189FFC023}")]
         string TreeId { get; set; }
 
 
-        /// <exclude />
+        /// <summary>
+        /// The position in which the tree elements should be shown, f.e. "Top" or "Bottom". See <see cref="ElementAttachingProviderPosition"/>
+        /// </summary>
         [StoreFieldType(PhysicalStoreFieldType.String, 64)]
         [ImmutableFieldId("{6624A6CF-29DC-4E6A-8B88-25514BC00758}")]
         string Position { get; set; }
 
 
-        /// <exclude />
+        /// <summary>
+        /// The data type of the data item, to which a tree is attached.
+        /// </summary>
         [StoreFieldType(PhysicalStoreFieldType.LargeString)]
         [ImmutableFieldId("{F08DB525-2218-4379-A6F6-A9A904DEF6FE}")]
         string InterfaceType { get; set; }
 
 
-        /// <exclude />
+        /// <summary>
+        /// The key value of the data item to which a tree is attached.
+        /// </summary>
         [StoreFieldType(PhysicalStoreFieldType.LargeString)]
         [ImmutableFieldId("{8A50C365-586D-45F5-8881-EC3878E16593}")]
         string KeyValue { get; set; }