From 7cad6c32f6446732c4a6e4e68e0c6476fedad927 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janis=20Daniel=20Da=CC=88hne?= <janis.daehne2@student.uni-halle.de> Date: Fri, 1 Nov 2019 17:44:53 +0100 Subject: [PATCH] - wrapped every beginTransaction in a try so frontend and backend get more details if an error occurs - removed transaction from creating exercise copy because it was not working (unknown reason, only in production environment) - added self healing when saving an exercise that has no main file (first tempalte file will be used) --- src/ClientServer/Config/Constants.cs | 2 +- .../EditCustomProjectController.cs | 75 +++-- .../DoExerciseAfterSolutionController.cs | 45 +-- .../Core/Exercises/DoExerciseController.cs | 97 ++++--- .../Exercises/ExerciseEditorController.cs | 265 ++++++++++++------ 5 files changed, 306 insertions(+), 178 deletions(-) diff --git a/src/ClientServer/Config/Constants.cs b/src/ClientServer/Config/Constants.cs index 6872872..bbf2915 100644 --- a/src/ClientServer/Config/Constants.cs +++ b/src/ClientServer/Config/Constants.cs @@ -13,7 +13,7 @@ namespace ClientServer.Helpers /// </summary> public static class Constants { - public static string VersionString = "2.5.3"; + public static string VersionString = "2.5.5"; /// <summary> /// this is only set once at program.cs!! diff --git a/src/ClientServer/Controllers/Core/CustomProjects/EditCustomProjectController.cs b/src/ClientServer/Controllers/Core/CustomProjects/EditCustomProjectController.cs index 5871e29..7b60f6e 100644 --- a/src/ClientServer/Controllers/Core/CustomProjects/EditCustomProjectController.cs +++ b/src/ClientServer/Controllers/Core/CustomProjects/EditCustomProjectController.cs @@ -419,30 +419,38 @@ namespace ClientServer.Controllers.Core.customProjects #endregion - - using (var transaction = _context.Database.BeginTransaction()) + try { - try + using (var transaction = _context.Database.BeginTransaction()) { - _context.CustomProjects.Add(customProject); - await _context.SaveChangesAsync(); + try + { + _context.CustomProjects.Add(customProject); + await _context.SaveChangesAsync(); - //then set main file (else we get an error) + //then set main file (else we get an error) - solution.MainFile = mainFile; + solution.MainFile = mainFile; - await _context.SaveChangesAsync(); + await _context.SaveChangesAsync(); - transaction.Commit(); - } - catch (Exception ex) - { - transaction.Rollback(); + transaction.Commit(); + } + catch (Exception ex) + { + transaction.Rollback(); - await base.HandleDbError(ex); - return; + await base.HandleDbError(ex); + return; + } } } + catch (Exception e) + { + //see https://www.npgsql.org/doc/transactions.html + await base.HandleDbError(e); + return; + } await Response.WriteAsync( @@ -592,27 +600,36 @@ namespace ClientServer.Controllers.Core.customProjects customProject.LastEditorPLangId = savedSolution.PLangId; customProject.LastUpdatedAt = DateTimeHelper.GetUtcNow(); - using (var transaction = _context.Database.BeginTransaction()) + try { - try + using (var transaction = _context.Database.BeginTransaction()) { - await _context.SaveChangesAsync(); - if (mainFile != null) + try { - //main file changed - savedSolution.MainFile = mainFile; await _context.SaveChangesAsync(); - } + if (mainFile != null) + { + //main file changed + savedSolution.MainFile = mainFile; + await _context.SaveChangesAsync(); + } - transaction.Commit(); - } - catch (Exception ex) - { - transaction.Rollback(); - await base.HandleDbError(ex); - return; + transaction.Commit(); + } + catch (Exception ex) + { + transaction.Rollback(); + await base.HandleDbError(ex); + return; + } } } + catch (Exception e) + { + //see https://www.npgsql.org/doc/transactions.html + await base.HandleDbError(e); + return; + } var response = new CustomProjectSolutionFullBase() { diff --git a/src/ClientServer/Controllers/Core/Exercises/DoExerciseAfterSolution/DoExerciseAfterSolutionController.cs b/src/ClientServer/Controllers/Core/Exercises/DoExerciseAfterSolution/DoExerciseAfterSolutionController.cs index 79f64a7..fe94152 100644 --- a/src/ClientServer/Controllers/Core/Exercises/DoExerciseAfterSolution/DoExerciseAfterSolutionController.cs +++ b/src/ClientServer/Controllers/Core/Exercises/DoExerciseAfterSolution/DoExerciseAfterSolutionController.cs @@ -522,34 +522,43 @@ namespace ClientServer.Controllers.Core.Exercises.DoExerciseAfterSolution return; } - using (var transaction = _context.Database.BeginTransaction()) + try { - try + using (var transaction = _context.Database.BeginTransaction()) { - if (shouldCreateNewAfterSolution) + try { - _context.AfterSolutions.Add(oldAfterSolution); - } + if (shouldCreateNewAfterSolution) + { + _context.AfterSolutions.Add(oldAfterSolution); + } - //use a transaction to ensure the main file is already + //use a transaction to ensure the main file is already - //save the solution changes - await _context.SaveChangesAsync(); + //save the solution changes + await _context.SaveChangesAsync(); - //always save in case user has an invalid solution - mainClassFileTuple.Item1.MainFile = mainClassFileTuple.Item2; - await _context.SaveChangesAsync(); + //always save in case user has an invalid solution + mainClassFileTuple.Item1.MainFile = mainClassFileTuple.Item2; + await _context.SaveChangesAsync(); - transaction.Commit(); - } - catch (Exception ex) - { - transaction.Rollback(); + transaction.Commit(); + } + catch (Exception ex) + { + transaction.Rollback(); - await base.HandleDbError(ex); - return; + await base.HandleDbError(ex); + return; + } } } + catch (Exception e) + { + //see https://www.npgsql.org/doc/transactions.html + await base.HandleDbError(e); + return; + } //return after solution e.g. we corrected some files... diff --git a/src/ClientServer/Controllers/Core/Exercises/DoExerciseController.cs b/src/ClientServer/Controllers/Core/Exercises/DoExerciseController.cs index 519e9cd..451e091 100644 --- a/src/ClientServer/Controllers/Core/Exercises/DoExerciseController.cs +++ b/src/ClientServer/Controllers/Core/Exercises/DoExerciseController.cs @@ -900,38 +900,47 @@ namespace ClientServer.Controllers.Core.Exercises LastEditingIpAddress = NetworkHelper.GetIpAddress(HttpContext) }; - using (var transaction = _context.Database.BeginTransaction()) + try { - try + using (var transaction = _context.Database.BeginTransaction()) { - if (oldSolutionToDelete != null) + try { - //we need to unset the main file first else we get circular dependency error... - oldSolutionToDelete.MainFile = null; - await _context.SaveChangesAsync(); - //this should automatically clear all old stuff (e.g. old test results) - _context.Solutions.Remove(oldSolutionToDelete); - await _context.SaveChangesAsync(); - } + if (oldSolutionToDelete != null) + { + //we need to unset the main file first else we get circular dependency error... + oldSolutionToDelete.MainFile = null; + await _context.SaveChangesAsync(); + //this should automatically clear all old stuff (e.g. old test results) + _context.Solutions.Remove(oldSolutionToDelete); + await _context.SaveChangesAsync(); + } - //first save solution - _context.Solutions.Add(userSolution); - await _context.SaveChangesAsync(); + //first save solution + _context.Solutions.Add(userSolution); + await _context.SaveChangesAsync(); - //then set main file else would give us circular dependency error on db update ... - userSolution.MainFile = mainFile; - await _context.SaveChangesAsync(); + //then set main file else would give us circular dependency error on db update ... + userSolution.MainFile = mainFile; + await _context.SaveChangesAsync(); - transaction.Commit(); - } - catch (Exception ex) - { - transaction.Rollback(); + transaction.Commit(); + } + catch (Exception ex) + { + transaction.Rollback(); - await base.HandleDbError(ex); - return null; + await base.HandleDbError(ex); + return null; + } } } + catch (Exception e) + { + //see https://www.npgsql.org/doc/transactions.html + await base.HandleDbError(e); + return null; + } #endregion @@ -1882,29 +1891,38 @@ namespace ClientServer.Controllers.Core.Exercises return; } - using (var transaction = _context.Database.BeginTransaction()) + try { - try + using (var transaction = _context.Database.BeginTransaction()) { - //use a transaction to ensure the main file is already + try + { + //use a transaction to ensure the main file is already - //save the solution changes - await _context.SaveChangesAsync(); + //save the solution changes + await _context.SaveChangesAsync(); - //always save in case user has an invalid solution - mainClassFileTuple.Item1.MainFile = mainClassFileTuple.Item2; - await _context.SaveChangesAsync(); + //always save in case user has an invalid solution + mainClassFileTuple.Item1.MainFile = mainClassFileTuple.Item2; + await _context.SaveChangesAsync(); - transaction.Commit(); - } - catch (Exception ex) - { - transaction.Rollback(); + transaction.Commit(); + } + catch (Exception ex) + { + transaction.Rollback(); - await base.HandleDbError(ex); - return; + await base.HandleDbError(ex); + return; + } } } + catch (Exception e) + { + //see https://www.npgsql.org/doc/transactions.html + await base.HandleDbError(e); + return; + } var solutionVersionForFrontend = new SolutionDoExerciseFullBase() @@ -3833,12 +3851,13 @@ namespace ClientServer.Controllers.Core.Exercises /// can be null /// </summary> public string TestServerVersion { get; set; } - + /// <summary> /// true: the limit <see cref="YapexDbContext.TestProtocolMaxStringLength"/> was exceeded and /// the <see cref="Protocol"/> was cut /// </summary> public bool CharacterLimitExceeded { get; set; } + /// <summary> /// the db will only store a max of characters <see cref="YapexDbContext.TestProtocolMaxStringLength"/> /// can be null for old results diff --git a/src/ClientServer/Controllers/Core/Exercises/ExerciseEditorController.cs b/src/ClientServer/Controllers/Core/Exercises/ExerciseEditorController.cs index 4d6250d..ea8f588 100644 --- a/src/ClientServer/Controllers/Core/Exercises/ExerciseEditorController.cs +++ b/src/ClientServer/Controllers/Core/Exercises/ExerciseEditorController.cs @@ -1,6 +1,8 @@ using System; using System.Collections.Generic; +using System.Data; using System.Linq; +using System.Threading; using System.Threading.Tasks; using ClientServer.Db; using ClientServer.Helpers; @@ -24,11 +26,11 @@ namespace ClientServer.Controllers.Core.Exercises { } - private async Task<ExerciseFromBackend> CreateFrontendExercise(Exercise oldExercise, int numReleases, bool includeAssetFileData) + private async Task<ExerciseFromBackend> CreateFrontendExercise(Exercise oldExercise, int numReleases, + bool includeAssetFileData) { - Dictionary<int, List<byte>> markdownFilesDict = null; - + if (includeAssetFileData) { var markdownFilesIdsToRead = oldExercise.Description.AssetReferences @@ -39,7 +41,7 @@ namespace ClientServer.Controllers.Core.Exercises markdownFilesDict = await Files.ReadUploadedFiles(markdownAssetsBasePath, markdownFilesIdsToRead); } - + var exerciseForFrontend = new ExerciseFromBackend() { @@ -66,8 +68,8 @@ namespace ClientServer.Controllers.Core.Exercises Id = p.FileReferenceMarkdownAsset.Id, DisplayName = p.FileReferenceMarkdownAsset.OriginalName, MimeType = p.FileReferenceMarkdownAsset.MimeType, - Content = (includeAssetFileData && markdownFilesDict != null) - ? markdownFilesDict[p.FileReferenceMarkdownAssetId] + Content = (includeAssetFileData && markdownFilesDict != null) + ? markdownFilesDict[p.FileReferenceMarkdownAssetId] : new List<byte>() }).ToList() }, @@ -333,7 +335,8 @@ namespace ClientServer.Controllers.Core.Exercises if (shouldLoadTestAssets) { - var exerciseForFrontendWithData = await CreateFrontendExerciseWithTestFilesData(oldExercise, numReleases); + var exerciseForFrontendWithData = + await CreateFrontendExerciseWithTestFilesData(oldExercise, numReleases); await Response.WriteAsync( @@ -590,31 +593,40 @@ namespace ClientServer.Controllers.Core.Exercises newExercise.Note = exerciseFromFrontend.Note; - using (var transaction = _context.Database.BeginTransaction()) + try { - try + using (var transaction = _context.Database.BeginTransaction()) { - _context.Exercises.Add(newExercise); - await _context.SaveChangesAsync(); - - //now set main files... - foreach (var mainFile in mainFiles) + try { - mainFile.Item1.MainFile = mainFile.Item2; - } + _context.Exercises.Add(newExercise); + await _context.SaveChangesAsync(); - await _context.SaveChangesAsync(); + //now set main files... + foreach (var mainFile in mainFiles) + { + mainFile.Item1.MainFile = mainFile.Item2; + } - transaction.Commit(); - } - catch (Exception ex) - { - transaction.Rollback(); + await _context.SaveChangesAsync(); - await base.HandleDbError(ex); - return; + transaction.Commit(); + } + catch (Exception ex) + { + transaction.Rollback(); + + await base.HandleDbError(ex); + return; + } } } + catch (Exception e) + { + //see https://www.npgsql.org/doc/transactions.html + await base.HandleDbError(e); + return; + } //do not send the full exercise back because when saving in frontend we reload the exercise fully (calling the get method) //this way we get all created ids and corrected values @@ -874,7 +886,8 @@ namespace ClientServer.Controllers.Core.Exercises oldExercise.DefaultCustomTestSettings.MemoryLimitInKb = exerciseFromFrontend.DefaultTestSettings.MemoryLimitInKb; oldExercise.DefaultCustomTestSettings.TimeoutInMs = exerciseFromFrontend.DefaultTestSettings.TimeoutInMs; - oldExercise.DefaultCustomTestSettings.CompilerOptions = exerciseFromFrontend.DefaultTestSettings.CompilerOptions; + oldExercise.DefaultCustomTestSettings.CompilerOptions = + exerciseFromFrontend.DefaultTestSettings.CompilerOptions; #endregion @@ -981,6 +994,8 @@ namespace ClientServer.Controllers.Core.Exercises #region templates + bool didSelfHeadlingMainFile = false; + //removing or adding template files is only allowed when there is no release... see frontend issue #56 var hasReleases = await _context.ExerciseReleases @@ -1201,10 +1216,23 @@ namespace ClientServer.Controllers.Core.Exercises if (mainClassFileFound == false) { - await - Response.WriteAsync( - Jc.Serialize(new BasicResponse(ResponseCode.NotFound, "main file not found"))); - return; + + //because we removed the main file transaction... we do self healing here... + //we use the first file as main file... + + if (newCodeTemplate.TemplateFiles.Count > 0) + { + didSelfHeadlingMainFile = true; + mainClassFiles.Add(new Tuple<CodeTemplate, TemplateFile>(newCodeTemplate, newCodeTemplate.TemplateFiles.First())); + mainClassFileFound = true; + } + else + { + await + Response.WriteAsync( + Jc.Serialize(new BasicResponse(ResponseCode.NotFound, "main file not found"))); + return; + } } #endregion @@ -1247,45 +1275,56 @@ namespace ClientServer.Controllers.Core.Exercises #endregion - using (var transaction = _context.Database.BeginTransaction()) + try { - try + using (var transaction = _context.Database.BeginTransaction()) { - await _context.SaveChangesAsync(); - - //now main files... - if (mainClassFiles.Count > 0) + try { - foreach (Tuple<CodeTemplate, TemplateFile> mainClassFile in mainClassFiles) - { - mainClassFile.Item1.MainFile = mainClassFile.Item2; - } - await _context.SaveChangesAsync(); - } - //now remove entire code templates (before we just set the main file to null to avoid circular dependency) - if (codeTemplatesToRemove.Count > 0) - { - foreach (var tuple in codeTemplatesToRemove) + Thread.Sleep(2000); + + //now main files... + if (mainClassFiles.Count > 0) { - tuple.Item1.CodeTemplates.Remove(tuple.Item2); + foreach (Tuple<CodeTemplate, TemplateFile> mainClassFile in mainClassFiles) + { + mainClassFile.Item1.MainFile = mainClassFile.Item2; + } + + await _context.SaveChangesAsync(); } - await _context.SaveChangesAsync(); - } + //now remove entire code templates (before we just set the main file to null to avoid circular dependency) + if (codeTemplatesToRemove.Count > 0) + { + foreach (var tuple in codeTemplatesToRemove) + { + tuple.Item1.CodeTemplates.Remove(tuple.Item2); + } + await _context.SaveChangesAsync(); + } - transaction.Commit(); - } - catch (Exception ex) - { - transaction.Rollback(); - await base.HandleDbError(ex); - return; + transaction.Commit(); + } + catch (Exception ex) + { + transaction.Rollback(); + + await base.HandleDbError(ex); + return; + } } } + catch (Exception e) + { + //see https://www.npgsql.org/doc/transactions.html + await base.HandleDbError(e); + return; + } //we need to send back the exercise (or at least the new ids) because we need the new ids //e.g. we create the exercise then add a new teSt (gets id -60) and save again but the -60 is never updated... @@ -1297,6 +1336,15 @@ namespace ClientServer.Controllers.Core.Exercises //assets are not included on update!! var exerciseForFrontend = await CreateFrontendExercise(oldExercise, numReleases, false); + if (didSelfHeadlingMainFile) + { + await + Response.WriteAsync( + Jc.Serialize(new BasicResponseWithData<ExerciseFromBackend>(ResponseCode.PartialOk, + "main file was set to the first template file", exerciseForFrontend))); + return; + } + await Response.WriteAsync( Jc.Serialize(new BasicResponseWithData<ExerciseFromBackend>(ResponseCode.Ok, @@ -1399,6 +1447,7 @@ namespace ClientServer.Controllers.Core.Exercises .Include(p => p.Tests) .ThenInclude(p => p.AssetReferences) .ThenInclude(p => p.FileReferenceTestAsset) + .AsNoTracking() .FirstOrDefaultAsync(); if (oldExercise == null) @@ -1410,8 +1459,10 @@ namespace ClientServer.Controllers.Core.Exercises } var targetUserGroup = - await _context.UserGroups.FirstOrDefaultAsync(p => - p.Id == oldExercise.UserGroupId); + await _context.UserGroups + .AsNoTracking() + .FirstOrDefaultAsync(p => + p.Id == oldExercise.UserGroupId); if (targetUserGroup == null) { @@ -1606,30 +1657,61 @@ namespace ClientServer.Controllers.Core.Exercises }, }; - using (var transaction = _context.Database.BeginTransaction()) + try { - try - { - _context.Exercises.Add(exCopy); - await _context.SaveChangesAsync(); - - //now set main files... - foreach (var mainFile in mainFiles) + + //NOT WORKING AT ALL in production environment... + //we already fail at _context.Database.BeginTransaction() (locally works) + //sometimes we can force this locally when we login as a different user (other browser) save a solution/exercise + //e.g. via shortcut and try to create a copy (time between the two actions must be < ~0.5s?) + //without transaction this works fine... + //in the worst case we have a exercise but no main file!! + + //see https://docs.microsoft.com/en-us/previous-versions/sql/sql-server-2008-r2/ms189122(v=sql.105)?redirectedfrom=MSDN + //also see https://coderwall.com/p/jnniww/why-you-shouldn-t-use-entity-framework-with-transactions + //default is read committed + +// using (var transaction = _context.Database.BeginTransaction(IsolationLevel.Snapshot)) + +// using (var transaction = _context.Database.BeginTransaction(IsolationLevel.Serializable)) +// using (var transaction = _context.Database.BeginTransaction()) +// { + try { - mainFile.Item1.MainFile = mainFile.Item2; + _context.Exercises.Add(exCopy); + await _context.SaveChangesAsync(); + + //now set main files... + foreach (var mainFile in mainFiles) + { + mainFile.Item1.MainFile = mainFile.Item2; + } + + await _context.SaveChangesAsync(); + +// transaction.Commit(); } + catch (Exception ex) + { + Console.WriteLine(ex.Message); + if (ex.InnerException != null) + { + Console.WriteLine(ex.InnerException); + } - await _context.SaveChangesAsync(); +// transaction.Rollback(); - transaction.Commit(); - } - catch (Exception ex) - { - transaction.Rollback(); - await base.HandleDbError(ex); - return; - } + await base.HandleDbError(ex); + return; + } +// } + } + catch (Exception e) + { + //see https://www.npgsql.org/doc/transactions.html + await base.HandleDbError(e); + return; } //do not send the full exercise back because when saving in frontend we reload the exercise fully (calling the get method) @@ -2003,18 +2085,18 @@ namespace ClientServer.Controllers.Core.Exercises && groupIdsWhereUserHasAccess.Contains(p.ExerciseDescription.Exercise.UserGroupId)) .ToListAsync() ; - var allFileReferencesForFrontend = references - .DistinctBy(p => p.FileReferenceMarkdownAssetId) //a file can be connected to two exercises in the same group + .DistinctBy(p => + p.FileReferenceMarkdownAssetId) //a file can be connected to two exercises in the same group .Select(p => new FilePreviewFromBackend() - { - Id = p.FileReferenceMarkdownAsset.Id, - SizeInBytes = p.FileReferenceMarkdownAsset.SizeInBytes, - OriginalName = p.FileReferenceMarkdownAsset.OriginalName, - MimeType = p.FileReferenceMarkdownAsset.MimeType - }).ToList(); + { + Id = p.FileReferenceMarkdownAsset.Id, + SizeInBytes = p.FileReferenceMarkdownAsset.SizeInBytes, + OriginalName = p.FileReferenceMarkdownAsset.OriginalName, + MimeType = p.FileReferenceMarkdownAsset.MimeType + }).ToList(); await Response.WriteAsync( @@ -2415,14 +2497,15 @@ namespace ClientServer.Controllers.Core.Exercises ; var allFileReferencesForFrontend = references - .DistinctBy(p => p.FileReferenceTestAssetId) //a test file can be connected to two exercises in the same group + .DistinctBy(p => + p.FileReferenceTestAssetId) //a test file can be connected to two exercises in the same group .Select(p => new FilePreviewFromBackend() - { - Id = p.FileReferenceTestAsset.Id, - SizeInBytes = p.FileReferenceTestAsset.SizeInBytes, - OriginalName = p.FileReferenceTestAsset.OriginalName, - MimeType = p.FileReferenceTestAsset.MimeType - }).ToList(); + { + Id = p.FileReferenceTestAsset.Id, + SizeInBytes = p.FileReferenceTestAsset.SizeInBytes, + OriginalName = p.FileReferenceTestAsset.OriginalName, + MimeType = p.FileReferenceTestAsset.MimeType + }).ToList(); await Response.WriteAsync( @@ -2637,7 +2720,7 @@ namespace ClientServer.Controllers.Core.Exercises /// the max disk space to users program can write to /// </summary> public int MaxDiskSpaceInKb { get; set; } - + /// <summary> /// some compiler options .g. -Xlint options needs to be separated by whitespaces (like normal command args) /// </summary> -- GitLab