Update existing repo: #122
Conversation
when a user submits a repository that already exists, a page is returned where he can choose if he wants to go to the existing source or force the update. Added a new action "Update" (and a View) in the UploadController. Refactored some of the ProcessSolution logic into the UploadRepository class. As suggested in the comments (by Amadeus Wieczorek), added support for multi-solution repository and parallel solution processing.
|
Thanks for the contribution! It's been a long time since we've done anything here, but I'll try to jog my memory and take a look this weekend. /cc @AmadeusW |
|
Yes, I noticed the repo's inactivity. Thank you for your time. I'm actually using SourceBrowser for a project of mine, and I came across the inability to update the previous version that I uploaded, so this is the main functionality that I'd like to see in production. There is one problem with the multi-solution repository support, though: references of types defined in another solution are not recognized during the processing, therefore you're not able to "click" on a type that is defined in another solution. Let me know if this PR requires some refactoring to be ready to be merged. |
| @@ -26,7 +26,7 @@ public virtual void Visit(WorkspaceModel workspaceModel) | |||
|
|
|||
| protected virtual void VisitWorkspace(WorkspaceModel workspaceModel) | |||
| { | |||
| foreach(var child in _workspaceModel.Children) | |||
There was a problem hiding this comment.
Weird, we're not even using this backing field _workspaceModel.
| @@ -34,20 +35,55 @@ public TreeViewTransformer(string savePath, string userName, string repoName) | |||
|
|
|||
| protected override void VisitWorkspace(WorkspaceModel workspaceModel) | |||
| { | |||
| using (var stringWriter = new StreamWriter(_savePath, false)) | |||
| using(_writer = new HtmlTextWriter(stringWriter)) | |||
| // The first WorkspaceModel that is visited is the root of the tree view | |||
There was a problem hiding this comment.
I don't understand why this code had to change. This PR is making it possible to re-analyze solutions so I wouldn't think this would be a required change.
| var stagingSolutionPaths = GetSolutionPaths(repoSourceStagingPath); | ||
| if (stagingSolutionPaths.Length == 0) | ||
| { | ||
| throw new NoSolutionsFoundException(); |
There was a problem hiding this comment.
I'd prefer if we didn't introduce a custom exception type. Instead, you could consider returning an enum.
There was a problem hiding this comment.
you're saying something like this?
public enum ProcessRepoResult
{
Failed,
NoSolutionsFound,
Successful,
}
internal static ProcessRepoResult ProcessRepo(...There was a problem hiding this comment.
Yup, that looks good to me.
| _writer.RenderEndTag(); | ||
| _writer.WriteLine(); | ||
| _writer.Dispose(); | ||
| _sw.Dispose(); |
There was a problem hiding this comment.
_sw should be null so that it's created again. But I also don't understand why this change is necessary.
There was a problem hiding this comment.
I'll address here your doubts about the changes in the TreeViewTransformer.VisitWorkspace method:
the changes that I made to make the application able to process multiple solutions in the same repository were to process all the .sln files in parallel and when this processing was finished a new WorkspaceModel item was created and it's children were the WorkspaceModel resulted from the solutions.
When the TreeViewTransformer visited the first WorkspaceModel (which is the root) it created the writers and then it invoked the base method, which recursively visited all the sub nodes.
This means that when the transformer visited the solutions' WorkspaceModels under the root, it created again the writers (because projectItem is a WorkspaceModel), and therefore tried to create the same output file, which lead to an IOException: "The process cannot access the file 'treeView.html' because it is being used by another process."
Anyway, what I wrote was more of a hack than a proper fix, so I'd like to use another approach: by restoring the TreeViewTransformer class as it was before of my changes, we can bypass the same problem by adding all the solutions' children to the root node, instead of the solutions' WorkspaceModels.
So, in the UploadRepository.ProcessRepo method, replace
// Add all the results to the root workspace model.
foreach (var workspace in processedWorkspaces)
rootWorkspaceModel.Children.Add(workspace);
// WorkspaceModel (root)
// - WorkspaceModel (sln1)
// - FolderModel
// - ...
// - WorkspaceModel (sln2)
// - FolderModel
// - ...with
foreach (var children in processedWorkspaces.SelectMany(e => e.Children))
rootWorkspaceModel.Children.Add(children);
// WorkspaceModel (root)
// - FolderModel (from sln1's WorkspaceModel)
// - ...
// - FolderModel (from sln2's WorkspaceModel)
// - ...resulting in a single WorkspaceModel.
Please, give me your opinions about this. Thanks!
There was a problem hiding this comment.
Hi,
I tried in implementing the changes that I described above, but in some cases there are problems with repositories with many solutions such that the result is not really good.
Take for example Hangfire: there are two solutions in the root and they share some source files. By flattening the hierarchy in the root WorkplaceModel, the generated treeview is not really readable
I tried to solve this by adding the solutions' children to a new FolderModel instead of directly to the root, and indeed the new treeview was better
but then the navigation didn't work well because the relative paths didn't match the files in the treeview anymore.
I'm thinking that this could be solved by navigating to a solution's treeview when you click on the solution's name, so basically when you go to /Browse/{user}/{repo} the treeview section shows the available solutions.
But before going this path, I need your opinions.
Thank you
Added lock in SolutionAnalayzer to fix the exception thrown by MSBuildWorkspace.Create() when executed concurrently.
When a user submits a repository that already exists, a page is returned where he can choose if he wants to go to the existing source or force the update.
Added a new action "Update" (and a View) in the UploadController.
Refactored some of the ProcessSolution logic into the UploadRepository class.
As suggested in the comments (by Amadeus Wieczorek), added support for multi-solution repository and parallel solution processing.
(Also proposed in issue #114)