Skip to content

Fix Filename sync issue#555

Merged
johnml1135 merged 4 commits intomainfrom
data_file_fix_2
Dec 5, 2024
Merged

Fix Filename sync issue#555
johnml1135 merged 4 commits intomainfrom
data_file_fix_2

Conversation

@johnml1135
Copy link
Collaborator

@johnml1135 johnml1135 commented Dec 4, 2024

Resolves #547
Sync corpora and data files in translation engine
Make DataFile Corpuses only have a link to the DataFile
Fix ParallelCorpus Delete CorpusFile
Use mongoDB strings directly for Mongo array filtering (hacky solution - need to fix later #553).
Broken implementation for MemoryRepository - #554


This change is Reviewable

@johnml1135 johnml1135 requested a review from ddaspit December 4, 2024 02:09
@johnml1135 johnml1135 changed the title Don't store data filename in corpus data structure Fix Filename sync issue Dec 4, 2024
@codecov-commenter
Copy link

codecov-commenter commented Dec 4, 2024

Codecov Report

Attention: Patch coverage is 91.44737% with 26 lines in your changes missing coverage. Please review.

Project coverage is 62.89%. Comparing base (2fa330d) to head (ef0a47a).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...taAccess/src/SIL.DataAccess/MemoryUpdateBuilder.cs 78.72% 2 Missing and 8 partials ⚠️
...ataAccess/src/SIL.DataAccess/MongoUpdateBuilder.cs 88.37% 4 Missing and 1 partial ⚠️
...c/DataAccess/src/SIL.DataAccess/MongoRepository.cs 66.66% 2 Missing and 1 partial ⚠️
src/Serval/src/Serval.Client/Client.g.cs 50.00% 3 Missing ⚠️
...DataAccess/src/SIL.DataAccess/ParameterReplacer.cs 75.00% 1 Missing and 1 partial ⚠️
...val/src/Serval.DataFiles/Services/CorpusService.cs 95.00% 1 Missing and 1 partial ⚠️
...erval/src/Serval.Shared/Contracts/CorpusUpdated.cs 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #555      +/-   ##
==========================================
+ Coverage   61.31%   62.89%   +1.57%     
==========================================
  Files         275      279       +4     
  Lines       13731    13959     +228     
  Branches     1793     1811      +18     
==========================================
+ Hits         8419     8779     +360     
+ Misses       4716     4557     -159     
- Partials      596      623      +27     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Use mongoDB strings directly for Mongo array filtering (hacky solution - need to fix later #553).
Broken implementation for MemoryRepository - #554
Copy link
Collaborator

@Enkidu93 Enkidu93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 25 of 25 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @ddaspit)

Copy link
Contributor

@ddaspit ddaspit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 16 of 25 files at r1, 11 of 13 files at r2, 1 of 1 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @johnml1135)


src/Serval/src/Serval.DataFiles/Services/CorpusService.cs line 6 at r4 (raw file):

    IRepository<Corpus> corpora,
    IDataAccessContext dataAccessContext,
    IDataFileService dataFileService,

We usually just use the repositories directly in entity services.


src/Serval/src/Serval.DataFiles/Services/CorpusService.cs line 47 at r4 (raw file):

                            {
                                TextId = f.TextId!,
                                File = Map(await _dataFileService.GetAsync(f.FileId))

You can retrieve all of the data files in a single query.


src/Serval/src/Serval.DataFiles/Models/CorpusFile.cs line 5 at r4 (raw file):

public record CorpusFile
{
    public required string FileId { get; init; }

The convention when a property is a foreign key reference is to suffix with Ref instead of Id, i.e. FileRef.


src/Serval/src/Serval.DataFiles/Consumers/GetCorpusConsumer.cs line 27 at r4 (raw file):

                        {
                            TextId = f.TextId!,
                            File = Map(await _dataFileService.GetAsync(f.FileId))

You can retrieve all of the data files in a single query.


src/Serval/src/Serval.DataFiles/Contracts/CorpusFileDto.cs line 5 at r4 (raw file):

public record CorpusFileDto
{
    public required string FileId { get; init; }

This should be called File and be a ResourceLinkDto.

Copy link
Collaborator Author

@johnml1135 johnml1135 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 11 of 13 files at r2, 1 of 1 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @johnml1135)

@johnml1135 johnml1135 merged commit f25fc43 into main Dec 5, 2024
@johnml1135 johnml1135 deleted the data_file_fix_2 branch December 5, 2024 20:54
@johnml1135
Copy link
Collaborator Author

Damien made updates to address the concerns he had.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The parallel Corpus database holds onto old Filename when the file is updated

4 participants