Merge pull request #7982 from EVAST9919/comments-crash-fix

Fix potential crash when clicking on show more button in comments
This commit is contained in:
Dean Herbert 2020-02-25 20:14:58 +09:00 committed by GitHub
commit 61af80c1af
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 60 additions and 17 deletions

View File

@ -13,6 +13,8 @@ using osu.Framework.Bindables;
using osu.Framework.Graphics.Containers;
using osu.Framework.Graphics;
using osuTK;
using JetBrains.Annotations;
using NUnit.Framework;
namespace osu.Game.Tests.Visual.Online
{
@ -30,6 +32,8 @@ namespace osu.Game.Tests.Visual.Online
private readonly BindableBool showDeleted = new BindableBool();
private readonly Container content;
private TestCommentsPage commentsPage;
public TestSceneCommentsPage()
{
Add(new FillFlowContainer
@ -57,15 +61,29 @@ namespace osu.Game.Tests.Visual.Online
}
}
});
}
AddStep("load comments", () => createPage(getCommentBundle()));
AddStep("load empty comments", () => createPage(getEmptyCommentBundle()));
[Test]
public void TestAppendDuplicatedComment()
{
AddStep("Create page", () => createPage(getCommentBundle()));
AddAssert("Dictionary length is 10", () => commentsPage?.DictionaryLength == 10);
AddStep("Append existing comment", () => commentsPage?.AppendComments(getCommentSubBundle()));
AddAssert("Dictionary length is 10", () => commentsPage?.DictionaryLength == 10);
}
[Test]
public void TestEmptyBundle()
{
AddStep("Create page", () => createPage(getEmptyCommentBundle()));
AddAssert("Dictionary length is 0", () => commentsPage?.DictionaryLength == 0);
}
private void createPage(CommentBundle commentBundle)
{
commentsPage = null;
content.Clear();
content.Add(new CommentsPage(commentBundle)
content.Add(commentsPage = new TestCommentsPage(commentBundle)
{
ShowDeleted = { BindTarget = showDeleted }
});
@ -182,5 +200,33 @@ namespace osu.Game.Tests.Visual.Online
}
},
};
private CommentBundle getCommentSubBundle() => new CommentBundle
{
Comments = new List<Comment>
{
new Comment
{
Id = 1,
Message = "Simple test comment",
LegacyName = "TestUser1",
CreatedAt = DateTimeOffset.Now,
VotesCount = 5
},
},
IncludedComments = new List<Comment>(),
};
private class TestCommentsPage : CommentsPage
{
public TestCommentsPage(CommentBundle commentBundle)
: base(commentBundle)
{
}
public new void AppendComments([NotNull] CommentBundle bundle) => base.AppendComments(bundle);
public int DictionaryLength => CommentDictionary.Count;
}
}
}

View File

@ -61,15 +61,15 @@ namespace osu.Game.Overlays.Comments
return;
}
appendComments(commentBundle);
AppendComments(commentBundle);
}
private DrawableComment getDrawableComment(Comment comment)
{
if (commentDictionary.TryGetValue(comment.Id, out var existing))
if (CommentDictionary.TryGetValue(comment.Id, out var existing))
return existing;
return commentDictionary[comment.Id] = new DrawableComment(comment)
return CommentDictionary[comment.Id] = new DrawableComment(comment)
{
ShowDeleted = { BindTarget = ShowDeleted },
Sort = { BindTarget = Sort },
@ -81,31 +81,28 @@ namespace osu.Game.Overlays.Comments
{
var request = new GetCommentsRequest(CommentableId.Value, Type.Value, Sort.Value, page, drawableComment.Comment.Id);
request.Success += response => Schedule(() => appendComments(response));
request.Success += response => Schedule(() => AppendComments(response));
api.PerformAsync(request);
}
private readonly Dictionary<long, DrawableComment> commentDictionary = new Dictionary<long, DrawableComment>();
protected readonly Dictionary<long, DrawableComment> CommentDictionary = new Dictionary<long, DrawableComment>();
/// <summary>
/// Appends retrieved comments to the subtree rooted of comments in this page.
/// </summary>
/// <param name="bundle">The bundle of comments to add.</param>
private void appendComments([NotNull] CommentBundle bundle)
protected void AppendComments([NotNull] CommentBundle bundle)
{
var orphaned = new List<Comment>();
foreach (var topLevel in bundle.Comments)
addNewComment(topLevel);
foreach (var child in bundle.IncludedComments)
foreach (var comment in bundle.Comments.Concat(bundle.IncludedComments))
{
// Included comments can contain the parent comment, which already exists in the hierarchy.
if (commentDictionary.ContainsKey(child.Id))
// Exclude possible duplicated comments.
if (CommentDictionary.ContainsKey(comment.Id))
continue;
addNewComment(child);
addNewComment(comment);
}
// Comments whose parents were seen later than themselves can now be added.
@ -121,7 +118,7 @@ namespace osu.Game.Overlays.Comments
// Comments that have no parent are added as top-level comments to the flow.
flow.Add(drawableComment);
}
else if (commentDictionary.TryGetValue(comment.ParentId.Value, out var parentDrawable))
else if (CommentDictionary.TryGetValue(comment.ParentId.Value, out var parentDrawable))
{
// The comment's parent has already been seen, so the parent<-> child links can be added.
comment.ParentComment = parentDrawable.Comment;