From 9cebfab2f48dfe3f8cc185d51f5cc7045718bb44 Mon Sep 17 00:00:00 2001 From: "kay.one" Date: Wed, 29 Feb 2012 21:10:56 -0800 Subject: [PATCH] Finished Exception Controller --- .../Contract/ExceptionReportResponse.cs | 4 +- .../Contract/ExistingExceptionReport.cs | 26 +++++ NzbDrone.Common/NzbDrone.Common.csproj | 1 + .../App_Start/Logging.cs | 2 +- .../Controllers/ExceptionController.cs | 64 +++++++++--- .../Migrations/Migration20120229.cs | 11 ++- .../Migrations/MigrationHelper.cs | 2 +- .../Repository/Reporting/ExceptionDetail.cs | 5 +- .../Repository/Reporting/ExceptionInstance.cs | 2 +- .../ReportExistingFixture.cs | 66 +++++++++++++ .../ReportNewFixture.cs} | 97 +++++++++++++++++-- .../Framework/ServicesTestBase.cs | 7 ++ .../NzbDrone.Services.Tests.csproj | 6 +- .../NzbDrone.Services.Tests/packages.config | 1 + 14 files changed, 255 insertions(+), 39 deletions(-) create mode 100644 NzbDrone.Common/Contract/ExistingExceptionReport.cs create mode 100644 NzbDrone.Services/NzbDrone.Services.Tests/ExceptionController/ReportExistingFixture.cs rename NzbDrone.Services/NzbDrone.Services.Tests/{ExceptionControllerFixture.cs => ExceptionController/ReportNewFixture.cs} (73%) diff --git a/NzbDrone.Common/Contract/ExceptionReportResponse.cs b/NzbDrone.Common/Contract/ExceptionReportResponse.cs index 0b6d05375..8aa98e8e8 100644 --- a/NzbDrone.Common/Contract/ExceptionReportResponse.cs +++ b/NzbDrone.Common/Contract/ExceptionReportResponse.cs @@ -5,7 +5,7 @@ namespace NzbDrone.Common.Contract { public class ExceptionReportResponse { - [JsonProperty("id")] - public int ExceptionId { get; set; } + [JsonProperty("h")] + public string ExceptionHash { get; set; } } } \ No newline at end of file diff --git a/NzbDrone.Common/Contract/ExistingExceptionReport.cs b/NzbDrone.Common/Contract/ExistingExceptionReport.cs new file mode 100644 index 000000000..b66ad1ccb --- /dev/null +++ b/NzbDrone.Common/Contract/ExistingExceptionReport.cs @@ -0,0 +1,26 @@ +using System.Collections.Generic; +using System.Linq; +using Newtonsoft.Json; + +namespace NzbDrone.Common.Contract +{ + public class ExistingExceptionReport : ReportBase + { + + [JsonProperty("h")] + public string Hash { get; set; } + + [JsonProperty("lm")] + public string LogMessage { get; set; } + + protected override Dictionary GetString() + { + var dic = new Dictionary + { + {"Message", LogMessage.NullSafe()} + }; + + return dic; + } + } +} \ No newline at end of file diff --git a/NzbDrone.Common/NzbDrone.Common.csproj b/NzbDrone.Common/NzbDrone.Common.csproj index 9ddbfb235..9426c50aa 100644 --- a/NzbDrone.Common/NzbDrone.Common.csproj +++ b/NzbDrone.Common/NzbDrone.Common.csproj @@ -55,6 +55,7 @@ + diff --git a/NzbDrone.Services/NzbDrone.Services.Service/App_Start/Logging.cs b/NzbDrone.Services/NzbDrone.Services.Service/App_Start/Logging.cs index 4070485ed..8b1aa0cda 100644 --- a/NzbDrone.Services/NzbDrone.Services.Service/App_Start/Logging.cs +++ b/NzbDrone.Services/NzbDrone.Services.Service/App_Start/Logging.cs @@ -15,7 +15,7 @@ namespace NzbDrone.Services.Service.App_Start public static void PreStart() { - string logPath = string.Format("C:\\NLog\\{0}\\{1}\\${{shortdate}}.log", HostingEnvironment.SiteName, new EnviromentProvider().Version); + string logPath = string.Format("C:\\NLog\\{0}\\{1}\\${{shortdate}}-${{logger}}.log", HostingEnvironment.SiteName, new EnviromentProvider().Version); string error = string.Format("C:\\NLog\\{0}\\{1}\\${{shortdate}}_Error.log", HostingEnvironment.SiteName, new EnviromentProvider().Version); LogConfiguration.RegisterUdpLogger(); diff --git a/NzbDrone.Services/NzbDrone.Services.Service/Controllers/ExceptionController.cs b/NzbDrone.Services/NzbDrone.Services.Service/Controllers/ExceptionController.cs index d3331a02d..502b7a45d 100644 --- a/NzbDrone.Services/NzbDrone.Services.Service/Controllers/ExceptionController.cs +++ b/NzbDrone.Services/NzbDrone.Services.Service/Controllers/ExceptionController.cs @@ -2,7 +2,6 @@ using System.Linq; using System.Web.Mvc; using NLog; -using NzbDrone.Common; using NzbDrone.Common.Contract; using NzbDrone.Services.Service.Repository.Reporting; using Services.PetaPoco; @@ -14,23 +13,53 @@ namespace NzbDrone.Services.Service.Controllers private readonly IDatabase _database; private static readonly Logger logger = LogManager.GetCurrentClassLogger(); - private const string OK = "OK"; - public ExceptionController(IDatabase database) { _database = database; } + + [HttpPost] + public EmptyResult ReportExisting(ExistingExceptionReport existingExceptionReport) + { + try + { + if (ExceptionHashExists(existingExceptionReport.Hash)) + { + var exceptionInstance = new ExceptionInstance + { + ExceptionHash = existingExceptionReport.Hash, + IsProduction = existingExceptionReport.IsProduction, + LogMessage = existingExceptionReport.LogMessage, + Timestamp = DateTime.Now + }; + + _database.Insert(exceptionInstance); + } + else + { + logger.Warn("Invalid exception hash '{0}'", existingExceptionReport.Hash); + } + } + catch (Exception e) + { + logger.FatalException("Error has occurred while saving exception", e); + throw; + } + + return new EmptyResult(); + } + [HttpPost] public JsonResult ReportNew(ExceptionReport exceptionReport) { try { - var exceptionId = GetExceptionDetailId(exceptionReport); + var exceptionHash = GetExceptionDetailId(exceptionReport); var exceptionInstance = new ExceptionInstance { - ExceptionDetail = exceptionId, + ExceptionHash = exceptionHash, IsProduction = exceptionReport.IsProduction, LogMessage = exceptionReport.LogMessage, Timestamp = DateTime.Now @@ -38,22 +67,20 @@ namespace NzbDrone.Services.Service.Controllers _database.Insert(exceptionInstance); - return new JsonResult { Data = new ExceptionReportResponse { ExceptionId = exceptionId } }; + return new JsonResult { Data = new ExceptionReportResponse { ExceptionHash = exceptionHash } }; } catch (Exception e) { - logger.FatalException("Error has occurred while logging exception", e); + logger.FatalException("Error has occurred while saving exception", e); throw; } } - - - private int GetExceptionDetailId(ExceptionReport exceptionReport) + + private string GetExceptionDetailId(ExceptionReport exceptionReport) { - var reportHash = Hash(exceptionReport.Version + exceptionReport.String + exceptionReport.Logger); - var id = _database.FirstOrDefault("SELECT Id FROM Exceptions WHERE Hash =@0", reportHash); - - if (id == 0) + var reportHash = Hash(String.Concat(exceptionReport.Version, exceptionReport.String, exceptionReport.Logger)); + + if (!ExceptionHashExists(reportHash)) { var exeptionDetail = new ExceptionDetail(); exeptionDetail.Hash = reportHash; @@ -62,10 +89,15 @@ namespace NzbDrone.Services.Service.Controllers exeptionDetail.Type = exceptionReport.Type; exeptionDetail.Version = exceptionReport.Version; - id = Convert.ToInt32(_database.Insert(exeptionDetail)); + _database.Insert(exeptionDetail); } - return id; + return reportHash; + } + + private bool ExceptionHashExists(string reportHash) + { + return _database.Exists(reportHash); } private static string Hash(string input) diff --git a/NzbDrone.Services/NzbDrone.Services.Service/Migrations/Migration20120229.cs b/NzbDrone.Services/NzbDrone.Services.Service/Migrations/Migration20120229.cs index 852c44cb3..edb1419bb 100644 --- a/NzbDrone.Services/NzbDrone.Services.Service/Migrations/Migration20120229.cs +++ b/NzbDrone.Services/NzbDrone.Services.Service/Migrations/Migration20120229.cs @@ -10,24 +10,25 @@ namespace NzbDrone.Services.Service.Migrations { public override void Up() { - Database.AddTable("ExceptionInstances", new Column("Id", DbType.Int64, ColumnProperty.PrimaryKeyWithIdentity), - new Column("ExceptionDetail", DbType.Int16, ColumnProperty.NotNull), + new Column("ExceptionHash", DbType.String, ColumnProperty.NotNull), new Column("LogMessage", DbType.String, 3000, ColumnProperty.NotNull), MigrationsHelper.TimestampColumn, MigrationsHelper.ProductionColumn); - Database.AddTable("Exceptions", new Column("Id", DbType.Int64, ColumnProperty.PrimaryKeyWithIdentity), + Database.AddTable("Exceptions", new Column("Hash", DbType.String, ColumnProperty.Unique), new Column("Logger", DbType.String, ColumnProperty.NotNull), new Column("Type", DbType.String, ColumnProperty.NotNull), new Column("String", DbType.String, ColumnProperty.NotNull), - new Column("Hash", DbType.String, ColumnProperty.NotNull), MigrationsHelper.VersionColumn); + var indexName = MigrationsHelper.GetIndexName("Exceptions", "Hash"); + Database.AddIndex(indexName, "Exceptions", "Hash"); + + //Database.AddForeignKey("FK_Exceptions_ExceptionInstances", "Exceptions", "Hash", "ExceptionInstances", "ExceptionHash", ForeignKeyConstraint.Cascade); Database.ExecuteNonQuery("ALTER TABLE ExceptionReports ALTER COLUMN String NTEXT"); Database.ExecuteNonQuery("ALTER TABLE Exceptions ALTER COLUMN String NTEXT"); - } public override void Down() diff --git a/NzbDrone.Services/NzbDrone.Services.Service/Migrations/MigrationHelper.cs b/NzbDrone.Services/NzbDrone.Services.Service/Migrations/MigrationHelper.cs index 689eb9b08..0490e647f 100644 --- a/NzbDrone.Services/NzbDrone.Services.Service/Migrations/MigrationHelper.cs +++ b/NzbDrone.Services/NzbDrone.Services.Service/Migrations/MigrationHelper.cs @@ -20,7 +20,7 @@ namespace NzbDrone.Services.Service.Migrations try { - var migrator = new Migrator.Migrator("sqlserver", connetionString, Assembly.GetAssembly(typeof(MigrationsHelper)), true, new MigrationLogger()); + var migrator = new Migrator.Migrator("sqlserver", connetionString, Assembly.GetAssembly(typeof(MigrationsHelper)), true); migrator.MigrateToLastVersion(); logger.Info("Database migration completed"); } diff --git a/NzbDrone.Services/NzbDrone.Services.Service/Repository/Reporting/ExceptionDetail.cs b/NzbDrone.Services/NzbDrone.Services.Service/Repository/Reporting/ExceptionDetail.cs index 512f40679..b35821fd0 100644 --- a/NzbDrone.Services/NzbDrone.Services.Service/Repository/Reporting/ExceptionDetail.cs +++ b/NzbDrone.Services/NzbDrone.Services.Service/Repository/Reporting/ExceptionDetail.cs @@ -1,17 +1,16 @@ -using System; using System.Linq; using Services.PetaPoco; namespace NzbDrone.Services.Service.Repository.Reporting { [TableName("Exceptions")] + [PrimaryKey("Hash", autoIncrement = false)] public class ExceptionDetail { - public int Id { get; set; } + public string Hash { get; set; } public string Logger { get; set; } public string Type { get; set; } public string String { get; set; } - public string Hash { get; set; } public string Version { get; set; } } } \ No newline at end of file diff --git a/NzbDrone.Services/NzbDrone.Services.Service/Repository/Reporting/ExceptionInstance.cs b/NzbDrone.Services/NzbDrone.Services.Service/Repository/Reporting/ExceptionInstance.cs index 452388f87..9d2e4a217 100644 --- a/NzbDrone.Services/NzbDrone.Services.Service/Repository/Reporting/ExceptionInstance.cs +++ b/NzbDrone.Services/NzbDrone.Services.Service/Repository/Reporting/ExceptionInstance.cs @@ -8,7 +8,7 @@ namespace NzbDrone.Services.Service.Repository.Reporting public class ExceptionInstance { public long Id { get; set; } - public int ExceptionDetail { get; set; } + public string ExceptionHash { get; set; } public string LogMessage { get; set; } public DateTime Timestamp { get; set; } public bool IsProduction { get; set; } diff --git a/NzbDrone.Services/NzbDrone.Services.Tests/ExceptionController/ReportExistingFixture.cs b/NzbDrone.Services/NzbDrone.Services.Tests/ExceptionController/ReportExistingFixture.cs new file mode 100644 index 000000000..00e92b737 --- /dev/null +++ b/NzbDrone.Services/NzbDrone.Services.Tests/ExceptionController/ReportExistingFixture.cs @@ -0,0 +1,66 @@ +using System; +using System.Linq; +using FizzWare.NBuilder; +using FluentAssertions; +using NUnit.Framework; +using NzbDrone.Common.Contract; +using NzbDrone.Services.Service.Repository.Reporting; +using NzbDrone.Services.Tests.Framework; +using NzbDrone.Test.Common; + +namespace NzbDrone.Services.Tests.ExceptionController +{ + [TestFixture] + public class ReportExistingFixture : ServicesTestBase + { + + Service.Controllers.ExceptionController Controller + { + get + { + return Mocker.Resolve(); + } + } + + private static ExistingExceptionReport CreateExceptionReport() + { + return new ExistingExceptionReport + { + IsProduction = true, + Version = "1.1.2.323456", + UGuid = Guid.NewGuid(), + LogMessage = @"Log message", + Hash = "ABC123" + }; + } + + [Test] + public void should_log_warn_if_hash_doesnt_exist() + { + WithRealDb(); + + Controller.ReportExisting(CreateExceptionReport()); + + Db.Fetch().Should().BeEmpty(); + Db.Fetch().Should().BeEmpty(); + + ExceptionVerification.ExpectedWarns(1); + } + + [Test] + public void should_save_instance_if_hash_is_valid() + { + WithRealDb(); + + var existing = CreateExceptionReport(); + + Db.Insert(Builder.CreateNew().With(c => c.Hash = existing.Hash).Build()); + + Controller.ReportExisting(CreateExceptionReport()); + + Db.Fetch().Should().HaveCount(1); + Db.Fetch().Should().HaveCount(1); + } + + } +} \ No newline at end of file diff --git a/NzbDrone.Services/NzbDrone.Services.Tests/ExceptionControllerFixture.cs b/NzbDrone.Services/NzbDrone.Services.Tests/ExceptionController/ReportNewFixture.cs similarity index 73% rename from NzbDrone.Services/NzbDrone.Services.Tests/ExceptionControllerFixture.cs rename to NzbDrone.Services/NzbDrone.Services.Tests/ExceptionController/ReportNewFixture.cs index 808a804b6..1d872d6aa 100644 --- a/NzbDrone.Services/NzbDrone.Services.Tests/ExceptionControllerFixture.cs +++ b/NzbDrone.Services/NzbDrone.Services.Tests/ExceptionController/ReportNewFixture.cs @@ -3,21 +3,20 @@ using System.Linq; using FluentAssertions; using NUnit.Framework; using NzbDrone.Common.Contract; -using NzbDrone.Services.Service.Controllers; using NzbDrone.Services.Service.Repository.Reporting; using NzbDrone.Services.Tests.Framework; -namespace NzbDrone.Services.Tests +namespace NzbDrone.Services.Tests.ExceptionController { [TestFixture] - public class ExceptionControllerFixture : ServicesTestBase + public class ReportNewFixture : ServicesTestBase { - ExceptionController Controller + Service.Controllers.ExceptionController Controller { get { - return Mocker.Resolve(); + return Mocker.Resolve(); } } @@ -89,12 +88,30 @@ namespace NzbDrone.Services.Tests var exceptionInstance = Db.Fetch(); exceptionInstance.Should().HaveCount(1); exceptionInstance.Single().Id.Should().BeGreaterThan(0); - exceptionInstance.Single().ExceptionDetail.Should().BeGreaterThan(0); + exceptionInstance.Single().ExceptionHash.Should().NotBeBlank(); exceptionInstance.Single().IsProduction.Should().Be(exceptionReport.IsProduction); exceptionInstance.Single().Timestamp.Should().BeWithin(TimeSpan.FromSeconds(4)).Before(DateTime.Now); exceptionInstance.Single().LogMessage.Should().Be(exceptionReport.LogMessage); } + [Test] + public void ReportNew_should_save_detail() + { + var exceptionReport = CreateExceptionReport(); + + WithRealDb(); + + Controller.ReportNew(exceptionReport); + + var exceptionDetails = Db.Fetch(); + exceptionDetails.Should().HaveCount(1); + exceptionDetails.Single().Hash.Should().NotBeBlank(); + exceptionDetails.Single().Logger.Should().Be(exceptionReport.Logger); + exceptionDetails.Single().Type.Should().Be(exceptionReport.Type); + exceptionDetails.Single().String.Should().Be(exceptionReport.String); + exceptionDetails.Single().Version.Should().Be(exceptionReport.Version); + } + [Test] public void ReportNew_should_return_exception_id() { @@ -105,7 +122,7 @@ namespace NzbDrone.Services.Tests var response = Controller.ReportNew(exceptionReport); response.Data.Should().BeOfType(); - ((ExceptionReportResponse)response.Data).ExceptionId.Should().BeGreaterThan(0); + ((ExceptionReportResponse)response.Data).ExceptionHash.Should().NotBeBlank(); } @@ -119,15 +136,77 @@ namespace NzbDrone.Services.Tests var response1 = Controller.ReportNew(exceptionReport); var response2 = Controller.ReportNew(exceptionReport); var response3 = Controller.ReportNew(exceptionReport); - + var detail = Db.Fetch(); var instances = Db.Fetch(); detail.Should().HaveCount(1); instances.Should().HaveCount(3); - instances.Should().OnlyContain(c => c.ExceptionDetail == detail.Single().Id); + instances.Should().OnlyContain(c => c.ExceptionHash == detail.Single().Hash); + } + + [Test] + public void Reporting_exception_with_diffrent_version_should_create_new_detail() + { + var exceptionReport1 = CreateExceptionReport(); + exceptionReport1.Version = "0.1.1"; + + var exceptionReport2 = CreateExceptionReport(); + exceptionReport2.Version = "0.2.1"; + + WithRealDb(); + + Controller.ReportNew(exceptionReport1); + Controller.ReportNew(exceptionReport2); + + var detail = Db.Fetch(); + var instances = Db.Fetch(); + + detail.Should().HaveCount(2); + instances.Should().HaveCount(2); + } + + [Test] + public void Reporting_exception_with_diffrent_strting_should_create_new_detail() + { + var exceptionReport1 = CreateExceptionReport(); + exceptionReport1.String = "Error1"; + + var exceptionReport2 = CreateExceptionReport(); + exceptionReport2.String = "Error2"; + + WithRealDb(); + + Controller.ReportNew(exceptionReport1); + Controller.ReportNew(exceptionReport2); + + var detail = Db.Fetch(); + var instances = Db.Fetch(); + + detail.Should().HaveCount(2); + instances.Should().HaveCount(2); + } + + [Test] + public void Reporting_exception_with_diffrent_logger_should_create_new_detail() + { + var exceptionReport1 = CreateExceptionReport(); + exceptionReport1.Logger = "logger1"; + + var exceptionReport2 = CreateExceptionReport(); + exceptionReport2.Logger = "logger2"; + + WithRealDb(); + + Controller.ReportNew(exceptionReport1); + Controller.ReportNew(exceptionReport2); + + var detail = Db.Fetch(); + var instances = Db.Fetch(); + detail.Should().HaveCount(2); + instances.Should().HaveCount(2); } } } \ No newline at end of file diff --git a/NzbDrone.Services/NzbDrone.Services.Tests/Framework/ServicesTestBase.cs b/NzbDrone.Services/NzbDrone.Services.Tests/Framework/ServicesTestBase.cs index 49a649356..df2aebc8e 100644 --- a/NzbDrone.Services/NzbDrone.Services.Tests/Framework/ServicesTestBase.cs +++ b/NzbDrone.Services/NzbDrone.Services.Tests/Framework/ServicesTestBase.cs @@ -4,6 +4,7 @@ using System.Configuration; using System.Linq; using System.Text; using Migrator.Providers.SqlServer; +using NUnit.Framework; using NzbDrone.Services.Service.Migrations; using NzbDrone.Test.Common; using NzbDrone.Test.Common.AutoMoq; @@ -60,5 +61,11 @@ namespace NzbDrone.Services.Tests.Framework Mocker.SetConstant(Db); } + [TearDown] + public void ServiceTearDown() + { + ExceptionVerification.IgnoreWarns(); + } + } } diff --git a/NzbDrone.Services/NzbDrone.Services.Tests/NzbDrone.Services.Tests.csproj b/NzbDrone.Services/NzbDrone.Services.Tests/NzbDrone.Services.Tests.csproj index 63b078b3b..11bf63b31 100644 --- a/NzbDrone.Services/NzbDrone.Services.Tests/NzbDrone.Services.Tests.csproj +++ b/NzbDrone.Services/NzbDrone.Services.Tests/NzbDrone.Services.Tests.csproj @@ -50,6 +50,9 @@ ..\..\packages\Moq.4.0.10827\lib\NET40\Moq.dll + + ..\..\packages\NLog.2.0.0.2000\lib\net40\NLog.dll + False ..\..\packages\NUnit.2.6.0.12054\lib\nunit.framework.dll @@ -65,7 +68,8 @@ - + + diff --git a/NzbDrone.Services/NzbDrone.Services.Tests/packages.config b/NzbDrone.Services/NzbDrone.Services.Tests/packages.config index 168be1c4d..1a44bb0a2 100644 --- a/NzbDrone.Services/NzbDrone.Services.Tests/packages.config +++ b/NzbDrone.Services/NzbDrone.Services.Tests/packages.config @@ -3,5 +3,6 @@ + \ No newline at end of file