From 4b2d80d2cd79e1bfc4303da6bd0a8d7d85bdab70 Mon Sep 17 00:00:00 2001 From: Dmitry Dzygin Date: Fri, 4 Nov 2016 10:27:44 +0100 Subject: [PATCH 1/6] Rewriting PhantomJS integration to use JSON and collect execution log --- Composite/Composite.csproj | 21 +- Composite/Core/WebClient/BrowserRender.cs | 33 +- .../WebClient/BrowserRender_PhantomServer.cs | 395 ---------------- .../Core/WebClient/PhantomJs/PhantomServer.cs | 438 ++++++++++++++++++ .../PhantomJs/RenderPreviewRequest.cs | 36 ++ .../WebClient/PhantomJs/RenderingResult.cs | 12 + .../PhantomJs/RenderingResultStatus.cs | 13 + .../WebClient/Renderings/FunctionPreview.cs | 14 +- .../WysiwygEditor/PageTemplatePreview.cs | 18 +- .../Composite/PhantomJs/renderingServer.js | 211 +++++---- 10 files changed, 659 insertions(+), 532 deletions(-) delete mode 100644 Composite/Core/WebClient/BrowserRender_PhantomServer.cs create mode 100644 Composite/Core/WebClient/PhantomJs/PhantomServer.cs create mode 100644 Composite/Core/WebClient/PhantomJs/RenderPreviewRequest.cs create mode 100644 Composite/Core/WebClient/PhantomJs/RenderingResult.cs create mode 100644 Composite/Core/WebClient/PhantomJs/RenderingResultStatus.cs diff --git a/Composite/Composite.csproj b/Composite/Composite.csproj index 193ea68102..e7c16b30fb 100644 --- a/Composite/Composite.csproj +++ b/Composite/Composite.csproj @@ -174,6 +174,10 @@ + + + + @@ -263,7 +267,6 @@ - @@ -2521,16 +2524,16 @@ - - copy "$(TargetPath)" "$(ProjectDir)..\bin\" + copy "$(TargetPath)" "$(ProjectDir)..\bin\" copy "$(TargetPath)" "$(ProjectDir)..\Website\bin\" \ No newline at end of file diff --git a/Composite/Core/WebClient/BrowserRender.cs b/Composite/Core/WebClient/BrowserRender.cs index 73c1dde5d3..acc7ad4431 100644 --- a/Composite/Core/WebClient/BrowserRender.cs +++ b/Composite/Core/WebClient/BrowserRender.cs @@ -13,10 +13,11 @@ using Composite.Data.Plugins.DataProvider.Streams; using Timer = System.Timers.Timer; using System.Threading.Tasks; +using Composite.Core.WebClient.PhantomJs; namespace Composite.Core.WebClient { - internal static partial class BrowserRender + internal static class BrowserRender { private static readonly string LogTitle = typeof (BrowserRender).Name; private static readonly string CacheImagesFolder = Path.Combine(PathUtil.Resolve(GlobalSettingsFacade.CacheDirectory), "PreviewImages"); @@ -39,25 +40,6 @@ static BrowserRender() private static volatile bool ServerAvailabilityChecked; private static readonly AsyncLock _serverAvailabilityCheckLock = new AsyncLock(); - public enum RenderingResultStatus - { - Success = 0, - Redirect = 1, - Error = 2, - Timeout = 3, - PhantomServerTimeout = 4, - PhantomServerIncorrectResponse = 5, - PhantomServerNoOutput = 6 - } - - public class RenderingResult - { - public RenderingResultStatus Status { get; set; } - public string FilePath { get; set; } - public string Output { get; set; } - public string RedirectUrl { get; set; } - } - /// /// Ensures that the BrowserRenderer service is launched, without blocking the current thread /// @@ -104,6 +86,7 @@ public static async Task RenderUrlAsync(HttpContext context, st string outputImageFileName = Path.Combine(dropFolder, urlHash + ".png"); string outputFileName = Path.Combine(dropFolder, urlHash + ".output"); + string redirectLogFileName = Path.Combine(dropFolder, urlHash + ".redirect"); string errorFileName = Path.Combine(dropFolder, urlHash + ".error"); if (C1File.Exists(outputImageFileName) || C1File.Exists(outputFileName)) @@ -111,7 +94,7 @@ public static async Task RenderUrlAsync(HttpContext context, st #if BrowserRender_NoCache File.Delete(outputFileName); #else - string output = C1File.Exists(outputFileName) ? C1File.ReadAllText(outputFileName) : null; + string[] output = C1File.Exists(outputFileName) ? C1File.ReadAllLines(outputFileName) : null; return new RenderingResult { FilePath = outputImageFileName, Output = output, Status = RenderingResultStatus.Success}; #endif @@ -126,7 +109,7 @@ public static async Task RenderUrlAsync(HttpContext context, st if (result.Status >= RenderingResultStatus.Error) { - C1File.WriteAllText(errorFileName, result.Output); + C1File.WriteAllLines(errorFileName, result.Output); } if (!Enabled) @@ -136,7 +119,11 @@ public static async Task RenderUrlAsync(HttpContext context, st if (result.Status == RenderingResultStatus.Success) { - C1File.WriteAllText(outputFileName, result.Output); + C1File.WriteAllLines(outputFileName, result.Output); + } + else if (result.Status == RenderingResultStatus.Redirect) + { + C1File.WriteAllLines(redirectLogFileName, result.Output); } return result; diff --git a/Composite/Core/WebClient/BrowserRender_PhantomServer.cs b/Composite/Core/WebClient/BrowserRender_PhantomServer.cs deleted file mode 100644 index dd91c7c4de..0000000000 --- a/Composite/Core/WebClient/BrowserRender_PhantomServer.cs +++ /dev/null @@ -1,395 +0,0 @@ -using System; -using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; -using System.IO; -using System.Linq; -using System.Text; -using System.Threading.Tasks; -using System.Web; -using System.Web.Hosting; -using Composite.Core.Application; -using Composite.Core.Configuration; -using Composite.Core.Extensions; -using Composite.Core.IO; -using Composite.Core.Parallelization; - -namespace Composite.Core.WebClient -{ - internal static partial class BrowserRender - { - /// - /// Contains information about currently running instance of a PhantomJS server - /// - private class PhantomServer : IDisposable - { - const string ConfigFileName = "config.json"; - const string ScriptFileName = "renderingServer.js"; - static readonly string _phantomJsFolder = HostingEnvironment.MapPath("~/App_Data/Composite/PhantomJs"); - static readonly string _phantomJsPath = Path.Combine(_phantomJsFolder, "phantomjs.exe"); - - - private readonly StreamWriter _stdin; - private readonly StreamReader _stdout; - private readonly StreamReader _stderror; - - private readonly Process _process; - private readonly Job _job; - - private static PhantomServer _instance; - private static readonly AsyncLock _instanceAsyncLock = new AsyncLock(); - - [SuppressMessage("Composite.IO", "Composite.DotNotUseStreamWriterClass:DotNotUseStreamWriterClass")] - private PhantomServer() - { - var tempDirectory = PathUtil.Resolve(GlobalSettingsFacade.TempDirectory); - var cachePath = Path.Combine(tempDirectory, "phantomjs_cache"); - var localStoragePath = Path.Combine(tempDirectory, "phantomjs_ls"); - - _process = new Process - { - StartInfo = - { - WorkingDirectory = _phantomJsFolder, - FileName = "\"" + _phantomJsPath + "\"", - Arguments = $"\"--local-storage-path={localStoragePath}\" \"--disk-cache-path={cachePath}\" --config={ConfigFileName} {ScriptFileName}", - RedirectStandardOutput = true, - RedirectStandardError = true, - RedirectStandardInput = true, - CreateNoWindow = true, - StandardOutputEncoding = Encoding.UTF8, - UseShellExecute = false, - WindowStyle = ProcessWindowStyle.Hidden - } - }; - - _process.Start(); - - _stdin = _process.StandardInput; - _stdout = _process.StandardOutput; - _stderror = _process.StandardError; - _stdin.AutoFlush = true; - - _job = new Job(); - _job.AddProcess(_process.Handle); - } - - - public static void ShutDown(bool alreadyLocked, bool silent = false) - { - PhantomServer ps; - - using (alreadyLocked ? null : _instanceAsyncLock.Lock()) - { - if (_instance == null) - { - return; - } - - ps = _instance; - _instance = null; - } - - ps.DisposeInternal(silent); - GC.SuppressFinalize(ps); - } - - - // Ensures that an instance have been started - //public async static Task StartAsync() - //{ - // using (await _instanceAsyncLock.LockAsync()) - // { - // var instance = PhantomServer.Instance; - // } - //} - - - private static PhantomServer Instance - { - get - { - return _instance ?? (_instance = new PhantomServer()); - } - } - - - public static async Task RenderUrlAsync(HttpCookie[] cookies, string url, string outputImageFilePath, string mode) - { - using (await _instanceAsyncLock.LockAsync()) - { - _lastUsageDate = DateTime.Now; - var renderingResult = Instance.RenderUrlImpl(cookies, url, outputImageFilePath, mode); - - if (renderingResult.Status == RenderingResultStatus.PhantomServerTimeout - || renderingResult.Status == RenderingResultStatus.PhantomServerIncorrectResponse - || renderingResult.Status == RenderingResultStatus.PhantomServerNoOutput) - { - Log.LogWarning("PhantomServer", "Shutting down PhantomJs server. Reason: {0}, Output: {1}", renderingResult.Status, renderingResult.Output); - - try - { - ShutDown(true); - } - catch (Exception shutdownException) - { - Log.LogError("PhantomServer", shutdownException); - } - } - - return renderingResult; - } - } - - - private RenderingResult RenderUrlImpl(HttpCookie[] cookies, string url, string outputImageFilePath, string mode) - { - Verify.ArgumentNotNull(cookies, nameof(cookies)); - - string cookieDomain = new Uri(url).Host; - string cookieInfo = string.Join(",", - cookies.Select(cookie => cookie.Name + "," + cookie.Value + "," + cookieDomain)); - - string requestLine = cookieInfo + ";" + url + ";" + outputImageFilePath + ";" + mode; - - // Async way: - //Task readerTask = Task.Run(async () => - //{ - // await _stdin.WriteLineAsync(requestLine); - // return await _stdout.ReadLineAsync(); - //}); - - Task readerTask = Task.Run(() => - { - _stdin.WriteLine(requestLine); - return _stdout.ReadLine(); - }); - - var secondsSinceStartup = (DateTime.Now - _process.StartTime).TotalSeconds; - double timeout = secondsSinceStartup < 120 || mode == "test" ? 65 : 30; - - readerTask.Wait(TimeSpan.FromSeconds(timeout)); - - string output; - - switch (readerTask.Status) - { - case TaskStatus.RanToCompletion: - output = readerTask.Result; - if (output == null) - { - return new RenderingResult - { - Status = RenderingResultStatus.PhantomServerNoOutput, - Output = "(null)" - }; - } - break; - default: - return new RenderingResult - { - Status = RenderingResultStatus.PhantomServerTimeout, - Output = "Request failed to complete within expected time: " + -#if DEBUG - requestLine -#else - url + " " + mode -#endif - }; - } - - if (C1File.Exists(outputImageFilePath)) - { - return new RenderingResult - { - Status = RenderingResultStatus.Success, - Output = output, - FilePath = outputImageFilePath - }; - } - - const string redirectResponsePrefix = "REDIRECT: "; - if (output.StartsWith(redirectResponsePrefix)) - { - return new RenderingResult - { - Status = RenderingResultStatus.Redirect, - Output = output, - RedirectUrl = output.Substring(redirectResponsePrefix.Length) - }; - } - - const string timeoutResponsePrefix = "TIMEOUT: "; - if (output.StartsWith(timeoutResponsePrefix)) - { - return new RenderingResult - { - Status = RenderingResultStatus.Timeout, - Output = output, - RedirectUrl = output.Substring(timeoutResponsePrefix.Length) - }; - } - - const string errorResponsePrefix = "ERROR: "; - if (output.StartsWith(errorResponsePrefix)) - { - return new RenderingResult - { - Status = RenderingResultStatus.Error, - Output = output, - RedirectUrl = output.Substring(errorResponsePrefix.Length) - }; - } - - return new RenderingResult - { - Status = RenderingResultStatus.PhantomServerIncorrectResponse, - Output = output - }; - } - - public static string ScriptFilePath - { - get { return Path.Combine(_phantomJsFolder, ScriptFileName); } - } - - public void Dispose() - { - Dispose(true); - GC.SuppressFinalize(this); - } - - ~PhantomServer() - { - // Finalizer calls Dispose(false) - Dispose(false); - } - - void Dispose(bool disposing) - { - if (!disposing) - { - return; - } - - DisposeInternal(false); - } - - void DisposeInternal(bool silent) - { - bool processHasExited; - - try - { - processHasExited = _process.HasExited; - } - catch (Exception) - { - processHasExited = true; - } - - if (!processHasExited) - { - _stdin.WriteLine("exit"); - } - - bool streamsClosed = false; - - Task errorFeedbackTask = null; - string processOutputSummary = null; - - if (!silent) - { - errorFeedbackTask = Task.Factory.StartNew(() => - { - try - { - if (streamsClosed) - { - // Simplifies debugging - return null; - } - - string stdOut = _stdout.ReadToEnd(); - string stdError = _stderror.ReadToEnd(); - - string result = !string.IsNullOrEmpty(stdOut) ? $"stdout: '{stdOut}'" : ""; - - if (!string.IsNullOrWhiteSpace(stdError)) - { - if (result.Length > 0) { result += Environment.NewLine; } - - result += $"stderr: {stdError}"; - } - - return result; - } - catch (Exception ex) - { - return ex.Message; - } - }); - - - errorFeedbackTask.Wait(500); - - processOutputSummary = errorFeedbackTask.Status == TaskStatus.RanToCompletion ? errorFeedbackTask.Result : "Process Hang"; - } - - if (!processHasExited) - { - try - { - processHasExited = _process.HasExited; - } - catch (Exception) - { - processHasExited = true; - } - - if (!processHasExited) - { - streamsClosed = true; - - _stdin.Close(); - _stdout.Close(); - _stderror.Close(); - _process.Kill(); - _process.WaitForExit(500); - } - } - - int exitCode = _process.ExitCode; - - streamsClosed = true; - - _stdin.Dispose(); - _stdout.Dispose(); - _stderror.Dispose(); - - _process.Dispose(); - _job.Dispose(); - - bool meaningfullExitCode = exitCode != 0 && exitCode != -1073741819 /* Access violation, the ExitCode property returns this value by default for some reason */; - - if (silent) - { - return; - } - - if (meaningfullExitCode - || errorFeedbackTask.Status != TaskStatus.RanToCompletion - || !string.IsNullOrEmpty(processOutputSummary)) - { - string errorMessage = "Error executing PhantomJs.exe"; - if (meaningfullExitCode) errorMessage += "; Exit code: {0}".FormatWith(exitCode); - - if (!string.IsNullOrEmpty(processOutputSummary)) - { - errorMessage += Environment.NewLine + processOutputSummary; - } - throw new InvalidOperationException(errorMessage); - } - } - } - } -} diff --git a/Composite/Core/WebClient/PhantomJs/PhantomServer.cs b/Composite/Core/WebClient/PhantomJs/PhantomServer.cs new file mode 100644 index 0000000000..811753ef56 --- /dev/null +++ b/Composite/Core/WebClient/PhantomJs/PhantomServer.cs @@ -0,0 +1,438 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.IO; +using System.Linq; +using System.Runtime.Serialization.Json; +using System.Text; +using System.Threading.Tasks; +using System.Web; +using System.Web.Hosting; +using Composite.Core.Application; +using Composite.Core.Configuration; +using Composite.Core.Extensions; +using Composite.Core.IO; +using Composite.Core.Parallelization; + +namespace Composite.Core.WebClient.PhantomJs +{ + /// + /// Contains information about currently running instance of a PhantomJS server + /// + internal class PhantomServer : IDisposable + { + private const string LogTitle = nameof(PhantomServer); + private const string EndOfReplyMarker = "END_OF_REPLY"; + + const string ConfigFileName = "config.json"; + const string ScriptFileName = "renderingServer.js"; + static readonly string _phantomJsFolder = HostingEnvironment.MapPath("~/App_Data/Composite/PhantomJs"); + static readonly string _phantomJsPath = Path.Combine(_phantomJsFolder, "phantomjs.exe"); + + + private readonly StreamWriter _stdin; + private readonly StreamReader _stdout; + private readonly StreamReader _stderror; + + private readonly Process _process; + private readonly Job _job; + + private static PhantomServer _instance; + private static readonly AsyncLock _instanceAsyncLock = new AsyncLock(); + + [SuppressMessage("Composite.IO", "Composite.DotNotUseStreamWriterClass:DotNotUseStreamWriterClass")] + private PhantomServer() + { + var tempDirectory = PathUtil.Resolve(GlobalSettingsFacade.TempDirectory); + var cachePath = Path.Combine(tempDirectory, "phantomjs_cache"); + var localStoragePath = Path.Combine(tempDirectory, "phantomjs_ls"); + + _process = new Process + { + StartInfo = + { + WorkingDirectory = _phantomJsFolder, + FileName = "\"" + _phantomJsPath + "\"", + Arguments = $"\"--local-storage-path={localStoragePath}\" \"--disk-cache-path={cachePath}\" --config={ConfigFileName} {ScriptFileName}", + RedirectStandardOutput = true, + RedirectStandardError = true, + RedirectStandardInput = true, + CreateNoWindow = true, + StandardOutputEncoding = Encoding.UTF8, + UseShellExecute = false, + WindowStyle = ProcessWindowStyle.Hidden + } + }; + + _process.Start(); + + _stdin = _process.StandardInput; + _stdout = _process.StandardOutput; + _stderror = _process.StandardError; + _stdin.AutoFlush = true; + + _job = new Job(); + _job.AddProcess(_process.Handle); + } + + + public static void ShutDown(bool alreadyLocked, bool silent = false) + { + PhantomServer ps; + + using (alreadyLocked ? null : _instanceAsyncLock.Lock()) + { + if (_instance == null) + { + return; + } + + ps = _instance; + _instance = null; + } + + ps.DisposeInternal(silent); + GC.SuppressFinalize(ps); + } + + + // Ensures that an instance have been started + //public async static Task StartAsync() + //{ + // using (await _instanceAsyncLock.LockAsync()) + // { + // var instance = PhantomServer.Instance; + // } + //} + + + private static PhantomServer Instance + { + get + { + return _instance ?? (_instance = new PhantomServer()); + } + } + + + public static async Task RenderUrlAsync(HttpCookie[] cookies, string url, string outputImageFilePath, string mode) + { + url = url.Replace(" ", "%20"); // Preventing a redirect in PhantomJS + + using (await _instanceAsyncLock.LockAsync()) + { + var renderingResult = Instance.RenderUrlImpl(cookies, url, outputImageFilePath, mode); + + if (renderingResult.Status == RenderingResultStatus.PhantomServerTimeout + || renderingResult.Status == RenderingResultStatus.PhantomServerIncorrectResponse + || renderingResult.Status == RenderingResultStatus.PhantomServerNoOutput) + { + Log.LogWarning(LogTitle, "Shutting down PhantomJs server. Reason: {0}, Output: {1}", + renderingResult.Status, string.Join(Environment.NewLine, renderingResult.Output)); + + try + { + ShutDown(true); + } + catch (Exception shutdownException) + { + Log.LogError(LogTitle, shutdownException); + } + } + + return renderingResult; + } + } + + private bool IsEndOfReply(string line) + { + return line.StartsWith(EndOfReplyMarker); + } + + + private RenderingResult RenderUrlImpl(HttpCookie[] cookies, string url, string outputImageFilePath, string mode) + { + Verify.ArgumentNotNull(cookies, nameof(cookies)); + + string cookieDomain = new Uri(url).Host; + + var request = new RenderPreviewRequest + { + requestId = "1", + mode = mode, + url = url, + outputFilePath = outputImageFilePath, + cookies = cookies.Select(cookie => new CookieInformation + { + name = cookie.Name, + value = cookie.Value, + domain = cookieDomain + }).ToArray() + }; + + var ms = new MemoryStream(); + var ser = new DataContractJsonSerializer(typeof(RenderPreviewRequest)); + ser.WriteObject(ms, request); + + var json = Encoding.UTF8.GetString(ms.ToArray()); + + var output = new List(); + + Task readerTask = Task.Run(() => + { + _stdin.WriteLine(json); + + string line; + + do + { + line = _stdout.ReadLine(); + lock (output) + { + output.Add(line); + } + + } while (!IsEndOfReply(line)); + }); + + var secondsSinceStartup = (DateTime.Now - _process.StartTime).TotalSeconds; + double timeout = secondsSinceStartup < 120 || mode == "test" ? 65 : 30; + + readerTask.Wait(TimeSpan.FromSeconds(timeout)); + + // TODO: check for theother task statuses + switch (readerTask.Status) + { + case TaskStatus.RanToCompletion: + if (output.Count == 0) + { + return new RenderingResult + { + Status = RenderingResultStatus.PhantomServerNoOutput, + Output = new [] { "(null)" } + }; + } + break; + default: + string[] outputCopy; + lock (output) + { + outputCopy = output.ToArray(); + } + + string logMessage = "Request failed to complete within expected time: " + +#if DEBUG + json +#else + url + " " + mode +#endif + ; + + + return new RenderingResult + { + Status = RenderingResultStatus.PhantomServerTimeout, + Output = new [] { logMessage}.Concat(outputCopy).ToArray(), + FilePath = outputImageFilePath + }; + } + + if (C1File.Exists(outputImageFilePath)) + { + return new RenderingResult + { + Status = RenderingResultStatus.Success, + Output = output, + FilePath = outputImageFilePath + }; + } + + var lastMessage = output.Last(); + + if (!lastMessage.StartsWith(EndOfReplyMarker)) + { + Log.LogError(LogTitle, $"Missing {EndOfReplyMarker} in the response"); + } + + string redirectUrl = null; + RenderingResultStatus? status = null; + + foreach (var line in output) + { + const string redirectResponsePrefix = "REDIRECT: "; + + if (line == "SUCCESS") + { + status = RenderingResultStatus.Success; + } + else if (line.StartsWith(redirectResponsePrefix)) + { + status = RenderingResultStatus.Redirect; + redirectUrl = line.Substring(redirectResponsePrefix.Length); + } + else if (line.StartsWith("TIMEOUT: ")) + { + status = RenderingResultStatus.Timeout; + } + else if (line.StartsWith("ERROR: ")) + { + status = RenderingResultStatus.Error; + } + } + + status = status ?? RenderingResultStatus.PhantomServerIncorrectResponse; + + return new RenderingResult + { + Status = status.Value, + Output = output, + RedirectUrl = redirectUrl + }; + } + + public static string ScriptFilePath + { + get { return Path.Combine(_phantomJsFolder, ScriptFileName); } + } + + public void Dispose() + { + Dispose(true); + GC.SuppressFinalize(this); + } + + ~PhantomServer() + { + // Finalizer calls Dispose(false) + Dispose(false); + } + + void Dispose(bool disposing) + { + if (!disposing) + { + return; + } + + DisposeInternal(false); + } + + void DisposeInternal(bool silent) + { + bool processHasExited; + + try + { + processHasExited = _process.HasExited; + } + catch (Exception) + { + processHasExited = true; + } + + if (!processHasExited) + { + _stdin.WriteLine("exit"); + } + + bool streamsClosed = false; + + Task errorFeedbackTask = null; + string processOutputSummary = null; + + if (!silent) + { + errorFeedbackTask = Task.Factory.StartNew(() => + { + try + { + if (streamsClosed) + { + // Simplifies debugging + return null; + } + + string stdOut = _stdout.ReadToEnd(); + string stdError = _stderror.ReadToEnd(); + + string result = !string.IsNullOrEmpty(stdOut) ? $"stdout: '{stdOut}'" : ""; + + if (!string.IsNullOrWhiteSpace(stdError)) + { + if (result.Length > 0) { result += Environment.NewLine; } + + result += $"stderr: {stdError}"; + } + + return result; + } + catch (Exception ex) + { + return ex.Message; + } + }); + + + errorFeedbackTask.Wait(500); + + processOutputSummary = errorFeedbackTask.Status == TaskStatus.RanToCompletion ? errorFeedbackTask.Result : "Process Hang"; + } + + if (!processHasExited) + { + try + { + processHasExited = _process.HasExited; + } + catch (Exception) + { + processHasExited = true; + } + + if (!processHasExited) + { + streamsClosed = true; + + _stdin.Close(); + _stdout.Close(); + _stderror.Close(); + _process.Kill(); + _process.WaitForExit(500); + } + } + + int exitCode = _process.ExitCode; + + streamsClosed = true; + + _stdin.Dispose(); + _stdout.Dispose(); + _stderror.Dispose(); + + _process.Dispose(); + _job.Dispose(); + + bool meaningfullExitCode = exitCode != 0 && exitCode != -1073741819 /* Access violation, the ExitCode property returns this value by default for some reason */; + + if (silent) + { + return; + } + + if (meaningfullExitCode + || errorFeedbackTask.Status != TaskStatus.RanToCompletion + || !string.IsNullOrEmpty(processOutputSummary)) + { + string errorMessage = "Error executing PhantomJs.exe"; + if (meaningfullExitCode) errorMessage += "; Exit code: {0}".FormatWith(exitCode); + + if (!string.IsNullOrEmpty(processOutputSummary)) + { + errorMessage += Environment.NewLine + processOutputSummary; + } + throw new InvalidOperationException(errorMessage); + } + } + } +} diff --git a/Composite/Core/WebClient/PhantomJs/RenderPreviewRequest.cs b/Composite/Core/WebClient/PhantomJs/RenderPreviewRequest.cs new file mode 100644 index 0000000000..46648950e7 --- /dev/null +++ b/Composite/Core/WebClient/PhantomJs/RenderPreviewRequest.cs @@ -0,0 +1,36 @@ +using System.Runtime.Serialization; + +namespace Composite.Core.WebClient.PhantomJs +{ + [DataContract] + internal class CookieInformation + { + [DataMember] + public string name { get; set; } + + [DataMember] + public string value { get; set; } + + [DataMember] + public string domain { get; set; } + } + + [DataContract] + internal class RenderPreviewRequest + { + [DataMember] + public string requestId { get; set; } + + [DataMember] + public string mode { get; set; } + + [DataMember] + public string url { get; set; } + + [DataMember] + public string outputFilePath { get; set; } + + [DataMember] + public CookieInformation[] cookies { get; set; } + } +} diff --git a/Composite/Core/WebClient/PhantomJs/RenderingResult.cs b/Composite/Core/WebClient/PhantomJs/RenderingResult.cs new file mode 100644 index 0000000000..c567c9d00d --- /dev/null +++ b/Composite/Core/WebClient/PhantomJs/RenderingResult.cs @@ -0,0 +1,12 @@ +using System.Collections.Generic; + +namespace Composite.Core.WebClient.PhantomJs +{ + internal class RenderingResult + { + public RenderingResultStatus Status { get; set; } + public string FilePath { get; set; } + public ICollection Output { get; set; } + public string RedirectUrl { get; set; } + } +} diff --git a/Composite/Core/WebClient/PhantomJs/RenderingResultStatus.cs b/Composite/Core/WebClient/PhantomJs/RenderingResultStatus.cs new file mode 100644 index 0000000000..f735a61b97 --- /dev/null +++ b/Composite/Core/WebClient/PhantomJs/RenderingResultStatus.cs @@ -0,0 +1,13 @@ +namespace Composite.Core.WebClient.PhantomJs +{ + internal enum RenderingResultStatus + { + Success = 0, + Redirect = 1, + Error = 2, + Timeout = 3, + PhantomServerTimeout = 4, + PhantomServerIncorrectResponse = 5, + PhantomServerNoOutput = 6 + } +} diff --git a/Composite/Core/WebClient/Renderings/FunctionPreview.cs b/Composite/Core/WebClient/Renderings/FunctionPreview.cs index 651f364ba2..0f5a384f3f 100644 --- a/Composite/Core/WebClient/Renderings/FunctionPreview.cs +++ b/Composite/Core/WebClient/Renderings/FunctionPreview.cs @@ -1,7 +1,9 @@ -using System.Threading.Tasks; +using System; +using System.Threading.Tasks; using System.Web; using Composite.C1Console.Events; using Composite.Core.Configuration; +using Composite.Core.WebClient.PhantomJs; namespace Composite.Core.WebClient.Renderings { @@ -26,15 +28,17 @@ internal static async Task GetPreviewFunctionPreviewImageFile(HttpContex return null; } - if (renderingResult.Status == BrowserRender.RenderingResultStatus.Success) + if (renderingResult.Status == RenderingResultStatus.Success) { return renderingResult.FilePath; } - if (renderingResult.Status >= BrowserRender.RenderingResultStatus.Error) + if (renderingResult.Status >= RenderingResultStatus.Error) { - Log.LogWarning("FunctionPreview", "Failed to build preview for function. Reason: {0}; Output:\r\n{1}", - renderingResult.Status, renderingResult.Output); + string functionTitle = context.Request.QueryString["title"] ?? "null"; + + Log.LogWarning("FunctionPreview", "Failed to build preview for function '{0}'. Reason: {1}; Output:\r\n{2}", + functionTitle, renderingResult.Status, string.Join(Environment.NewLine, renderingResult.Output)); } return null; } diff --git a/Composite/Core/WebClient/Services/WysiwygEditor/PageTemplatePreview.cs b/Composite/Core/WebClient/Services/WysiwygEditor/PageTemplatePreview.cs index 245d917583..f372ef422c 100644 --- a/Composite/Core/WebClient/Services/WysiwygEditor/PageTemplatePreview.cs +++ b/Composite/Core/WebClient/Services/WysiwygEditor/PageTemplatePreview.cs @@ -1,9 +1,11 @@ using System; using System.Collections.Generic; using System.Drawing; +using System.Linq; using System.Web; using Composite.C1Console.Events; using System.Threading.Tasks; +using Composite.Core.WebClient.PhantomJs; namespace Composite.Core.WebClient.Services.WysiwygEditor { @@ -39,7 +41,7 @@ public static bool GetPreviewInformation(HttpContext context, Guid pageId, Guid string requestUrl = new UrlBuilder(context.Request.Url.ToString()).ServerUrl + ServiceUrl + $"?p={pageId}&t={templateId}&hash={updateHash}"; - BrowserRender.RenderingResult result = null; + RenderingResult result = null; var renderTask = BrowserRender.RenderUrlAsync(context, requestUrl, RenderingMode); renderTask.Wait(10000); @@ -55,7 +57,7 @@ public static bool GetPreviewInformation(HttpContext context, Guid pageId, Guid return false; } - if (result.Status != BrowserRender.RenderingResultStatus.Success) + if (result.Status != RenderingResultStatus.Success) { Log.LogWarning("PageTemplatePreview", "Failed to build preview for page template '{0}'. Reason: {1}; Output:\r\n{2}", templateId, result.Status, result.Output); @@ -66,15 +68,17 @@ public static bool GetPreviewInformation(HttpContext context, Guid pageId, Guid } imageFilePath = result.FilePath; - string output = result.Output; + ICollection output = result.Output; + const string templateInfoPrefix = "templateInfo:"; - var pList = new List(); + var placeholderData = output.FirstOrDefault(l => l.StartsWith(templateInfoPrefix)); - string templateInfoPrefix = "templateInfo:"; + var pList = new List(); - if (output.StartsWith(templateInfoPrefix)) + // TODO: use JSON + if (placeholderData != null) { - foreach (var infoPart in output.Substring(templateInfoPrefix.Length).Split('|')) + foreach (var infoPart in placeholderData.Substring(templateInfoPrefix.Length).Split('|')) { string[] parts = infoPart.Split(','); diff --git a/Website/App_Data/Composite/PhantomJs/renderingServer.js b/Website/App_Data/Composite/PhantomJs/renderingServer.js index f5cb0501a9..9d6cfdf2a2 100644 --- a/Website/App_Data/Composite/PhantomJs/renderingServer.js +++ b/Website/App_Data/Composite/PhantomJs/renderingServer.js @@ -1,5 +1,25 @@ var page = require('webpage').create(); +function WriteSuccess(console) { + console.log("SUCCESS"); +} + +function WriteError(console, message) { + console.log("ERROR: " + message); +} + +function WriteTimeout(console, message) { + console.log("TIMEOUT: " + message); +} + +function WriteRedirect(console, location) { + console.log("REDIRECT: " + location); +} + +function WriteEndOfReply(console) { + console.log("END_OF_REPLY"); +} + function getPlaceholdersLocationInfo(placeholderElementName) { var ret = []; @@ -22,6 +42,13 @@ function BuildFunctionPreview(system, console, address, output, cookies, mode) { } }; + // Sends "END_OF_REQUESTS" message and runs the message loop + var endRequest = function() { + clearGlobalTimeout(); + WriteEndOfReply(console); + WaitForInput(system, console); + } + if (cookies != null) { var customCookieHeader = null; @@ -49,7 +76,7 @@ function BuildFunctionPreview(system, console, address, output, cookies, mode) { } } - if (mode == "test") { + if (mode === "test") { page.viewportSize = { width: 320, height: 200 }; page.clipRect = { top: 0, left: 0, height: 320, width: 200 }; } else { @@ -57,15 +84,20 @@ function BuildFunctionPreview(system, console, address, output, cookies, mode) { } page.settings.resourceTimeout = 30000; - - page.onResourceTimeout = function (request) { - if (request.id == 1) { - clearGlobalTimeout(); - var errorMessage = 'TIMEOUT: page.onResourceTimeout: ' + JSON.stringify(request.errorString) + ', URL: ' + JSON.stringify(request.url); + page.onUrlChanged = function (targetUrl) { + if (page.preview_url !== targetUrl) { + WriteRedirect(console, targetUrl); + + endRequest(); + } + }; + + page.onResourceTimeout = function (request) { + if (request.id === 1) { + WriteTimeout(console, "page.onResourceTimeout: " + JSON.stringify(request.errorString) + ", URL: " + JSON.stringify(request.url)); - console.log(errorMessage); - WaitForInput(system, console); + endRequest(); } }; @@ -75,37 +107,36 @@ function BuildFunctionPreview(system, console, address, output, cookies, mode) { } page.onResourceError = function (resourceError) { - page.fail_reason = "Error opening url '" + resourceError.url + "'; Error code: " + resourceError.errorCode + ". Description: " + resourceError.errorString; + console.log("page.onResourceError: url '" + resourceError.url + "'; Error code: " + resourceError.errorCode + ". Description: " + resourceError.errorString); }; - // redirects ... page.onResourceReceived = function (response) { - if (response.id == 1) { + console.log("Resource received. Id: " + response.id + " status = " + response.status + " url = " + request.url); + if (response.id === 1) { var closePage = false; - if (response.status == 301 || response.status == 302) { - console.log('REDIRECT: ' + response.url); + if (response.status === 301 || response.status === 302) { + WriteRedirect(console, response.url); closePage = true; } if (response.status >= 400) { - var description = 'HTTP Status-Code ' + response.status + '.'; + var description = "HTTP Status-Code " + response.status + "."; - if (response.status == 500) { - description = '500 Internal Server Error.'; + if (response.status === 500) { + description = "500 Internal Server Error."; } - else if (response.status == 503) { - description = '503 Service Unavailable.'; + else if (response.status === 503) { + description = "503 Service Unavailable."; } - console.log('ERROR: ' + description); + WriteError(console, description); closePage = true; } if (closePage) { - clearGlobalTimeout(); - WaitForInput(system, console); + endRequest(); } } } @@ -114,7 +145,7 @@ function BuildFunctionPreview(system, console, address, output, cookies, mode) { page.onCallback = function (data) { clearGlobalTimeout(); - if (mode == "function") { + if (mode === "function") { var previewElementId = "CompositeC1FunctionPreview"; var clientRect = page.evaluate("getFunctionPreviewClientRect", previewElementId); @@ -127,58 +158,58 @@ function BuildFunctionPreview(system, console, address, output, cookies, mode) { } page.clipRect = clientRect; } else { + console.log("warn: clientRect is empty, redering a 1x1 image. " + JSON.stringify(clientRect)); + // Rendering an empty spot page.clipRect = { top: 0, left: 0, height: 1, width: 1 }; } page.render(output); - console.log('SUCCESS: ' + address); - } else if (mode == "template") { + WriteSuccess(console); + } else if (mode === "template") { // Template preview: - var placeholdersInfo = page.evaluate(getPlaceholdersLocationInfo, 'placeholderpreview'); + var placeholdersInfo = page.evaluate(getPlaceholdersLocationInfo, "placeholderpreview"); page.render(output); - console.log('templateInfo:' + placeholdersInfo); + console.log('templateInfo:' + placeholdersInfo); // TODO: pass placeholders info as JSON + WriteSuccess(console); } else { page.render(output); - console.log('SUCCESS'); + WriteSuccess(console); } - WaitForInput(system, console); + endRequest(); }; - try { + try { + console.log("Opening url: " + address); + + page.preview_url = address; page.open(address, function (status) { - if (status !== 'success') { - clearGlobalTimeout(); - console.log('ERROR, page.open: ' + status - + "; " + page.fail_reason); + if (status !== "success") { + WriteError("page.open(), status=" + status); - WaitForInput(system, console); + endRequest(); } else { if (mode === "test") { - clearGlobalTimeout(); + page.render(output); - page.render(output); + WriteSuccess(console); - console.log('SUCCESS'); - - WaitForInput(system, console); + endRequest(); } else { var previewJsExecuted = page.evaluate(function () { - return window.previewJsInitialized == true; + return window.previewJsInitialized === true; }); // If "preview.js" isn't inserted, closing the page, as the default callback will not be called if (!previewJsExecuted) { - clearGlobalTimeout(); - - console.log('ERROR: preview.js script is not present in the response body'); + WriteError(console, "preview.js script is not present in the response body"); - WaitForInput(system, console); + endRequest(); } } } @@ -187,9 +218,10 @@ function BuildFunctionPreview(system, console, address, output, cookies, mode) { var timeoutInSeconds = 60; globalTimeout = setTimeout(function () { - console.log("TIMEOUT: Max execution time - " + timeoutInSeconds + " seconds - exceeded"); - globalTimeout = null; - WaitForInput(system, console); + globalTimeout = null; + WriteTimeout(console, "Max execution time - " + timeoutInSeconds + " seconds - exceeded"); + + endRequest(); }, timeoutInSeconds * 1000); } } @@ -197,55 +229,48 @@ function BuildFunctionPreview(system, console, address, output, cookies, mode) { var system = require('system'); function WaitForInput(system, console) { - while(true) { - var line = system.stdin.readLine(); - if(line == "exit") - { + while (true) { + var line = system.stdin.readLine(); + if (line === "exit") { phantom.exit(0); return; - } - - var parameters = line.split(";"); - if(parameters.length == 4) { - - var cookieInfo = parameters[0]; - var url = parameters[1]; - var outputFilePath = parameters[2]; - var mode = parameters[3]; - - var cookies = null; - - if(cookieInfo !== "") { - var cookieInfoParts = cookieInfo.split(","); - - if(cookieInfoParts.length % 3 !== 0) { - console.log('Invalid cookie information, correct format is {name},{value},{domain}[,{name},{value},{domain}]*'); - continue; - } - - cookies = []; - for (var i = 0; i < cookieInfoParts.length / 3; i++) { - var cookie = { - 'name': cookieInfoParts[i*3], - 'value': cookieInfoParts[i * 3 + 1], - 'domain': cookieInfoParts[i * 3 + 2], - 'path': '/', - 'httponly': true, - 'secure': false, - 'expires': (new Date()).getTime() + (1000 * 60 * 60) - }; - - cookies.push(cookie); + } + + var request = JSON.parse(line); + if (request.mode) { + + var mode = request.mode; + var url = request.url; + var outputFilePath = request.outputFilePath; + var cookieInfo = request.cookies; + + var cookies = null; + + if (cookieInfo != null) { + + cookies = []; + for (var i = 0; i < cookieInfo.length; i++) { + var cookie = { + 'name': cookieInfo[i].name, + 'value': cookieInfo[i].value, + 'domain': cookieInfo[i].value, + 'path': '/', + 'httponly': true, + 'secure': false, + 'expires': (new Date()).getTime() + (1000 * 60 * 60) + }; + + cookies.push(cookie); + } } - } - - BuildFunctionPreview(system, console, url, outputFilePath, cookies, mode); - return; - } - else { - console.log('Usage: {Authentication cookie information};{url};{out put file name};{mode}. Where {Authentication cookie information} = {name},{value},{domain}[,{name},{value},{domain}]*'); - } - } + + BuildFunctionPreview(system, console, url, outputFilePath, cookies, mode); + return; + } + else { + console.log('Usage: {mode: "...", url: "...", ...}'); + } + } } WaitForInput(system, console); \ No newline at end of file From 52cce75e890a6d2a0450cdbcb398b0f626822e1e Mon Sep 17 00:00:00 2001 From: Dmitry Dzygin Date: Fri, 4 Nov 2016 13:40:54 +0100 Subject: [PATCH 2/6] Cleaning up composite.config --- Website/App_Data/Composite/DebugBuild.Composite.config | 1 - Website/App_Data/Composite/ReleaseBuild.Composite.config | 1 - 2 files changed, 2 deletions(-) diff --git a/Website/App_Data/Composite/DebugBuild.Composite.config b/Website/App_Data/Composite/DebugBuild.Composite.config index b84a94f52c..9cc1a14ae1 100644 --- a/Website/App_Data/Composite/DebugBuild.Composite.config +++ b/Website/App_Data/Composite/DebugBuild.Composite.config @@ -472,7 +472,6 @@ - diff --git a/Website/App_Data/Composite/ReleaseBuild.Composite.config b/Website/App_Data/Composite/ReleaseBuild.Composite.config index 53f95cc9c6..1ea4976136 100644 --- a/Website/App_Data/Composite/ReleaseBuild.Composite.config +++ b/Website/App_Data/Composite/ReleaseBuild.Composite.config @@ -466,7 +466,6 @@ - From 89bf03405bfd5a6db8e965e6512e7556ed4ca868 Mon Sep 17 00:00:00 2001 From: Dmitry Dzygin Date: Tue, 8 Nov 2016 15:07:50 +0100 Subject: [PATCH 3/6] Fix #294 multiple copies of the same dll loaded in memory during package installation causes problems when installing static data types + their data --- .../PackageSystem/PackageAssemblyHandler.cs | 76 ++++++------- .../FilePackageFragmentInstaller.cs | 42 ++++--- Composite/Core/Types/CodeGenerationManager.cs | 107 ++++++++---------- .../Foundation/AssemblyFilenameCollection.cs | 7 +- Composite/Data/DataMetaDataFacade.cs | 48 ++++++-- .../Foundation/EmptyDataClassTypeManager.cs | 4 +- 6 files changed, 157 insertions(+), 127 deletions(-) diff --git a/Composite/Core/PackageSystem/PackageAssemblyHandler.cs b/Composite/Core/PackageSystem/PackageAssemblyHandler.cs index 1250445527..96f08e2106 100644 --- a/Composite/Core/PackageSystem/PackageAssemblyHandler.cs +++ b/Composite/Core/PackageSystem/PackageAssemblyHandler.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Text; using System.Reflection; using Composite.C1Console.Events; using Composite.Core.Types.Foundation; @@ -11,29 +10,36 @@ namespace Composite.Core.PackageSystem { internal static class PackageAssemblyHandler { - private static bool _initialized = false; + private static bool _initialized; private static readonly object _lock = new object(); private static AssemblyFilenameCollection _loadedAssemblyFilenames = new AssemblyFilenameCollection(); - + private static List _inMemoryAssemblies = new List(); public static void Initialize() { - if (!_initialized) + if (_initialized) return; + + lock (_lock) { - lock (_lock) - { - if (!_initialized) - { - GlobalEventSystemFacade.SubscribeToFlushEvent(OnFlushEvent); + if (_initialized) return; - AppDomain.CurrentDomain.AssemblyResolve += OnAssemblyResolve; + GlobalEventSystemFacade.SubscribeToFlushEvent(args => ClearAssemblyList()); - _initialized = true; - } - } + AppDomain.CurrentDomain.AssemblyResolve += OnAssemblyResolve; + AppDomain.CurrentDomain.AssemblyLoad += OnAssemblyLoad; + + _initialized = true; } } + private static void OnAssemblyLoad(object sender, AssemblyLoadEventArgs args) + { + var asm = args.LoadedAssembly; + if (!asm.IsDynamic) + { + Log.LogVerbose(nameof(PackageAssemblyHandler), $"Assembly loaded: {asm.Location}"); + } + } public static void AddAssembly(string assemblyFilePath) @@ -47,24 +53,25 @@ public static void AddAssembly(string assemblyFilePath) } - - private static Assembly OnAssemblyResolve(object sender, ResolveEventArgs args) + public static Assembly TryGetAlreadyLoadedAssembly(string assemblyFileName) { - string filename = args.Name; + string assemblyName = AssemblyFilenameCollection.GetAssemblyName(assemblyFileName); - // Why can the system not load the "System.Web.Extensions" assembly? - // And "Composite.Core.XmlSerializers" <-- Licensing? - // For now ignore it, so no exception is thrown /MRJ - if ((filename == "System.Web.Extensions") || - (filename.StartsWith("Composite.Core.XmlSerializers"))) + lock (_lock) { - return null; + return _inMemoryAssemblies.FirstOrDefault(asm => asm.GetName().Name == assemblyName); } + } + + + private static Assembly OnAssemblyResolve(object sender, ResolveEventArgs args) + { + string filename = args.Name; string fn = filename; if (fn.Contains(",")) { - fn = fn.Remove(fn.IndexOf(",")).Trim(); + fn = fn.Remove(fn.IndexOf(',')).Trim(); } if (_loadedAssemblyFilenames.ContainsAssemblyName(fn)) @@ -77,11 +84,16 @@ private static Assembly OnAssemblyResolve(object sender, ResolveEventArgs args) { try { - assembly = Assembly.LoadFile(filename); + assembly = Assembly.LoadFrom(filename); } catch (Exception ex) { - Log.LogError("PackageAssemblyHandler", ex); + Log.LogError(nameof(PackageAssemblyHandler), ex); + } + + lock (_lock) + { + _inMemoryAssemblies.Add(assembly); } } @@ -90,24 +102,12 @@ private static Assembly OnAssemblyResolve(object sender, ResolveEventArgs args) public static void ClearAssemblyList() - { - Flush(); - } - - - private static void Flush() { lock (_lock) { _loadedAssemblyFilenames = new AssemblyFilenameCollection(); + _inMemoryAssemblies = new List(); } } - - - - private static void OnFlushEvent(FlushEventArgs args) - { - Flush(); - } } } diff --git a/Composite/Core/PackageSystem/PackageFragmentInstallers/FilePackageFragmentInstaller.cs b/Composite/Core/PackageSystem/PackageFragmentInstallers/FilePackageFragmentInstaller.cs index 1537b6222a..1fe8bfa6d3 100644 --- a/Composite/Core/PackageSystem/PackageFragmentInstallers/FilePackageFragmentInstaller.cs +++ b/Composite/Core/PackageSystem/PackageFragmentInstallers/FilePackageFragmentInstaller.cs @@ -347,24 +347,7 @@ public override IEnumerable Install() if (targetFilePath.StartsWith(Path.Combine(PathUtil.BaseDirectory, "Bin"), StringComparison.InvariantCultureIgnoreCase) && targetFilePath.EndsWith(".dll", StringComparison.InvariantCultureIgnoreCase)) { - string fileName = Path.GetFileName(targetFilePath); - - if (!DllsNotToLoad.Any(fileName.StartsWith)) - { - Assembly assembly; - - try - { - assembly = Assembly.LoadFrom(targetFilePath); - } - catch (Exception) - { - continue; - } - - DataTypeTypesManager.AddNewAssembly(assembly, false); - } - + LoadDataTypesFromDll(targetFilePath); } var fileElement = new XElement("File", new XAttribute("filename", fileToCopy.TargetRelativeFilePath)); @@ -380,6 +363,29 @@ public override IEnumerable Install() yield return new XElement("Files", fileElements); } + private void LoadDataTypesFromDll(string filePath) + { + string fileName = Path.GetFileName(filePath); + + if (DllsNotToLoad.Any(fileName.StartsWith)) return; + + var assembly = PackageAssemblyHandler.TryGetAlreadyLoadedAssembly(filePath); + + if(assembly == null) + { + try + { + assembly = Assembly.LoadFrom(filePath); + } + catch (Exception) + { + return; + } + } + + DataTypeTypesManager.AddNewAssembly(assembly, false); + } + private string GetBackupFileName(string targetFilePath) { string fileName = Path.GetFileName(targetFilePath); diff --git a/Composite/Core/Types/CodeGenerationManager.cs b/Composite/Core/Types/CodeGenerationManager.cs index 92e4eb3074..2e9b3552d9 100644 --- a/Composite/Core/Types/CodeGenerationManager.cs +++ b/Composite/Core/Types/CodeGenerationManager.cs @@ -5,13 +5,16 @@ using System.CodeDom; using System.CodeDom.Compiler; using System.Collections.Generic; +using System.Collections.Specialized; using System.IO; using System.Linq; using System.Reflection; using System.Text; using Composite.C1Console.Events; using Composite.Core.Configuration; +using Composite.Core.Extensions; using Composite.Core.IO; +using Composite.Core.PackageSystem; using Composite.Data.Foundation.CodeGeneration; using Composite.Data.GeneratedTypes; using Microsoft.CSharp; @@ -178,8 +181,8 @@ public static IEnumerable CompileRuntimeTempTypes(CodeGenerationBuilder co int t2 = Environment.TickCount; - Log.LogVerbose(LogTitle, string.Format("Compile '{0}' in {1}ms", codeGenerationBuilder.DebugLabel, t2 - t1)); - Log.LogVerbose(LogTitle, string.Format("Types from : {0}", compilerParameters.OutputAssembly)); + Log.LogVerbose(LogTitle, $"Compile '{codeGenerationBuilder.DebugLabel}' in {t2 - t1}ms"); + Log.LogVerbose(LogTitle, $"Types from : {compilerParameters.OutputAssembly}"); foreach (Type resultType in resultTypes) { @@ -199,31 +202,9 @@ public static IEnumerable CompileRuntimeTempTypes(CodeGenerationBuilder co } } #endif + - var failedAssemblyLoads = new List>(); - foreach (string assemblyLocation in compilerParameters.ReferencedAssemblies) - { - try - { - Assembly assembly = Assembly.LoadFrom(assemblyLocation); - var types = assembly.GetTypes(); // Accessing GetTypes() to iterate classes - } - catch (Exception ex) - { - Exception exceptionToLog = ex; - - var loadException = ex as ReflectionTypeLoadException; - if (loadException != null - && loadException.LoaderExceptions != null - && loadException.LoaderExceptions.Any()) - { - exceptionToLog = loadException.LoaderExceptions.First(); - } - - failedAssemblyLoads.Add(new Pair( assemblyLocation, exceptionToLog)); - } - } - + var failedAssemblyLoads = LoadAssembliesToMemory(compilerParameters.ReferencedAssemblies); var sb = new StringBuilder(); failedAssemblyLoads.ForEach(asm => sb.AppendFormat("Failed to load dll: '{0}' : {1}", asm.First, asm.Second).AppendLine()); @@ -247,10 +228,44 @@ public static IEnumerable CompileRuntimeTempTypes(CodeGenerationBuilder co } + private static ICollection> LoadAssembliesToMemory(StringCollection assemblyLocations) + { + var failedAssemblyLoads = new List>(); + foreach (string assemblyLocation in assemblyLocations) + { + if (PackageAssemblyHandler.TryGetAlreadyLoadedAssembly(assemblyLocation) != null) + { + continue; + } + + try + { + var assembly = Assembly.LoadFrom(assemblyLocation); + assembly.GetTypes(); // Accessing GetTypes() to iterate classes + } + catch (Exception ex) + { + Exception exceptionToLog = ex; + + var loadException = ex as ReflectionTypeLoadException; + if (loadException != null + && loadException.LoaderExceptions != null + && loadException.LoaderExceptions.Any()) + { + exceptionToLog = loadException.LoaderExceptions.First(); + } + + failedAssemblyLoads.Add(new Pair(assemblyLocation, exceptionToLog)); + } + } + + return failedAssemblyLoads; + } + /// /// This method returns true if the given type is - /// compiled at runetime. Otherwice false. + /// compiled at runetime. Otherwise false. /// /// /// @@ -285,7 +300,7 @@ public static bool IsRecompileNeeded(Type dependableType, IEnumerable depe /// - /// Use this method to add a impelementation + /// Use this method to add a implementation /// that will be used when (and only) generating the final Composite.Generated.dll. /// /// @@ -365,26 +380,12 @@ private static void Compile(CodeGenerationBuilder builder) /// /// Returns all currently temp compiled assemblies. /// - internal static IEnumerable CompiledAssemblies - { - get - { - return _compiledAssemblies; - } - } - + internal static IEnumerable CompiledAssemblies => _compiledAssemblies; /// /// - internal static string TempAssemblyFolderPath - { - get - { - return PathUtil.Resolve(GlobalSettingsFacade.GeneratedAssembliesDirectory); - } - } - + internal static string TempAssemblyFolderPath => PathUtil.Resolve(GlobalSettingsFacade.GeneratedAssembliesDirectory); /// @@ -403,26 +404,12 @@ internal static string BinFolder /// /// - internal static string CompositeGeneratedFileName - { - get - { - return "Composite.Generated.dll"; - } - } - + internal static string CompositeGeneratedFileName => "Composite.Generated.dll"; /// /// - internal static string CompositeGeneratedAssemblyPath - { - get - { - return Path.Combine(BinFolder, "Composite.Generated.dll"); - } - } - + internal static string CompositeGeneratedAssemblyPath => Path.Combine(BinFolder, "Composite.Generated.dll"); private static void PopulateBuilder(CodeGenerationBuilder builder) diff --git a/Composite/Core/Types/Foundation/AssemblyFilenameCollection.cs b/Composite/Core/Types/Foundation/AssemblyFilenameCollection.cs index 369bd4caff..a5d4ea7285 100644 --- a/Composite/Core/Types/Foundation/AssemblyFilenameCollection.cs +++ b/Composite/Core/Types/Foundation/AssemblyFilenameCollection.cs @@ -7,7 +7,7 @@ namespace Composite.Core.Types.Foundation { internal sealed class AssemblyFilenameCollection { - private Dictionary _assemblyFilenames = new Dictionary(); + private readonly Dictionary _assemblyFilenames = new Dictionary(); @@ -47,7 +47,10 @@ public string GetFilenameByAssemblyName(string assemblyName) if (string.IsNullOrEmpty(assemblyName)) throw new ArgumentNullException("assemblyName"); string assemblyFilename; - if (_assemblyFilenames.TryGetValue(assemblyName, out assemblyFilename) == false) throw new ArgumentException(string.Format("Does not contain the assembly name '{0}'", assemblyName)); + if (!_assemblyFilenames.TryGetValue(assemblyName, out assemblyFilename)) + { + throw new ArgumentException($"Does not contain the assembly name '{assemblyName}'"); + } return assemblyFilename; } diff --git a/Composite/Data/DataMetaDataFacade.cs b/Composite/Data/DataMetaDataFacade.cs index d3b645ec26..4a8c18bdd5 100644 --- a/Composite/Data/DataMetaDataFacade.cs +++ b/Composite/Data/DataMetaDataFacade.cs @@ -172,18 +172,17 @@ public static IEnumerable GeneratedTypeDataTypeDescriptors /// /// This method will return the data type descriptor for the given data type id. /// If the data type descriptor has not yet been created (file not existing) and - /// the is set to true, + /// the is set to true, /// this method will try getting it through the - /// that will try locating the type from the data type id using refelction + /// that will try locating the type from the data type id using refelection /// going through know assemblies. /// /// The id of the data type. - /// - /// If this is true and the data type descriptor does not exists, it will try to - /// be created. + /// + /// If this is true and the data type descriptor does not exists, the method will try to create it. /// /// - public static DataTypeDescriptor GetDataTypeDescriptor(Guid dataTypeId, bool allowDataTypeCreation = false) + public static DataTypeDescriptor GetDataTypeDescriptor(Guid dataTypeId, bool allowTypeMetaDataCreation = false) { Initialize(); @@ -194,7 +193,7 @@ public static DataTypeDescriptor GetDataTypeDescriptor(Guid dataTypeId, bool all if (dataTypeDescriptor != null) return dataTypeDescriptor; - if (!allowDataTypeCreation) return null; + if (!allowTypeMetaDataCreation) return null; foreach (Assembly assembly in AssemblyFacade.GetLoadedAssembliesFromBin()) { @@ -237,6 +236,41 @@ public static DataTypeDescriptor GetDataTypeDescriptor(Guid dataTypeId, bool all } + /// + /// This method will return the data type descriptor for the given data type id. + /// If the data type descriptor has not yet been created (file not existing) and + /// the is set to true, + /// this method will try getting it through the + /// based on . + /// + /// The data type. + /// + /// If this is true and the data type descriptor does not exists, it will be created. + /// + /// + public static DataTypeDescriptor GetDataTypeDescriptor(Type interfaceType, bool allowTypeMetaDataCreation = false) + { + Verify.ArgumentNotNull(interfaceType, nameof(interfaceType)); + + Initialize(); + + DataTypeDescriptor dataTypeDescriptor; + + Guid dataTypeId = interfaceType.GetImmutableTypeId(); + + _dataTypeDescriptorCache.TryGetValue(dataTypeId, out dataTypeDescriptor); + + if (dataTypeDescriptor != null) return dataTypeDescriptor; + + if (!allowTypeMetaDataCreation) return null; + + var newDataTypeDescriptor = ReflectionBasedDescriptorBuilder.Build(interfaceType); + PersistMetaData(newDataTypeDescriptor); + + return newDataTypeDescriptor; + } + + /// public static void PersistMetaData(DataTypeDescriptor dataTypeDescriptor) diff --git a/Composite/Data/Foundation/EmptyDataClassTypeManager.cs b/Composite/Data/Foundation/EmptyDataClassTypeManager.cs index d1fa8a388f..b6f5b0aca9 100644 --- a/Composite/Data/Foundation/EmptyDataClassTypeManager.cs +++ b/Composite/Data/Foundation/EmptyDataClassTypeManager.cs @@ -43,7 +43,7 @@ public static Type GetEmptyDataClassType(Type interfaceType, bool forceReCompila { VerifyAssemblyLocation(interfaceType); - var dataTypeDescriptor = DataMetaDataFacade.GetDataTypeDescriptor(interfaceType.GetImmutableTypeId(), true); + var dataTypeDescriptor = DataMetaDataFacade.GetDataTypeDescriptor(interfaceType, true); return GetEmptyDataClassType(dataTypeDescriptor, forceReCompilation); } @@ -78,7 +78,7 @@ public static Type GetEmptyDataClassType(DataTypeDescriptor dataTypeDescriptor, return CreateEmptyDataClassType(dataTypeDescriptor); } - Type interfaceType = TypeManager.TryGetType(dataTypeDescriptor.GetFullInterfaceName()); + Type interfaceType = TypeManager.TryGetType(dataTypeDescriptor.TypeManagerTypeName); string emptyClassFullName = EmptyDataClassCodeGenerator.GetEmptyClassTypeFullName(dataTypeDescriptor); Type emptyClassType = TypeManager.TryGetType(emptyClassFullName); From ef545ddb60913b898add5c3d586612837c794bd8 Mon Sep 17 00:00:00 2001 From: Dmitry Dzygin Date: Tue, 8 Nov 2016 17:01:09 +0100 Subject: [PATCH 4/6] Setup time optimization - avoiding unnecessary Composite.Generated.dll recompilations; refactoring --- Composite/Core/Types/CodeGenerationManager.cs | 12 +++++----- .../Data/DynamicTypes/DynamicTypeManager.cs | 22 +++++++++++++++---- .../DynamicTypes/DynamicTypeManagerImpl.cs | 16 +++++++++----- .../Data/DynamicTypes/IDynamicTypeManager.cs | 2 +- 4 files changed, 34 insertions(+), 18 deletions(-) diff --git a/Composite/Core/Types/CodeGenerationManager.cs b/Composite/Core/Types/CodeGenerationManager.cs index 2e9b3552d9..ce654f61fc 100644 --- a/Composite/Core/Types/CodeGenerationManager.cs +++ b/Composite/Core/Types/CodeGenerationManager.cs @@ -88,7 +88,7 @@ internal static void ValidateCompositeGenerate(DateTime time) if (lastWrite <= time) return; _compositeGeneratedCompiled = true; - Log.LogVerbose(LogTitle, string.Format("Assembly in this application domain is newer than this application domain ({0})", AppDomain.CurrentDomain.Id)); + Log.LogVerbose(LogTitle, $"Assembly in this application domain is newer than this application domain ({AppDomain.CurrentDomain.Id})"); } @@ -107,7 +107,7 @@ public static void GenerateCompositeGeneratedAssembly(bool forceGeneration = fal { if (forceGeneration || !_compositeGeneratedCompiled) { - Log.LogVerbose(LogTitle, string.Format("Compiling new assembly in this application domain ({0})", AppDomain.CurrentDomain.Id)); + Log.LogVerbose(LogTitle, $"Compiling new assembly in this application domain ({AppDomain.CurrentDomain.Id})"); int t1 = Environment.TickCount; @@ -248,9 +248,7 @@ private static ICollection> LoadAssembliesToMemory(Strin Exception exceptionToLog = ex; var loadException = ex as ReflectionTypeLoadException; - if (loadException != null - && loadException.LoaderExceptions != null - && loadException.LoaderExceptions.Any()) + if (loadException?.LoaderExceptions != null && loadException.LoaderExceptions.Any()) { exceptionToLog = loadException.LoaderExceptions.First(); } @@ -470,8 +468,8 @@ private static void Flush() private static void ClearOldTempFiles() { - DateTime yeasterday = DateTime.Now.AddDays(-1); - var oldFiles = C1Directory.GetFiles(TempAssemblyFolderPath, "*.*").Where(filePath => C1File.GetCreationTime(filePath) < yeasterday).ToArray(); + DateTime yesterday = DateTime.Now.AddDays(-1); + var oldFiles = C1Directory.GetFiles(TempAssemblyFolderPath, "*.*").Where(filePath => C1File.GetCreationTime(filePath) < yesterday).ToArray(); foreach (var file in oldFiles) { diff --git a/Composite/Data/DynamicTypes/DynamicTypeManager.cs b/Composite/Data/DynamicTypes/DynamicTypeManager.cs index 5f21cd70cc..5c6425eaa5 100644 --- a/Composite/Data/DynamicTypes/DynamicTypeManager.cs +++ b/Composite/Data/DynamicTypes/DynamicTypeManager.cs @@ -4,11 +4,13 @@ using System.Globalization; using System.Linq; using Composite.Core; +using Composite.Core.Configuration; using Composite.Core.Instrumentation; using Composite.Data.Foundation; using Composite.Data.Foundation.PluginFacades; using Composite.Data.Plugins.DataProvider; using Composite.Core.Types; +using Composite.Core.WebClient.Setup; namespace Composite.Data.DynamicTypes @@ -272,18 +274,30 @@ public static void EnsureCreateStore(Type interfaceType, string providerName) dynamicProviderNames = new[] {providerName}; } - if (dynamicProviderNames + var possibleMatches = dynamicProviderNames .Select(DataProviderPluginFacade.GetDataProvider) .Cast() - .Any(dynamicDataProvider => dynamicDataProvider.GetKnownInterfaces().Contains(interfaceType))) + .SelectMany(dynamicDataProvider => dynamicDataProvider.GetKnownInterfaces()) + .Where(i => i.FullName == interfaceType.FullName); + + foreach(var match in possibleMatches) { - return; + if(match == interfaceType) return; + + if (match.GetImmutableTypeId() == interfaceType.GetImmutableTypeId()) + { + throw new InvalidOperationException($"The same type '{match.FullName}' is loaded in memory twice. Location 1: '{match.Assembly.Location}', location 2: {interfaceType.Assembly.Location}"); + } } var dataTypeDescriptor = BuildNewDataTypeDescriptor(interfaceType); CreateStore(providerName, dataTypeDescriptor, true); - CodeGenerationManager.GenerateCompositeGeneratedAssembly(true); + + if (!SystemSetupFacade.SetupIsRunning) + { + CodeGenerationManager.GenerateCompositeGeneratedAssembly(true); + } } diff --git a/Composite/Data/DynamicTypes/DynamicTypeManagerImpl.cs b/Composite/Data/DynamicTypes/DynamicTypeManagerImpl.cs index d6cf2a6399..2a7e6b2123 100644 --- a/Composite/Data/DynamicTypes/DynamicTypeManagerImpl.cs +++ b/Composite/Data/DynamicTypes/DynamicTypeManagerImpl.cs @@ -5,6 +5,7 @@ using Composite.Data.DynamicTypes.Foundation; using Composite.Data.Foundation.PluginFacades; using Composite.C1Console.Events; +using Composite.Core.Configuration; using Composite.Data.Transactions; using Composite.Core.Types; @@ -28,9 +29,9 @@ public DataTypeDescriptor BuildNewDataTypeDescriptor(Type typeToDescript) /// - public bool TryGetDataTypeDescriptor(Guid immuteableTypeId, out DataTypeDescriptor dataTypeDescriptor) + public bool TryGetDataTypeDescriptor(Guid immutableTypeId, out DataTypeDescriptor dataTypeDescriptor) { - dataTypeDescriptor = DataMetaDataFacade.GetDataTypeDescriptor(immuteableTypeId); + dataTypeDescriptor = DataMetaDataFacade.GetDataTypeDescriptor(immutableTypeId); return dataTypeDescriptor != null; } @@ -126,8 +127,8 @@ public void DropStore(string providerName, DataTypeDescriptor typeDescriptor, bo /// public void AddLocale(string providerName, CultureInfo cultureInfo) { - if (string.IsNullOrEmpty(providerName)) throw new ArgumentNullException("providerName"); - if (cultureInfo == null) throw new ArgumentNullException("cultureInfo"); + Verify.ArgumentNotNullOrEmpty(providerName, nameof(providerName)); + Verify.ArgumentNotNull(cultureInfo, nameof(cultureInfo)); using (var transactionScope = TransactionsFacade.CreateNewScope()) { @@ -135,7 +136,10 @@ public void AddLocale(string providerName, CultureInfo cultureInfo) transactionScope.Complete(); } - CodeGenerationManager.GenerateCompositeGeneratedAssembly(true); + if (!SystemSetupFacade.SetupIsRunning) + { + CodeGenerationManager.GenerateCompositeGeneratedAssembly(true); + } } @@ -152,7 +156,7 @@ public void RemoveLocale(string providerName, CultureInfo cultureInfo) transactionScope.Complete(); } - CodeGenerationManager.GenerateCompositeGeneratedAssembly(true); + CodeGenerationManager.GenerateCompositeGeneratedAssembly(true); } } } diff --git a/Composite/Data/DynamicTypes/IDynamicTypeManager.cs b/Composite/Data/DynamicTypes/IDynamicTypeManager.cs index a90b999858..ae6e9a52c3 100644 --- a/Composite/Data/DynamicTypes/IDynamicTypeManager.cs +++ b/Composite/Data/DynamicTypes/IDynamicTypeManager.cs @@ -15,7 +15,7 @@ public interface IDynamicTypeManager DataTypeDescriptor BuildNewDataTypeDescriptor(Type typeToDescript); /// - bool TryGetDataTypeDescriptor(Guid immuteableTypeId, out DataTypeDescriptor dataTypeDescriptor); + bool TryGetDataTypeDescriptor(Guid immutableTypeId, out DataTypeDescriptor dataTypeDescriptor); /// void UpdateDataTypeDescriptor(DataTypeDescriptor dataTypeDescriptor, bool flushTheSystem); From b0e8dd9d1b5212c899800ba5de51c8a09d2b45e3 Mon Sep 17 00:00:00 2001 From: Marcus Wendt Date: Wed, 9 Nov 2016 14:19:50 +0100 Subject: [PATCH 5/6] Fix #297 - making allowOverwrite="true" work also for data not already in the system. --- .../PackageFragmentInstallers/DataPackageFragmentInstaller.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Composite/Core/PackageSystem/PackageFragmentInstallers/DataPackageFragmentInstaller.cs b/Composite/Core/PackageSystem/PackageFragmentInstallers/DataPackageFragmentInstaller.cs index 27cb5566ad..dae6c68b27 100644 --- a/Composite/Core/PackageSystem/PackageFragmentInstallers/DataPackageFragmentInstaller.cs +++ b/Composite/Core/PackageSystem/PackageFragmentInstallers/DataPackageFragmentInstaller.cs @@ -183,7 +183,7 @@ private XElement AddData(DataType dataType, CultureInfo cultureInfo) { IData existingData = DataFacade.TryGetDataByUniqueKey(interfaceType, dataKey); - if (data != null) + if (existingData != null) { CopyFieldValues(dataType, existingData, addElement); DataFacade.Update(existingData, false, true, false); From 7b5d39e9879545da9764429bbaadf148499baa7a Mon Sep 17 00:00:00 2001 From: Marcus Wendt Date: Mon, 14 Nov 2016 17:26:45 +0100 Subject: [PATCH 6/6] Updating version moniker to Orckestra CMS 5.5 Just fixes and optimizations in this one. --- Composite/Properties/SharedAssemblyInfo.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Composite/Properties/SharedAssemblyInfo.cs b/Composite/Properties/SharedAssemblyInfo.cs index 0cf261b1b3..632049aea9 100644 --- a/Composite/Properties/SharedAssemblyInfo.cs +++ b/Composite/Properties/SharedAssemblyInfo.cs @@ -2,15 +2,15 @@ // General Information about the assemblies Composite and Composite.Workflows #if !InternalBuild -[assembly: AssemblyTitle("Orckestra CMS 5.4")] +[assembly: AssemblyTitle("Orckestra CMS 5.5")] #else -[assembly: AssemblyTitle("Orckestra CMS 5.4 (Internal Build)")] +[assembly: AssemblyTitle("Orckestra CMS 5.5 (Internal Build)")] #endif [assembly: AssemblyCompany("Orckestra Inc")] [assembly: AssemblyProduct("Orckestra CMS")] -[assembly: AssemblyCopyright("Copyright © Orckestra Inc 2016")] +[assembly: AssemblyCopyright("Copyright © Orckestra Inc 2016")] [assembly: AssemblyTrademark("")] [assembly: AssemblyCulture("")] -[assembly: AssemblyVersion("5.4.*")] \ No newline at end of file +[assembly: AssemblyVersion("5.5.*")]