Skip to content

Commit

Permalink
Port edit documentation functionality to gallery and cleanup code rel…
Browse files Browse the repository at this point in the history 
…ated to edit package (#5259)

* Move edit readme functionality to gallery

* Remove PackageEdit entity

* `ReadMeService` now properly saves `package.HasReadMe` to db

* Only audit package edit when readme actually changed

* Revert "Remove PackageEdit entity"

This reverts commit 82f6762.

* Removed dead code
  • Loading branch information
@xavierdecoster
xavierdecoster committed Jan 12, 2018
1 parent b059181 commit 1246665
Show file tree
Hide file tree
Showing 15 changed files with 136 additions and 501 deletions.
6 changes: 1 addition & 5 deletions src/NuGetGallery/App_Start/DefaultDependenciesModule.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,7 @@ protected override void Load(ContainerBuilder builder)
.AsSelf()
.As<IDeleteAccountService>()
.InstancePerLifetimeScope();

builder.RegisterType<EditPackageService>()
.AsSelf()
.InstancePerLifetimeScope();


builder.RegisterType<PackageOwnerRequestService>()
.AsSelf()
.As<IPackageOwnerRequestService>()
Expand Down
90 changes: 17 additions & 73 deletions src/NuGetGallery/Controllers/PackagesController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System;
using System.Collections.Generic;
using System.Data;
using System.Data.Entity;
using System.Diagnostics;
using System.Globalization;
using System.IO;
Expand Down Expand Up @@ -75,7 +74,6 @@ public partial class PackagesController
private readonly IEntitiesContext _entitiesContext;
private readonly IIndexingService _indexingService;
private readonly ICacheService _cacheService;
private readonly EditPackageService _editPackageService;
private readonly IPackageDeleteService _packageDeleteService;
private readonly ISupportRequestService _supportRequestService;
private readonly IAuditingService _auditingService;
Expand All @@ -99,7 +97,6 @@ public PackagesController(
IAppConfiguration config,
IIndexingService indexingService,
ICacheService cacheService,
EditPackageService editPackageService,
IPackageDeleteService packageDeleteService,
ISupportRequestService supportRequestService,
IAuditingService auditingService,
Expand All @@ -122,7 +119,6 @@ public PackagesController(
_config = config;
_indexingService = indexingService;
_cacheService = cacheService;
_editPackageService = editPackageService;
_packageDeleteService = packageDeleteService;
_supportRequestService = supportRequestService;
_auditingService = auditingService;
Expand Down Expand Up @@ -325,7 +321,7 @@ public virtual async Task<JsonResult> UploadPackage(HttpPostedFileBase uploadFil
var id = nuspec.GetId();
var existingPackageRegistration = _packageService.FindPackageRegistrationById(id);
// For a new package id verify if the user is allowed to use it.
if (existingPackageRegistration == null &&
if (existingPackageRegistration == null &&
ActionsRequiringPermissions.UploadNewPackageId.CheckPermissionsOnBehalfOfAnyAccount(
currentUser, new ActionOnNewPackageContext(id, _reservedNamespaceService)) != PermissionsCheckResult.Allowed)
{
Expand All @@ -337,7 +333,7 @@ public virtual async Task<JsonResult> UploadPackage(HttpPostedFileBase uploadFil

return Json(409, new string[] { string.Format(CultureInfo.CurrentCulture, Strings.UploadPackage_IdNamespaceConflict) });
}

// For existing package id verify if it is owned by the current user
if (existingPackageRegistration != null)
{
Expand Down Expand Up @@ -468,25 +464,7 @@ public virtual async Task<ActionResult> DisplayPackage(string id, string version
.Distinct()
.ToList();

var isReadMePending = false;
if (ActionsRequiringPermissions.EditPackage.CheckPermissionsOnBehalfOfAnyAccount(currentUser, package) == PermissionsCheckResult.Allowed)
{
// Tell logged-in package owners not to cache the package page,
// so they won't be confused about the state of pending edits.
Response.Cache.SetCacheability(HttpCacheability.NoCache);
Response.Cache.SetNoStore();
Response.Cache.SetMaxAge(TimeSpan.Zero);
Response.Cache.SetRevalidation(HttpCacheRevalidation.AllCaches);

var pendingMetadata = _editPackageService.GetPendingMetadata(package);
if (pendingMetadata != null)
{
model.SetPendingMetadata(pendingMetadata);
isReadMePending = pendingMetadata.ReadMeState != PackageEditReadMeState.Unchanged;
}
}

model.ReadMeHtml = await _readMeService.GetReadMeHtmlAsync(package, isReadMePending);
model.ReadMeHtml = await _readMeService.GetReadMeHtmlAsync(package);

var externalSearchService = _searchService as ExternalSearchService;
if (_searchService.ContainsAllVersions && externalSearchService != null)
Expand Down Expand Up @@ -674,7 +652,7 @@ public virtual async Task<ActionResult> ReportMyPackage(string id, string versio
{
return HttpNotFound();
}

if (ActionsRequiringPermissions.ReportPackageAsOwner.CheckPermissionsOnBehalfOfAnyAccount(GetCurrentUser(), package) != PermissionsCheckResult.Allowed)
{
return RedirectToAction(nameof(ReportAbuse), new { id, version });
Expand Down Expand Up @@ -702,7 +680,7 @@ public virtual async Task<ActionResult> ReportMyPackage(string id, string versio
public virtual async Task<ActionResult> ReportAbuse(string id, string version, ReportAbuseViewModel reportForm)
{
reportForm.Message = HttpUtility.HtmlEncode(reportForm.Message);

if (reportForm.Reason == ReportPackageReason.ViolatesALicenseIOwn
&& string.IsNullOrWhiteSpace(reportForm.Signature))
{
Expand Down Expand Up @@ -767,13 +745,13 @@ public virtual async Task<ActionResult> ReportAbuse(string id, string version, R
public virtual async Task<ActionResult> ReportMyPackage(string id, string version, ReportMyPackageViewModel reportForm)
{
var package = _packageService.FindPackageByIdAndVersionStrict(id, version);

var failureResult = await ValidateReportMyPackageViewModel(reportForm, package);
if (failureResult != null)
{
return failureResult;
}

// Override the copy sender and message fields if we are performing an auto-delete.
if (reportForm.DeleteDecision == PackageDeleteDecision.DeletePackage)
{
Expand Down Expand Up @@ -1215,17 +1193,13 @@ public virtual async Task<ActionResult> Edit(string id, string version)
url = UrlExtensions.EditPackage(Url, model.PackageId, e.NormalizedVersion)
}), "url", "text", UrlExtensions.EditPackage(Url, model.PackageId, model.Version));

// Create edit model from the latest pending edit.
var pendingMetadata = _editPackageService.GetPendingMetadata(package);

model.Edit = new EditPackageVersionReadMeRequest(pendingMetadata);
model.Edit = new EditPackageVersionReadMeRequest();

// Update edit model with the active or pending readme.md data.
var isReadMePending = model.Edit.ReadMeState != PackageEditReadMeState.Unchanged;
if (package.HasReadMe || isReadMePending)
// Update edit model with the readme.md data.
if (package.HasReadMe)
{
model.Edit.ReadMe.SourceType = ReadMeService.TypeWritten;
model.Edit.ReadMe.SourceText = await _readMeService.GetReadMeMdAsync(package, isReadMePending);
model.Edit.ReadMe.SourceText = await _readMeService.GetReadMeMdAsync(package);
}
}

Expand Down Expand Up @@ -1263,29 +1237,15 @@ public virtual async Task<JsonResult> Edit(string id, string version, VerifyPack

if (formData.Edit != null)
{
try
// Update readme.md file, if modified.
var readmeChanged = await _readMeService.SaveReadMeMdIfChanged(package, formData.Edit, Request.ContentEncoding);
if (readmeChanged)
{
// Update pending readme.md file, if modified.
var hasReadMe = await _readMeService.SavePendingReadMeMdIfChanged(package, formData.Edit, Request.ContentEncoding);
if (hasReadMe)
{
_telemetryService.TrackPackageReadMeChangeEvent(package, formData.Edit.ReadMe.SourceType, formData.Edit.ReadMeState);
}

// Queue package edit in database for processing in background (HandlePackageEdits job).
var user = GetCurrentUser();
_editPackageService.StartEditPackageRequest(package, formData.Edit, user);
await _entitiesContext.SaveChangesAsync();
_telemetryService.TrackPackageReadMeChangeEvent(package, formData.Edit.ReadMe.SourceType, formData.Edit.ReadMeState);

// Add an auditing record for the package edit. HasReadMe flag is updated in DB by background job.
package.HasReadMe = hasReadMe;
// Add an auditing record for the package edit.
await _auditingService.SaveAuditRecordAsync(new PackageAuditRecord(package, AuditedPackageAction.Edit));
}
catch (EntityException ex)
{
ModelState.AddModelError("Edit.VersionTitle", ex.Message);
return Json(400, new[] { ex.Message });
}
}

return Json(new
Expand Down Expand Up @@ -1677,32 +1637,16 @@ public virtual async Task<JsonResult> VerifyPackage(VerifyPackageRequest formDat
return Json(400, new[] { ex.Message });
}

var pendEdit = false;
if (formData.Edit != null)
{
if (await _readMeService.SavePendingReadMeMdIfChanged(package, formData.Edit, Request.ContentEncoding))
if (await _readMeService.SaveReadMeMdIfChanged(package, formData.Edit, Request.ContentEncoding))
{
pendEdit = true;
_telemetryService.TrackPackageReadMeChangeEvent(package, formData.Edit.ReadMe.SourceType, formData.Edit.ReadMeState);
}
}

await _packageService.PublishPackageAsync(package, commitChanges: false);

if (pendEdit)
{
try
{
_editPackageService.StartEditPackageRequest(package, formData.Edit, currentUser);
}
catch (EntityException ex)
{
_telemetryService.TraceException(ex);

return Json(400, new[] { ex.Message });
}
}

if (!formData.Listed)
{
await _packageService.MarkPackageUnlistedAsync(package, commitChanges: false);
Expand Down
1 change: 0 additions & 1 deletion src/NuGetGallery/NuGetGallery.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1794,7 +1794,6 @@
<Compile Include="Services\StatisticsReportResult.cs" />
<Compile Include="Services\StatusService.cs" />
<Compile Include="Services\UploadFileService.cs" />
<Compile Include="Services\EditPackageService.cs" />
<Compile Include="ViewModels\AggregateStats.cs" />
<Compile Include="ViewModels\ContactSupportViewModel.cs" />
<Compile Include="ViewModels\DeletePackageViewModel.cs" />
Expand Down
6 changes: 2 additions & 4 deletions src/NuGetGallery/RequestModels/EditPackageVersionRequest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,9 @@ public EditPackageVersionReadMeRequest()
ReadMe = new ReadMeRequest();
}

public EditPackageVersionReadMeRequest(PackageEdit pendingMetadata)
public EditPackageVersionReadMeRequest(PackageEditReadMeState readMeState)
{
var metadata = pendingMetadata ?? new PackageEdit();

ReadMeState = metadata.ReadMeState;
ReadMeState = readMeState;

ReadMe = new ReadMeRequest();
}
Expand Down
64 changes: 0 additions & 64 deletions src/NuGetGallery/Services/EditPackageService.cs
View file

This file was deleted.

8 changes: 3 additions & 5 deletions src/NuGetGallery/Services/IPackageFileService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,19 @@ public interface IPackageFileService : ICorePackageFileService
/// Deletes the package readme.md file from storage.
/// </summary>
/// <param name="package">The package associated with the readme.</param>
/// <param name="isPending">True to delete pending blob, false for active.</param>
Task DeleteReadMeMdFileAsync(Package package, bool isPending = false);
Task DeleteReadMeMdFileAsync(Package package);

/// <summary>
/// Saves the (pending) package readme.md file to storage.
/// </summary>
/// <param name="package">The package associated with the readme.</param>
/// <param name="readMeMd">Markdown content.</param>
Task SavePendingReadMeMdFileAsync(Package package, string readMeMd);
Task SaveReadMeMdFileAsync(Package package, string readMeMd);

/// <summary>
/// Downloads the readme.md from storage.
/// </summary>
/// <param name="package">The package associated with the readme.</param>
/// <param name="isPending">True to download the pending blob, false for active.</param>
Task<string> DownloadReadMeMdFileAsync(Package package, bool isPending = false);
Task<string> DownloadReadMeMdFileAsync(Package package);
}
}
18 changes: 8 additions & 10 deletions src/NuGetGallery/Services/IReadMeService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,24 +26,22 @@ public interface IReadMeService
/// Get the converted HTML from the stored ReadMe markdown.
/// </summary>
/// <param name="package">Package entity associated with the ReadMe.</param>
/// <param name="isPending">Whether to retrieve the pending ReadMe.</param>
/// <returns>Pending or active ReadMe converted to HTML.</returns>
Task<string> GetReadMeHtmlAsync(Package package, bool isPending = false);
/// <returns>ReadMe converted to HTML.</returns>
Task<string> GetReadMeHtmlAsync(Package package);

/// <summary>
/// Get package ReadMe markdown from storage.
/// </summary>
/// <param name="package">Package entity associated with the ReadMe.</param>
/// <param name="isPending">Whether to retrieve the pending ReadMe.</param>
/// <returns>Pending or active ReadMe markdown from storage.</returns>
Task<string> GetReadMeMdAsync(Package package, bool isPending = false);
/// <returns>ReadMe markdown from storage.</returns>
Task<string> GetReadMeMdAsync(Package package);

/// <summary>
/// Save a pending ReadMe if changes are detected.
/// Save a ReadMe if changes are detected.
/// </summary>
/// <param name="package">Package entity associated with the ReadMe.</param>
/// <param name="edit">Package edit entity.</param>
/// <returns>True if a ReadMe is pending, false otherwise.</returns>
Task<bool> SavePendingReadMeMdIfChanged(Package package, EditPackageVersionReadMeRequest edit, Encoding encoding);
/// <param name="edit">Package version edit readme request.</param>
/// <returns>True if the package readme changed, otherwise false.</returns>
Task<bool> SaveReadMeMdIfChanged(Package package, EditPackageVersionReadMeRequest edit, Encoding encoding);
}
}
12 changes: 5 additions & 7 deletions src/NuGetGallery/Services/PackageDeleteService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,21 +304,19 @@ private async Task BackupPackageBinaries(IEnumerable<Package> packages)
await _packageFileService.DeletePackageFileAsync(id, version);
await _packageFileService.DeleteValidationPackageFileAsync(id, version);

// Delete any active or pending readme files for this package.
await TryDeleteReadMeMdFile(package, false);
await TryDeleteReadMeMdFile(package, true);
// Delete readme file for this package.
await TryDeleteReadMeMdFile(package);
}
}

/// <summary>
/// Delete package readme.md file, if it exists. Doing a force delete here
/// rather than checking the HasReadMe (active) flag or PackageEdits (pending).
/// Delete package readme.md file, if it exists.
/// </summary>
private async Task TryDeleteReadMeMdFile(Package package, bool isPending)
private async Task TryDeleteReadMeMdFile(Package package)
{
try
{
await _packageFileService.DeleteReadMeMdFileAsync(package, isPending: isPending);
await _packageFileService.DeleteReadMeMdFileAsync(package);
}
catch (StorageException) { }
}
Expand Down
Loading

0 comments on commit 1246665

Please sign in to comment.