Skip to content

Commit

Permalink
Flag for deleting files when deleting corpus (#441)
Browse files Browse the repository at this point in the history
* Flag for deleting files when deleting corpus

* Use mediator; change to kebab case

* Use Send instead of publish; add description to param; fix typo in return code description

* Test delete: false
  • Loading branch information
Enkidu93 authored Jul 29, 2024
1 parent b7b8b68 commit 0e0e9f5
Show file tree
Hide file tree
Showing 10 changed files with 91 additions and 14 deletions.
16 changes: 12 additions & 4 deletions src/Serval/src/Serval.Client/Client.g.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1575,9 +1575,10 @@ public partial interface ITranslationEnginesClient
/// </remarks>
/// <param name="id">The translation engine id</param>
/// <param name="corpusId">The corpus id</param>
/// <returns>The data file was deleted successfully.</returns>
/// <param name="deleteFiles">If true, all files associated with the corpus will be deleted as well (even if they are associated with other corpora). If false, no files will be deleted.</param>
/// <returns>The corpus was deleted successfully.</returns>
/// <exception cref="ServalApiException">A server side error occurred.</exception>
System.Threading.Tasks.Task DeleteCorpusAsync(string id, string corpusId, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken));
System.Threading.Tasks.Task DeleteCorpusAsync(string id, string corpusId, bool? deleteFiles = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken));

