diff --git a/extension.json b/extension.json index af7f7a5..2b42952 100644 --- a/extension.json +++ b/extension.json @@ -65,6 +65,8 @@ "ext.SimpleBatchUpload/batchLimit.js", "ext.SimpleBatchUpload/rateLimitGate.js", "ext.SimpleBatchUpload/rateLimits.js", + "ext.SimpleBatchUpload/estimateRow.js", + "ext.SimpleBatchUpload/remainingTime.js", "ext.SimpleBatchUpload/renamePattern.js", "ext.SimpleBatchUpload/resultRow.js", "ext.SimpleBatchUpload/uploadQueue.js", @@ -82,6 +84,8 @@ ], "messages": [ "simplebatchupload-error-rename-pattern", + "simplebatchupload-estimate-minutes", + "simplebatchupload-estimate-under-a-minute", "simplebatchupload-max-files-reached", "simplebatchupload-rename-label", "simplebatchupload-result-error", diff --git a/i18n/en.json b/i18n/en.json index acdcb73..ae26375 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -10,6 +10,8 @@ "simplebatchupload-comment": "Uploaded with [[mw:Special:MyLanguage/Extension:SimpleBatchUpload|SimpleBatchUpload]]", "simplebatchupload-desc": "Allows for simple batch uploading of of files", "simplebatchupload-error-rename-pattern": "The +rename pattern is not a valid regular expression.", + "simplebatchupload-estimate-minutes": "About $1 {{PLURAL:$1|minute|minutes}} left at this wiki's upload rate limit.", + "simplebatchupload-estimate-under-a-minute": "Less than a minute left at this wiki's upload rate limit.", "simplebatchupload-max-files-reached": "Added $1 of $2 selected {{PLURAL:$2|file|files}}. No more can be added until the current uploads finish.", "simplebatchupload-name": "SimpleBatchUpload", "simplebatchupload-rename-label": "$1 → $2", diff --git a/i18n/qqq.json b/i18n/qqq.json index dd67552..df1a286 100644 --- a/i18n/qqq.json +++ b/i18n/qqq.json @@ -10,6 +10,8 @@ "simplebatchupload-comment": "Comment saved with the upload. Do not translate '[[mw:Special:MyLanguage/Extension:SimpleBatchUpload|SimpleBatchUpload]]'", "simplebatchupload-desc": "{{desc|name=SimpleBatchUpload|url=https://www.mediawiki.org/wiki/Extension:SimpleBatchUpload}}", "simplebatchupload-error-rename-pattern": "Reason shown when the +rename directive in the upload description is not a valid regular expression. Do not translate '+rename'. Used as $1 of {{msg-mw|simplebatchupload-result-error}}.", + "simplebatchupload-estimate-minutes": "Shown above the result list while the wiki is rate limiting the batch. Parameters:\n* $1 - a whole number of minutes, rounded up", + "simplebatchupload-estimate-under-a-minute": "Shown above the result list while the wiki is rate limiting the batch and less than a minute of waiting is left. Used instead of {{msg-mw|simplebatchupload-estimate-minutes}} so that no countdown in seconds is shown.", "simplebatchupload-max-files-reached": "Shown once in the result list when a file selection does not fit in the current batch. Parameters:\n* $1 - the number of files that were added\n* $2 - the number of files that were selected", "simplebatchupload-name": "{{Notranslate}} The name of the extesion as shown on special page 'Version'", "simplebatchupload-rename-label": "Label of a result row for a file that is renamed while uploading. Parameters:\n* $1 - the name of the selected file\n* $2 - the name it is uploaded under", diff --git a/release-notes.md b/release-notes.md index f70fbce..13fde26 100644 --- a/release-notes.md +++ b/release-notes.md @@ -8,6 +8,7 @@ * Changed uploading to pace itself to the rate limit the wiki advertises, once the wiki has refused an upload * Batches that fit inside the limit are unaffected and still upload at full speed * Batches on wikis with a long limit window now retry for much longer before giving up +* Added an estimate of how much longer a batch has left while the wiki is rate limiting it * Fixed files reported as uploaded when the wiki did not store them * Fixed an invalid `+rename` pattern cancelling the rest of the batch with no error shown * Fixed the result list losing uploads that were still running when more files were selected diff --git a/res/ext.SimpleBatchUpload/batchLimit.js b/res/ext.SimpleBatchUpload/batchLimit.js index b4bcb2c..35ef0e8 100644 --- a/res/ext.SimpleBatchUpload/batchLimit.js +++ b/res/ext.SimpleBatchUpload/batchLimit.js @@ -50,6 +50,9 @@ function createBatchLimit( limit ) { admit: admit, release: release, remaining: () => Math.max( 0, limit - active ), + // Admitted but not finished. Not queue.running() + queue.waiting(): a + // file being retried has left the queue and not yet rejoined it. + active: () => active, limit: () => limit }; } diff --git a/res/ext.SimpleBatchUpload/estimateRow.js b/res/ext.SimpleBatchUpload/estimateRow.js new file mode 100644 index 0000000..315d514 --- /dev/null +++ b/res/ext.SimpleBatchUpload/estimateRow.js @@ -0,0 +1,46 @@ +'use strict'; + +/** + * The one row that says how much longer the wiki's rate limit will hold the + * batch up. Kept above the file rows, and taken away when there is nothing to + * say. + */ + +/** + * @param {HTMLElement} list The ul.fileupload-results element + * @param {?string} text Null to take the row away + */ +function showEstimate( list, text ) { + let row = list.querySelector( 'li.ful-estimate' ); + + if ( !text ) { + if ( row ) { + row.remove(); + } + + return; + } + + if ( !row ) { + row = document.createElement( 'li' ); + row.className = 'ful-estimate'; + + // The live region is a child rather than the
  • itself: role="status" + // on the
  • would replace its listitem role, and the list would + // announce one fewer item than it has. + const region = document.createElement( 'span' ); + region.setAttribute( 'role', 'status' ); + row.appendChild( region ); + + list.insertBefore( row, list.firstChild ); + } + + const announced = row.firstChild; + + // Rewriting an unchanged live region announces it again. + if ( announced.textContent !== text ) { + announced.textContent = text; + } +} + +module.exports = { showEstimate: showEstimate }; diff --git a/res/ext.SimpleBatchUpload/ext.SimpleBatchUpload.css b/res/ext.SimpleBatchUpload/ext.SimpleBatchUpload.css index 49a20cc..0d67b35 100644 --- a/res/ext.SimpleBatchUpload/ext.SimpleBatchUpload.css +++ b/res/ext.SimpleBatchUpload/ext.SimpleBatchUpload.css @@ -42,3 +42,7 @@ ul.fileupload-results li.ful-error { ul.fileupload-results li.ful-notice { background-color: #fafad2; } + +ul.fileupload-results li.ful-estimate { + font-style: italic; +} diff --git a/res/ext.SimpleBatchUpload/ext.SimpleBatchUpload.js b/res/ext.SimpleBatchUpload/ext.SimpleBatchUpload.js index db0e995..c4a2564 100644 --- a/res/ext.SimpleBatchUpload/ext.SimpleBatchUpload.js +++ b/res/ext.SimpleBatchUpload/ext.SimpleBatchUpload.js @@ -18,6 +18,8 @@ const { createUploadQueue } = require( './uploadQueue.js' ); const { createUploadRunner } = require( './uploadRunner.js' ); const { createResultRow, pruneFinishedRows } = require( './resultRow.js' ); const { filePageUrl } = require( './uploadResult.js' ); +const { estimateRemainingMs, describeRemaining } = require( './remainingTime.js' ); +const { showEstimate } = require( './estimateRow.js' ); // The rate limit is per user, so one gate and one queue serve every widget on // the page. blueimp's own limit is set to the same number as a backstop. @@ -44,6 +46,24 @@ $( () => { mw.config.get( 'wgUserGroups' ) ) ); + const resultLists = []; + + /** + * Shows how much longer the wiki's rate limit will hold the batch up. + * + * Refreshed where its inputs change -- a file admitted, an upload refused, + * a file finished -- and never on a timer. + */ + function refreshEstimate() { + const text = describeRemaining( + estimateRemainingMs( batchLimit.active(), gate.schedule() ) + ); + + // Gate, queue and batch limit are page-wide, so every widget shows the + // same figure. + resultLists.forEach( ( results ) => showEstimate( results, text ) ); + } + function appendNotice( results, text ) { const notice = document.createElement( 'li' ); notice.className = 'ful-notice'; @@ -96,7 +116,10 @@ $( () => { const outcome = await runner.run( () => data.submit(), - () => row.showWaiting(), + () => { + row.showWaiting(); + refreshEstimate(); + }, async () => { api.badToken( 'csrf' ); data.formData.token = await api.getToken( 'csrf' ); @@ -112,11 +135,13 @@ $( () => { } } finally { batchLimit.release(); + refreshEstimate(); } } function initContainer( container ) { const results = container.querySelector( 'ul.fileupload-results' ); + resultLists.push( results ); // blueimp calls add() once per file and hands every file of one // selection the same originalFiles array, which is how a new selection @@ -157,6 +182,7 @@ $( () => { } admitted += 1; + refreshEstimate(); startUpload( this, container, results, data ); }, diff --git a/res/ext.SimpleBatchUpload/rateLimitGate.js b/res/ext.SimpleBatchUpload/rateLimitGate.js index 35cd288..7477d10 100644 --- a/res/ext.SimpleBatchUpload/rateLimitGate.js +++ b/res/ext.SimpleBatchUpload/rateLimitGate.js @@ -81,6 +81,13 @@ function createRateLimitGate( options ) { let nextReleaseAt = 0; let lastRefusalAt = 0; + /** + * @return {boolean} True once a full window has passed with nothing refused + */ + function refusalsAreStale() { + return pacing && now() - lastRefusalAt >= capMs; + } + /** * Stops pacing once the wiki has gone a full window without refusing * anything, because by then whatever was exhausted has refilled. @@ -90,12 +97,28 @@ function createRateLimitGate( options ) { * batches small enough to fit comfortably. */ function forgetStaleRefusals() { - if ( pacing && now() - lastRefusalAt >= capMs ) { + if ( refusalsAreStale() ) { pacing = false; nextReleaseAt = 0; } } + /** + * The schedule the gate is currently enforcing. + * + * @return {?{waitMs: number, intervalMs: number}} Null when nothing is paced + */ + function schedule() { + if ( halted || !pacing || refusalsAreStale() ) { + return null; + } + + return { + waitMs: Math.max( 0, Math.max( openAt, nextReleaseAt ) - now() ), + intervalMs: intervalMs + }; + } + /** * Adopts the limit the wiki advertises. * @@ -186,6 +209,7 @@ function createRateLimitGate( options ) { return { useLimit: useLimit, + schedule: schedule, wait: wait, noteRateLimited: noteRateLimited, noteProgress: noteProgress, diff --git a/res/ext.SimpleBatchUpload/remainingTime.js b/res/ext.SimpleBatchUpload/remainingTime.js new file mode 100644 index 0000000..797d4dc --- /dev/null +++ b/res/ext.SimpleBatchUpload/remainingTime.js @@ -0,0 +1,56 @@ +'use strict'; + +/** + * How much longer a rate limited batch has to run. + * + * Arithmetic on the schedule the gate already enforces, not a measurement of + * throughput, so there is nothing here to smooth. It excludes transfer time and + * can grow if the wiki refuses again, hence "about" and whole minutes. + */ + +const MS_PER_MINUTE = 60000; + +/** + * @param {number} pending Files admitted to the batch that have not finished + * @param {?Object} schedule From the gate: { waitMs, intervalMs }, or null when + * nothing is paced + * @return {?number} Milliseconds, or null when there is nothing to estimate + */ +function estimateRemainingMs( pending, schedule ) { + if ( !schedule || pending < 1 ) { + return null; + } + + const remaining = schedule.waitMs + ( pending - 1 ) * schedule.intervalMs; + + // A single file released immediately is not being held up by anything, so + // there is nothing to announce. + return remaining > 0 ? remaining : null; +} + +/** + * @param {?number} ms From estimateRemainingMs() + * @return {?string} Message text, or null when there is nothing to say + */ +function describeRemaining( ms ) { + if ( ms === null || ms === undefined ) { + return null; + } + + if ( ms < MS_PER_MINUTE ) { + return mw.msg( 'simplebatchupload-estimate-under-a-minute' ); + } + + // Rounded up because the figure excludes transfer time and so is already an + // underestimate; ceil keeps it an upper bound. Whole minutes also keep the + // live region quiet: the text changes once a minute, not on every refresh. + return mw.msg( + 'simplebatchupload-estimate-minutes', + Math.ceil( ms / MS_PER_MINUTE ) + ); +} + +module.exports = { + estimateRemainingMs: estimateRemainingMs, + describeRemaining: describeRemaining +}; diff --git a/res/ext.SimpleBatchUpload/resultRow.js b/res/ext.SimpleBatchUpload/resultRow.js index 0380d2d..324360d 100644 --- a/res/ext.SimpleBatchUpload/resultRow.js +++ b/res/ext.SimpleBatchUpload/resultRow.js @@ -169,6 +169,9 @@ function createResultRow( sourceName, targetName ) { // A row is finished once it carries one of these. Anything else is still // queued, waiting on the rate limit, or uploading. +// +// ful-estimate is deliberately absent: it is not an upload row, and it has to +// survive a new selection started while an earlier batch is still going. const FINISHED_ROW_CLASSES = [ 'ful-success', 'ful-error', 'ful-notice' ]; /** diff --git a/tests/vitest/ext.SimpleBatchUpload/batchLimit.test.js b/tests/vitest/ext.SimpleBatchUpload/batchLimit.test.js index a0f7098..db73d8e 100644 --- a/tests/vitest/ext.SimpleBatchUpload/batchLimit.test.js +++ b/tests/vitest/ext.SimpleBatchUpload/batchLimit.test.js @@ -46,3 +46,25 @@ describe( 'createBatchLimit', () => { expect( batch.admit() ).toBe( true ); } ); } ); + +describe( 'counting what is still in the batch', () => { + it( 'reports how many files are still queued or in flight', () => { + const batch = createBatchLimit( 10 ); + + batch.admit(); + batch.admit(); + batch.admit(); + batch.release(); + + expect( batch.active() ).toBe( 2 ); + } ); + + it( 'reports nothing left once every file has finished', () => { + const batch = createBatchLimit( 10 ); + + batch.admit(); + batch.release(); + + expect( batch.active() ).toBe( 0 ); + } ); +} ); diff --git a/tests/vitest/ext.SimpleBatchUpload/estimateRow.test.js b/tests/vitest/ext.SimpleBatchUpload/estimateRow.test.js new file mode 100644 index 0000000..25c9e0e --- /dev/null +++ b/tests/vitest/ext.SimpleBatchUpload/estimateRow.test.js @@ -0,0 +1,73 @@ +const { showEstimate } = require( '../../../res/ext.SimpleBatchUpload/estimateRow.js' ); + +function list() { + return document.createElement( 'ul' ); +} + +function estimateIn( ul ) { + return ul.querySelector( 'li.ful-estimate' ); +} + +describe( 'showEstimate', () => { + it( 'shows the text above the rows', () => { + const ul = list(); + ul.appendChild( document.createElement( 'li' ) ); + + showEstimate( ul, 'About 3 minutes left.' ); + + expect( estimateIn( ul ).textContent ).toBe( 'About 3 minutes left.' ); + expect( ul.firstChild.className ).toBe( 'ful-estimate' ); + } ); + + it( 'keeps the row a list item and announces from inside it', () => { + const ul = list(); + + showEstimate( ul, 'About 3 minutes left.' ); + + // role="status" on the
  • itself would replace its listitem role and + // the list would announce one fewer item than it has. + expect( estimateIn( ul ).getAttribute( 'role' ) ).toBeNull(); + expect( estimateIn( ul ).querySelector( '[role="status"]' ) ).not.toBeNull(); + } ); + + it( 'updates the text in place rather than replacing the row', () => { + const ul = list(); + + showEstimate( ul, 'About 3 minutes left.' ); + const first = estimateIn( ul ); + showEstimate( ul, 'About 2 minutes left.' ); + + expect( estimateIn( ul ) ).toBe( first ); + expect( estimateIn( ul ).textContent ).toBe( 'About 2 minutes left.' ); + } ); + + it( 'leaves the text untouched when it has not changed', () => { + const ul = list(); + + showEstimate( ul, 'About 3 minutes left.' ); + const announced = estimateIn( ul ).querySelector( '[role="status"]' ); + const before = announced.textContent; + showEstimate( ul, 'About 3 minutes left.' ); + + // Rewriting an unchanged live region announces it again. + expect( estimateIn( ul ).querySelector( '[role="status"]' ) ).toBe( announced ); + expect( announced.textContent ).toBe( before ); + } ); + + it( 'takes the row away once there is nothing left to say', () => { + const ul = list(); + + showEstimate( ul, 'About 3 minutes left.' ); + showEstimate( ul, null ); + + expect( estimateIn( ul ) ).toBeNull(); + } ); + + it( 'does nothing when there was never anything to say', () => { + const ul = list(); + + showEstimate( ul, null ); + + expect( ul.children.length ).toBe( 0 ); + } ); +} ); diff --git a/tests/vitest/ext.SimpleBatchUpload/rateLimitGate.test.js b/tests/vitest/ext.SimpleBatchUpload/rateLimitGate.test.js index d60f9d8..c5ef6a3 100644 --- a/tests/vitest/ext.SimpleBatchUpload/rateLimitGate.test.js +++ b/tests/vitest/ext.SimpleBatchUpload/rateLimitGate.test.js @@ -263,3 +263,84 @@ describe( 'pacing stops once the budget has demonstrably refilled', () => { expect( clock.now() - before ).toBeGreaterThan( 0 ); } ); } ); + +describe( 'reporting the schedule it is enforcing', () => { + const EIGHT_PER_MINUTE = { intervalMs: 7500, windowMs: 60000 }; + + function limitedGate( clock, maxRetries ) { + return createRateLimitGate( { + now: clock.now, + sleep: clock.sleep, + limit: EIGHT_PER_MINUTE, + maxRetries: maxRetries + } ); + } + + it( 'offers no schedule before the wiki has refused an upload', () => { + expect( limitedGate( createFakeClock() ).schedule() ).toBeNull(); + } ); + + it( 'offers no schedule for a user the wiki does not limit', () => { + const clock = createFakeClock(); + const gate = createRateLimitGate( { now: clock.now, sleep: clock.sleep } ); + + gate.noteRateLimited(); + + expect( gate.schedule() ).toBeNull(); + } ); + + it( 'reports the wait left on the current backoff and the spacing behind it', () => { + const clock = createFakeClock(); + const gate = limitedGate( clock ); + + gate.noteRateLimited(); + + expect( gate.schedule() ).toEqual( { waitMs: 2000, intervalMs: 7500 } ); + } ); + + it( 'counts the wait down as time passes', () => { + const clock = createFakeClock(); + const gate = limitedGate( clock ); + + gate.noteRateLimited(); + clock.advance( 1500 ); + + expect( gate.schedule().waitMs ).toBe( 500 ); + } ); + + it( 'offers no schedule once a full window has passed without a refusal', () => { + const clock = createFakeClock(); + const gate = limitedGate( clock ); + + gate.noteRateLimited(); + clock.advance( 120000 ); + + expect( gate.schedule() ).toBeNull(); + } ); + + it( 'offers no schedule once the batch has been given up on', () => { + const clock = createFakeClock(); + const gate = limitedGate( clock, 1 ); + + gate.noteRateLimited(); + expect( gate.schedule() ).not.toBeNull(); + + // Past openAt, or the second refusal is dismissed as the same overrun. + clock.advance( 2000 ); + gate.noteRateLimited(); + + expect( gate.schedule() ).toBeNull(); + } ); + + it( 'reports the spacing to the next slot once the backoff has expired', async () => { + const clock = createFakeClock(); + const gate = limitedGate( clock ); + + gate.noteRateLimited(); + await gate.wait(); + + // openAt is now in the past; the spacing behind the slot just claimed + // is the only thing left holding the next file up. + expect( gate.schedule() ).toEqual( { waitMs: 7500, intervalMs: 7500 } ); + } ); +} ); diff --git a/tests/vitest/ext.SimpleBatchUpload/remainingTime.test.js b/tests/vitest/ext.SimpleBatchUpload/remainingTime.test.js new file mode 100644 index 0000000..079ec05 --- /dev/null +++ b/tests/vitest/ext.SimpleBatchUpload/remainingTime.test.js @@ -0,0 +1,43 @@ +const { + estimateRemainingMs, + describeRemaining +} = require( '../../../res/ext.SimpleBatchUpload/remainingTime.js' ); + +const PACED = { waitMs: 10000, intervalMs: 5000 }; + +describe( 'estimateRemainingMs', () => { + it( 'counts only the current wait when one file is left', () => { + expect( estimateRemainingMs( 1, PACED ) ).toBe( 10000 ); + } ); + + it( 'adds one interval for every file queued behind the next release', () => { + expect( estimateRemainingMs( 4, PACED ) ).toBe( 10000 + 3 * 5000 ); + } ); + + it( 'says nothing while the wiki has refused nothing', () => { + // No schedule means no pacing, so there is no wait to describe. + expect( estimateRemainingMs( 20, null ) ).toBeNull(); + } ); + + it( 'says nothing once no files are left', () => { + expect( estimateRemainingMs( 0, PACED ) ).toBeNull(); + } ); +} ); + +describe( 'describeRemaining', () => { + it( 'rounds up to whole minutes rather than inventing precision', () => { + expect( describeRemaining( 61000 ) ).toBe( 'simplebatchupload-estimate-minutes(2)' ); + } ); + + it( 'says less than a minute rather than counting seconds down', () => { + expect( describeRemaining( 4000 ) ).toBe( 'simplebatchupload-estimate-under-a-minute' ); + } ); + + it( 'never reads as no time left while files remain', () => { + expect( describeRemaining( 60000 ) ).toBe( 'simplebatchupload-estimate-minutes(1)' ); + } ); + + it( 'says nothing when there is nothing to estimate', () => { + expect( describeRemaining( null ) ).toBeNull(); + } ); +} ); diff --git a/tests/vitest/ext.SimpleBatchUpload/resultRow.test.js b/tests/vitest/ext.SimpleBatchUpload/resultRow.test.js index afdced4..8736d5d 100644 --- a/tests/vitest/ext.SimpleBatchUpload/resultRow.test.js +++ b/tests/vitest/ext.SimpleBatchUpload/resultRow.test.js @@ -106,6 +106,15 @@ describe( 'pruneFinishedRows', () => { expect( list.children.length ).toBe( 0 ); } ); + it( 'keeps the rate limit estimate, which is not an upload row', () => { + const list = listWith( [ 'ful-estimate', 'ful-success' ] ); + + pruneFinishedRows( list ); + + expect( list.children.length ).toBe( 1 ); + expect( list.children[ 0 ].className ).toBe( 'ful-estimate' ); + } ); + it( 'keeps rows whose upload is still running', () => { const list = listWith( [ 'ful-success', '', 'ful-error' ] );