SF-3819 Add support for draft rate limiting

[pmachapman]

This PR adds support for customizable rate limiting for starting Serval draft builds.

If have marked this as testing not required, as to use the rate limiting feature, the database must be modified. As such, it should be tested by a developer.

---

This change is 

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.06%. Comparing base (0ab7def) to head (cc909d3).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3974      +/-   ##
==========================================
+ Coverage   81.04%   81.06%   +0.01%     
==========================================
  Files         645      646       +1     
  Lines       41434    41476      +42     
  Branches     6724     6752      +28     
==========================================
+ Hits        33581    33623      +42     
  Misses       6760     6760              
  Partials     1093     1093              

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

[marksvc]

This is reviewable in Devin Review.

[marksvc]

@marksvc reviewed 10 files and all commit messages, and made 6 comments.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on pmachapman).

---

src/SIL.XForge/Models/SiteConfig.cs line 41 at r2 (raw file):

    /// to get total number of hours in the quota period.
    /// </remarks>
    public int BuildQuotaPeriod { get; set; }

Can you tell me some more about the thinking about implementing the build quota timeframes and checking in this way? I've found it to seem a bit awkward and I may not be recalling the right thinking on design or what is needed.

I might have thought we would implement it more like the following. Is that something you were considering as well but found not to be as preferable?

  public int BuildQuotaPerPeriod { get; set; } // eg 12
  public int BuildPeriodHours { get; set; } // eg 24

where we would then be checking if a project is over the quota by considering

count = eventMetrics.countOf(project builds that started in the last BuildPeriodHours hours);
if (count > BuildQuotaPerPeriod)
  throw

A comment in the Jira issue discusses multiple time periods that could be simultaneously limited. So another implementation, that considers multiple timeframes, could be something like

public int BuildQuotaPerDay
public int BuildQuotaPerWeek
public int BuildQuotaPerMonth

and then we would check if a project is over quota by considering

countInDay = eventMetrics.countOf(project builds that started in the last 24 hours);
countInWeek = eventMetrics.countOf(project builds that started in the last 24*7 hours);
countInMonth = eventMetrics.countOf(project builds that started in the last 24*30 hours);
if (countInDay > BuildQuotaPerDay ||
    countInWeek > BuildQuotaPerWeek ||
    countInMonth > BuildQuotaPerMonth)
  throw

I'm at the beginning of reviewing, so by the end I may convinced of usingBuildQuotaPeriod * BuildQuotaPeriodUnit.

---

src/SIL.XForge/Models/SiteConfig.cs line 46 at r2 (raw file):

    /// The quota period unit, stored as a number of hours corresponding to hour, day, week, month, and year.
    /// </summary>
    public QuotaPeriod BuildQuotaPeriodUnit { get; set; }

It appears that BuildQuotaPeriodUnit isn't really interacted with as an enum, but rather as a number. And so usages of it are then made a bit more awkward with serialization or casting. I think if the current implementation of having both BuildQuotaPeriod and BuildQuotaPeriodUnit is desired to move forward with, we could consider removing what may be unnecessarily complication by defining BuildQuotaPeriodUnit as an int rather than as the enum QuotaPeriod type. But perhaps there is more planned that will be using the enum type that I am not foreseeing. And if BuildQuotaPeriodUnit is made to be an int, a preferable name for the property might be BuildQuotaPeriodHours or BuildQuotaPeriodBaseHours.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.service.ts line 282 at r2 (raw file):

          .pipe(
            // No error means build successfully started, so start polling
            switchMap(() => this.pollBuildProgress(buildConfig.projectId))

If the above catchError shows a notice about an error, and we then go into this switchMap and pollBuildProgress, might we get into getBuildProgress and show yet another notice, saying temporarily_unavailable? And so show two notices when unavailable? I'll guess I'll see as soon as I test it.

Some of the mocking in the spec may be hiding some of this behaviour, for example making getBuildProgress return of(undefined) rather than get into its body where it may call noticeService.showError.

Okay I did some debugging and experimenting. In practice, it looks like both MachineApiController.GetBuildAsync and MachineApiController.StartPreTranslationBuildAsync would both need to fail, and with something like a 503 in order for getBuildProgress to also show a message. (Just being over quota is not enough.) And if it did, the result it to show a snackbar and immediately replace it with another. So, it probably isn't your intent that we go into pollBuildProgress even if we went into catchError, but it isn't being a big deal if we do.

---

src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json line 224 at r2 (raw file):

    "preview_last_draft_detail": "The last draft build did not complete, but you can still view the most recent successful draft.",
    "preview_last_draft_header": "Preview last draft",
    "quota_exceeded": "Generating drafts is temporarily unavailable for this project as your quota has been exceeded.",

I wonder if a better message might be to communicate that the quota for the project has been exceeded. For example,
"Generating drafts is temporarily unavailable for this project as its quota has been exceeded."

---

src/SIL.XForge.Scripture/Services/MachineApiService.cs line 2775 at r2 (raw file):

        );
    }

In my testing, I found that sometimes the
buildQuotaLimit,
buildQuotaPeriod, and
buildQuotaPeriodUnit fields in the DB would be all set to 0.

Yeah, try this:

• Set values to 0.
• Generate a draft with project A
• Set values all to 1.
• Generate a draft with project B
• The values are now 0.

---

test/SIL.XForge.Scripture.Tests/Services/MachineApiServiceTests.cs line 5562 at r2 (raw file):

                new SiteOptions
                {
                    Id = "SF",

It looks like from appsettings.json that the value used in the running application would be lower-case "sf", if that would be significant to use here.

Oh I guess it doesn't matter a ton if the SiteConfig Name below is being set to the value here.