/// <param name="cancellationToken">A cancellation token that can be used by other objects or threads to receive notice of cancellation.</param>
/// <summary>
Expand Down Expand Up @@ -3230,9 +3231,10 @@ public string BaseUrl
/// </remarks>
/// <param name="id">The translation engine id</param>
/// <param name="corpusId">The corpus id</param>
/// <returns>The data file was deleted successfully.</returns>
/// <param name="deleteFiles">If true, all files associated with the corpus will be deleted as well (even if they are associated with other corpora). If false, no files will be deleted.</param>
/// <returns>The corpus was deleted successfully.</returns>
/// <exception cref="ServalApiException">A server side error occurred.</exception>
public virtual async System.Threading.Tasks.Task DeleteCorpusAsync(string id, string corpusId, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken))
public virtual async System.Threading.Tasks.Task DeleteCorpusAsync(string id, string corpusId, bool? deleteFiles = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken))
{
if (id == null)
throw new System.ArgumentNullException("id");
Expand All @@ -3255,6 +3257,12 @@ public string BaseUrl
urlBuilder_.Append(System.Uri.EscapeDataString(ConvertToString(id, System.Globalization.CultureInfo.InvariantCulture)));
urlBuilder_.Append("/corpora/");
urlBuilder_.Append(System.Uri.EscapeDataString(ConvertToString(corpusId, System.Globalization.CultureInfo.InvariantCulture)));
urlBuilder_.Append('?');
if (deleteFiles != null)
{
urlBuilder_.Append(System.Uri.EscapeDataString("delete-files")).Append('=').Append(System.Uri.EscapeDataString(ConvertToString(deleteFiles, System.Globalization.CultureInfo.InvariantCulture))).Append('&');
}
urlBuilder_.Length--;

PrepareRequest(client_, request_, urlBuilder_);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ this IMediatorRegistrationConfigurator configurator
)
{
configurator.AddConsumer<GetDataFileConsumer>();
configurator.AddConsumer<DeleteDataFileConsumer>();
return configurator;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
namespace Serval.DataFiles.Consumers;

public class DeleteDataFileConsumer(IDataFileService dataFileService) : IConsumer<DeleteDataFile>
{
private readonly IDataFileService _dataFileService = dataFileService;

public async Task Consume(ConsumeContext<DeleteDataFile> context)
{
await _dataFileService.DeleteAsync(context.Message.DataFileId, context.CancellationToken);
}
}
6 changes: 6 additions & 0 deletions src/Serval/src/Serval.Shared/Contracts/DeleteDataFile.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
namespace Serval.Shared.Contracts;

public record DeleteDataFile
{
public required string DataFileId { get; init; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -478,8 +478,9 @@ CancellationToken cancellationToken
/// </remarks>
/// <param name="id">The translation engine id</param>
/// <param name="corpusId">The corpus id</param>
/// <param name="deleteFiles">If true, all files associated with the corpus will be deleted as well (even if they are associated with other corpora). If false, no files will be deleted.</param>
/// <param name="cancellationToken"></param>
/// <response code="200">The data file was deleted successfully.</response>
/// <response code="200">The corpus was deleted successfully.</response>
/// <response code="401">The client is not authenticated.</response>
/// <response code="403">The authenticated client cannot perform the operation or does not own the translation engine.</response>
/// <response code="404">The engine or corpus does not exist.</response>
Expand All @@ -494,11 +495,12 @@ CancellationToken cancellationToken
public async Task<ActionResult> DeleteCorpusAsync(
[NotNull] string id,
[NotNull] string corpusId,
[FromQuery(Name = "delete-files")] bool? deleteFiles,
CancellationToken cancellationToken
)
{
await AuthorizeAsync(id, cancellationToken);
await _engineService.DeleteCorpusAsync(id, corpusId, cancellationToken);
await _engineService.DeleteCorpusAsync(id, corpusId, deleteFiles ?? false, cancellationToken);
return Ok();
}

Expand Down
26 changes: 23 additions & 3 deletions src/Serval/src/Serval.Translation/Services/EngineService.cs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
using Serval.Translation.V1;
using MassTransit.Mediator;
using Serval.Translation.V1;

namespace Serval.Translation.Services;

public class EngineService(
IRepository<Engine> engines,
IRepository<Build> builds,
IRepository<Pretranslation> pretranslations,
IScopedMediator mediator,
GrpcClientFactory grpcClientFactory,
IOptionsMonitor<DataFileOptions> dataFileOptions,
IDataAccessContext dataAccessContext,
Expand All @@ -15,6 +17,7 @@ IScriptureDataFileService scriptureDataFileService
{
private readonly IRepository<Build> _builds = builds;
private readonly IRepository<Pretranslation> _pretranslations = pretranslations;
private readonly IScopedMediator _mediator = mediator;
private readonly GrpcClientFactory _grpcClientFactory = grpcClientFactory;
private readonly IOptionsMonitor<DataFileOptions> _dataFileOptions = dataFileOptions;
private readonly IDataAccessContext _dataAccessContext = dataAccessContext;
Expand Down Expand Up @@ -437,12 +440,18 @@ public Task AddCorpusAsync(string engineId, Models.Corpus corpus, CancellationTo
return engine.Corpora.First(c => c.Id == corpusId);
}

public async Task DeleteCorpusAsync(string engineId, string corpusId, CancellationToken cancellationToken = default)
public async Task DeleteCorpusAsync(
string engineId,
string corpusId,
bool deleteFiles,
CancellationToken cancellationToken = default
)
{
Engine? originalEngine = null;
await _dataAccessContext.WithTransactionAsync(
async (ct) =>
{
Engine? originalEngine = await Entities.UpdateAsync(
originalEngine = await Entities.UpdateAsync(
engineId,
u => u.RemoveAll(e => e.Corpora, c => c.Id == corpusId),
returnOriginal: true,
Expand All @@ -458,6 +467,17 @@ await _dataAccessContext.WithTransactionAsync(
},
cancellationToken: cancellationToken
);
if (deleteFiles && originalEngine != null)
{
foreach (
string id in originalEngine.Corpora.SelectMany(c =>
c.TargetFiles.Select(f => f.Id).Concat(c.SourceFiles.Select(f => f.Id).Distinct())
)
)
{
await _mediator.Send<DeleteDataFile>(new { DataFileId = id }, cancellationToken);
}
}
}

public Task DeleteAllCorpusFilesAsync(string dataFileId, CancellationToken cancellationToken = default)
Expand Down
7 changes: 6 additions & 1 deletion src/Serval/src/Serval.Translation/Services/IEngineService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,12 @@ Task<Corpus> UpdateCorpusAsync(
IReadOnlyList<CorpusFile>? targetFiles,
CancellationToken cancellationToken = default
);
Task DeleteCorpusAsync(string engineId, string corpusId, CancellationToken cancellationToken = default);
Task DeleteCorpusAsync(
string engineId,
string corpusId,
bool deleteFiles,
CancellationToken cancellationToken = default
);

Task DeleteAllCorpusFilesAsync(string dataFileId, CancellationToken cancellationToken = default);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -755,15 +755,15 @@ string engineId
{
case 200:
TranslationCorpus result = await client.AddCorpusAsync(engineId, TestCorpusConfig);
await client.DeleteCorpusAsync(engineId, result.Id);
await client.DeleteCorpusAsync(engineId, result.Id, deleteFiles: false);
ICollection<TranslationCorpus> resultsAfterDelete = await client.GetAllCorporaAsync(engineId);
Assert.That(resultsAfterDelete, Has.Count.EqualTo(0));
break;
case 403:
case 404:
ServalApiException? ex = Assert.ThrowsAsync<ServalApiException>(async () =>
{
await client.DeleteCorpusAsync(engineId, DOES_NOT_EXIST_CORPUS_ID);
await client.DeleteCorpusAsync(engineId, DOES_NOT_EXIST_CORPUS_ID, deleteFiles: false);
});
Assert.That(ex?.StatusCode, Is.EqualTo(expectedStatusCode));
break;
Expand All @@ -774,6 +774,28 @@ string engineId
}
}

[Test]
public async Task DeleteCorpusAndFilesAsync()
{
TranslationEnginesClient client = _env.CreateTranslationEnginesClient();
TranslationCorpus result = await client.AddCorpusAsync(ECHO_ENGINE1_ID, TestCorpusConfig);
await client.DeleteCorpusAsync(ECHO_ENGINE1_ID, result.Id, deleteFiles: true);
ICollection<TranslationCorpus> resultsAfterDelete = await client.GetAllCorporaAsync(ECHO_ENGINE1_ID);
Assert.That(resultsAfterDelete, Has.Count.EqualTo(0));
Assert.That(await _env.DataFiles.GetAllAsync(), Has.Count.EqualTo(2)); //Paratext files still exist
}

[Test]
public async Task DeleteCorpusButNotFilesAsync()
{
TranslationEnginesClient client = _env.CreateTranslationEnginesClient();
TranslationCorpus result = await client.AddCorpusAsync(ECHO_ENGINE1_ID, TestCorpusConfig);
await client.DeleteCorpusAsync(ECHO_ENGINE1_ID, result.Id, deleteFiles: false);
ICollection<TranslationCorpus> resultsAfterDelete = await client.GetAllCorporaAsync(ECHO_ENGINE1_ID);
Assert.That(resultsAfterDelete, Has.Count.EqualTo(0));
Assert.That(await _env.DataFiles.GetAllAsync(), Has.Count.EqualTo(4)); //Paratext & Text files still exist
}

[Test]
public async Task GetAllPretranslationsAsync_Exists()
{
Expand Down
4 changes: 2 additions & 2 deletions src/Serval/test/Serval.E2ETests/ServalApiTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -259,9 +259,9 @@ public async Task CircuitousRouteGetWordGraphAsync()
await _helperClient.BuildEngineAsync(smtEngineId);

//Remove added corpus (shouldn't affect translation)
await _helperClient.TranslationEnginesClient.DeleteCorpusAsync(smtEngineId, cId);
await _helperClient.TranslationEnginesClient.DeleteCorpusAsync(smtEngineId, cId, deleteFiles: false);

//Add corpus
// Add corpus
await _helperClient.AddTextCorpusToEngineAsync(
smtEngineId,
["1JN.txt", "2JN.txt", "3JN.txt"],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Google.Protobuf.WellKnownTypes;
using MassTransit.Mediator;
using Serval.Translation.V1;

namespace Serval.Translation.Services;
Expand Down Expand Up @@ -626,6 +627,7 @@ public TestEnvironment()
Engines,
new MemoryRepository<Build>(),
new MemoryRepository<Pretranslation>(),
Substitute.For<IScopedMediator>(),
grpcClientFactory,
dataFileOptions,
new MemoryDataAccessContext(),
Expand Down

0 comments on commit 0e0e9f5

Please sign in to comment.