diff --git a/web-ui/javascript/src/components/PackLibrary.js b/web-ui/javascript/src/components/PackLibrary.js index 713111db..727f194f 100644 --- a/web-ui/javascript/src/components/PackLibrary.js +++ b/web-ui/javascript/src/components/PackLibrary.js @@ -28,7 +28,7 @@ import Modal from "./Modal"; import { LOCAL_STORAGE_ALLOW_ENRICHED_BINARY_FORMAT } from "../utils/storage"; -import {chooseDropAction, applyProvenanceVerdict} from "../utils/packs"; +import {chooseDropAction, applyProvenanceVerdict, currentVersion, isDowngrade} from "../utils/packs"; import {verifyConversion} from "../services/library"; import './PackLibrary.css'; @@ -61,6 +61,10 @@ class PackLibrary extends React.Component { confirmConversionDialog: { show: false, data: null + }, + confirmDowngradeDialog: { + show: false, + data: null } }; } @@ -163,10 +167,37 @@ class PackLibrary extends React.Component { }; doAddToDevice = (data, path) => { + // Every path to the device funnels through here, so this is where a version going backwards + // is caught. The device has one slot per UUID and no notion of version: sending a v1 over the + // v2 it holds silently replaces the newer story. The user may mean it; they are asked. + const onDevice = this.state.device.packs.find(p => p.uuid === data.uuid); + if (isDowngrade(this.state.device.packs, data.uuid, data.version)) { + this.setState({ + confirmDowngradeDialog: { + show: true, + data: { pack: data, path, deviceVersion: onDevice.version } + } + }); + return; + } + this.doTransferToDevice(data, path); + }; + + doTransferToDevice = (data, path) => { // Transfer pack and show progress this.props.addFromLibrary(data.uuid, path, data.format, this.state.device.metadata.driver, this.context); }; + dismissConfirmDowngradeDialog = (replace) => { + return () => { + const { pack, path } = this.state.confirmDowngradeDialog.data; + this.setState({ confirmDowngradeDialog: { show: false, data: null } }); + if (replace) { + this.doTransferToDevice(pack, path); + } + }; + }; + dismissEnrichedDialog = (allow) => { return () => { this.props.setAllowEnriched(allow); @@ -432,6 +463,16 @@ class PackLibrary extends React.Component { ]} onClose={this.dismissConfirmConversionDialog('cancel')} />} + {this.state.confirmDowngradeDialog.show && + {t('dialogs.library.askConfirmDowngrade.content', { version: this.state.confirmDowngradeDialog.data.pack.version, deviceVersion: this.state.confirmDowngradeDialog.data.deviceVersion })}

} + buttons={[ + { label: t('dialogs.library.askConfirmDowngrade.cancel'), onClick: this.dismissConfirmDowngradeDialog(false)}, + { label: t('dialogs.library.askConfirmDowngrade.replace'), onClick: this.dismissConfirmDowngradeDialog(true)} + ]} + onClose={this.dismissConfirmDowngradeDialog(false)} + />} {/* Device view, if plugged */} {this.state.device.metadata &&
@@ -567,6 +608,7 @@ class PackLibrary extends React.Component {
+ {currentVersion(group.packs) !== undefined &&
{`v${currentVersion(group.packs)}`}
} {group.packs[0].official &&
{t('library.official')}
}
diff --git a/web-ui/javascript/src/utils/packs.js b/web-ui/javascript/src/utils/packs.js index 0f717abd..e7752281 100644 --- a/web-ui/javascript/src/utils/packs.js +++ b/web-ui/javascript/src/utils/packs.js @@ -57,6 +57,49 @@ export function isConversionOutput(pack) { return CONVERSION_OUTPUT_NAME.test(basename(pack.path)); } +/* + * Several versions of one story. + * + * The library keeps every archive of a UUID, so a story can exist as v1 and v2 side by side; the + * device has one slot per UUID and no notion of version (FORMATS.md §9). The story pack version is + * the one piece of identity the formats carry for telling the versions apart, so it is what decides + * which artefacts are "the story as it is now". The modification time says when a file was written, + * not what it holds, and only ever breaks ties within a version. + * + * Artefacts without a version (older listings, hand-made entries) are not compared: when nothing + * carries a version the whole group is current, as it was before versions were considered. + */ +function hasVersion(p) { + return Boolean(p) && Number.isFinite(p.version); +} + +export function currentVersion(packs) { + if (!Array.isArray(packs)) { + return undefined; + } + const versions = packs.filter(hasVersion).map(p => p.version); + return versions.length === 0 ? undefined : Math.max(...versions); +} + +function packsOfCurrentVersion(packs) { + const current = currentVersion(packs); + return current === undefined ? packs : packs.filter(p => hasVersion(p) && p.version === current); +} + +/* + * Whether sending `version` of `uuid` would replace a higher version already on the device. The + * device cannot tell, so the question is asked here, before the transfer. Missing versions on either + * side answer false: there is nothing to compare, and a guard that fires on missing data would block + * every legacy pack. + */ +export function isDowngrade(devicePacks, uuid, version) { + if (!Array.isArray(devicePacks) || !Number.isFinite(version)) { + return false; + } + const onDevice = devicePacks.find(p => p && p.uuid === uuid); + return hasVersion(onDevice) && onDevice.version > version; +} + /* * Decide what to do when a pack is dropped onto a device. * @@ -96,9 +139,12 @@ export function chooseDropAction(packs, driverFormat) { if (!Array.isArray(packs) || packs.length === 0) { return { action: 'none', source: undefined, cached: undefined }; } - const cached = packs.find(p => p.format === driverFormat); - const convertible = packs.find(p => p.format !== driverFormat); - const sourceCandidate = packs.find(p => p.format !== driverFormat && !isConversionOutput(p)); + // Only the current version is a candidate for the device. An older version's conversion may + // be device-readable and newer on disk; it is still not the story the user means to send. + const current = packsOfCurrentVersion(packs); + const cached = current.find(p => p.format === driverFormat); + const convertible = current.find(p => p.format !== driverFormat); + const sourceCandidate = current.find(p => p.format !== driverFormat && !isConversionOutput(p)); if (!cached) { // Nothing the device can read: convert. There is no cached artefact to compare anything diff --git a/web-ui/javascript/src/utils/packs.test.js b/web-ui/javascript/src/utils/packs.test.js index 25593b82..a0fafeae 100644 --- a/web-ui/javascript/src/utils/packs.test.js +++ b/web-ui/javascript/src/utils/packs.test.js @@ -26,7 +26,7 @@ * rendering stack and this lot does not add one. */ -const { chooseDropAction, isConversionOutput, applyProvenanceVerdict } = require('./packs'); +const { chooseDropAction, isConversionOutput, applyProvenanceVerdict, currentVersion, isDowngrade } = require('./packs'); // The library lists a pack's artefacts most-recent-first; these fixtures keep that order, and carry // the file names the backend actually sends. @@ -286,3 +286,101 @@ describe('applyProvenanceVerdict', () => { expect(applyProvenanceVerdict(none, 'UNKNOWN')).toBe(none); }); }); + +/* + * Several versions of one story in the library. + * + * The library keeps every archive of a UUID, so a story can exist as v1 and v2 side by side. The + * device, on the other hand, has one slot per UUID and no notion of version (FORMATS.md §9). Which + * version goes to the device is therefore decided here, and it is the highest one: the version is + * the one piece of identity the formats carry for this. The modification time, which used to order + * everything, says when a file was written, not what it holds. + */ +const versioned = (format, path, version, timestamp) => ({ uuid: 'u', format, path, version, timestamp }); + +describe('currentVersion', () => { + + it('is the highest version present, wherever it sits in the list', () => { + expect(currentVersion([versioned('archive', 'u-v1.zip', 1, 3000), versioned('archive', 'u-v2.zip', 2, 1000)])).toBe(2); + }); + + it('is undefined when nothing carries a version', () => { + expect(currentVersion([pack('archive', 'u.zip', 1000)])).toBeUndefined(); + expect(currentVersion([])).toBeUndefined(); + expect(currentVersion(undefined)).toBeUndefined(); + }); + + it('ignores artefacts without a version when at least one has it', () => { + expect(currentVersion([pack('fs', 'u.converted_1', 5000), versioned('archive', 'u-v3.zip', 3, 1000)])).toBe(3); + }); +}); + +describe('chooseDropAction across versions', () => { + + it('V1: sends the highest version, not the most recent file', () => { + const v1 = versioned('fs', 'u-v1.converted_9000', 1, 9000); + const v2 = versioned('archive', 'u-v2.zip', 2, 1000); + + const decision = chooseDropAction([v1, v2], 'fs'); + + // The v1 conversion is device-readable and newer, and it is not what the user means: the + // story moved on. Nothing of the current version is device-readable, so it is converted. + expect(decision.action).toBe('convert'); + expect(decision.source).toBe(v2); + expect(decision.cached).toBeUndefined(); + }); + + it('V2: a conversion of the current version is still reused, with the usual question', () => { + const cachedV2 = versioned('fs', 'u-v2.converted_2000', 2, 2000); + const sourceV2 = versioned('archive', 'u-v2.zip', 2, 1000); + const oldV1 = versioned('archive', 'u-v1.zip', 1, 500); + + const decision = chooseDropAction([cachedV2, sourceV2, oldV1], 'fs'); + + expect(decision.action).toBe('confirm'); + expect(decision.cached).toBe(cachedV2); + expect(decision.source).toBe(sourceV2); + }); + + it('V3: an older version is never the source, even when it is the only non-conversion', () => { + // Only a conversion output exists for v2; the v1 archive is the only "source" in the group. + // Offering it would re-convert the old story over the new one. + const cachedV2 = versioned('fs', 'u-v2.converted_2000', 2, 2000); + const oldV1 = versioned('archive', 'u-v1.zip', 1, 500); + + const decision = chooseDropAction([cachedV2, oldV1], 'fs'); + + expect(decision.action).toBe('transfer'); + expect(decision.cached).toBe(cachedV2); + expect(decision.source).toBeUndefined(); + }); + + it('V4: artefacts without a version are treated as one group, as before', () => { + // Every earlier case in this file builds packs without a version: none of them may change. + const cached = pack('fs', 'u.converted_3000', 3000); + const source = pack('archive', 'u.zip', 1000); + + expect(chooseDropAction([cached, source], 'fs').action).toBe('confirm'); + }); +}); + +describe('isDowngrade', () => { + const onDevice = (uuid, version) => ({ uuid, version }); + + it('is true when the device holds a higher version of the same story', () => { + expect(isDowngrade([onDevice('u', 2)], 'u', 1)).toBe(true); + }); + + it('is false for the same or a higher version, or a story the device does not hold', () => { + expect(isDowngrade([onDevice('u', 2)], 'u', 2)).toBe(false); + expect(isDowngrade([onDevice('u', 2)], 'u', 3)).toBe(false); + expect(isDowngrade([onDevice('other', 9)], 'u', 1)).toBe(false); + expect(isDowngrade([], 'u', 1)).toBe(false); + }); + + it('is false when either side has no version to compare, rather than guessing', () => { + expect(isDowngrade([onDevice('u', undefined)], 'u', 1)).toBe(false); + expect(isDowngrade([onDevice('u', 2)], 'u', undefined)).toBe(false); + expect(isDowngrade(undefined, 'u', 1)).toBe(false); + }); +}); diff --git a/web-ui/src/main/java/studio/webui/service/LibraryService.java b/web-ui/src/main/java/studio/webui/service/LibraryService.java index 83edeb75..95a82169 100644 --- a/web-ui/src/main/java/studio/webui/service/LibraryService.java +++ b/web-ui/src/main/java/studio/webui/service/LibraryService.java @@ -40,6 +40,7 @@ import java.nio.file.attribute.FileTime; import java.time.Duration; import java.util.Base64; +import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.Optional; @@ -134,6 +135,18 @@ public JsonObject libraryInfos() { .put("path", libraryPath()); } + /** + * The order of one story's artefacts: highest story pack version first, and within a version the + * most recently modified file first. The first entry is what the UI shows as latest, what a drop + * on the device considers first, and what the unofficial metadata is refreshed from. Ordering + * by modification time alone let a v1 copied in after the v2 take all three roles; the version + * is the one piece of identity the formats carry for this (FORMATS.md §9), so it decides, and + * the timestamp only breaks ties. + */ + private static final Comparator HIGHEST_VERSION_FIRST = Comparator + .comparingInt((LibraryPack p) -> p.getMetadata().getVersion()).reversed() + .thenComparing(Comparator.comparingLong(LibraryPack::getTimestamp).reversed()); + public JsonArray packs() { // Check that local library folder exists File libraryFolder = new File(libraryPath()); @@ -153,7 +166,7 @@ public JsonArray packs() { .entrySet() .forEach(entry -> { List packs = entry.getValue(); - packs.sort((a, b) -> Long.compare(b.getTimestamp(), a.getTimestamp())); + packs.sort(HIGHEST_VERSION_FIRST); LOGGER.debug("Refreshing metadata for pack `" + entry.getKey() + "` from file `" + packs.get(0).getPath() + "`"); this.readPackFile(packs.get(0).getPath()).ifPresent( meta -> databaseMetadataService.refreshUnofficialMetadata( @@ -187,8 +200,7 @@ public JsonArray packs() { Map.Entry::getKey, entry -> new JsonArray( entry.getValue().stream() - // Sort packs by timestamp descending - .sorted((a,b) -> Long.compare(b.getTimestamp(), a.getTimestamp())) + .sorted(HIGHEST_VERSION_FIRST) .map(this::getPackMetadata) .collect(Collectors.toList()) ) diff --git a/web-ui/src/main/resources/webroot/locales/en/translation.json b/web-ui/src/main/resources/webroot/locales/en/translation.json index 07cf19ff..a01aee5d 100644 --- a/web-ui/src/main/resources/webroot/locales/en/translation.json +++ b/web-ui/src/main/resources/webroot/locales/en/translation.json @@ -40,6 +40,7 @@ "convertPackToFs": "Convert this story pack to FS format (v2.x)", "editPack": "Open this story pack in editor", "removePack": "Remove this story pack from library", + "currentVersion": "Current version in the library", "addPack": "Add a story pack to the library" }, "official": "Official", @@ -329,6 +330,12 @@ "convert": "Convert again", "transfer": "Send the existing one", "cancel": "Cancel" + }, + "askConfirmDowngrade": { + "title": "Send an older version?", + "content": "The device holds version {{deviceVersion}} of this story; you are about to replace it with version {{version}}. The device keeps one copy per story, so the newer one would be gone from it (it stays in the library).", + "replace": "Replace with the older version", + "cancel": "Cancel" } }, "settings": { diff --git a/web-ui/src/main/resources/webroot/locales/fr/translation.json b/web-ui/src/main/resources/webroot/locales/fr/translation.json index 1ffe961e..3856f430 100644 --- a/web-ui/src/main/resources/webroot/locales/fr/translation.json +++ b/web-ui/src/main/resources/webroot/locales/fr/translation.json @@ -40,6 +40,7 @@ "convertPackToFs": "Convertir ce pack d'histoires au format FS (v2.x)", "editPack": "Ouvrir ce pack d'histoires dans l'éditeur", "removePack": "Supprimer ce pack d'histoires de la bibliothèque", + "currentVersion": "Version courante dans la bibliothèque", "addPack": "Ajouter un pack d'histoires à la bibliothèque" }, "official": "Officiel", @@ -329,6 +330,12 @@ "convert": "Reconvertir", "transfer": "Envoyer l'existante", "cancel": "Annuler" + }, + "askConfirmDowngrade": { + "title": "Envoyer une version plus ancienne ?", + "content": "La Lunii contient la version {{deviceVersion}} de cette histoire ; vous allez la remplacer par la version {{version}}. La Lunii ne garde qu'un exemplaire par histoire : la plus récente en disparaîtrait (elle reste dans la bibliothèque).", + "replace": "Remplacer par l'ancienne version", + "cancel": "Annuler" } }, "settings": { diff --git a/web-ui/src/test/java/studio/webui/service/LibraryVersionOrderingTest.java b/web-ui/src/test/java/studio/webui/service/LibraryVersionOrderingTest.java new file mode 100644 index 00000000..80edbf9e --- /dev/null +++ b/web-ui/src/test/java/studio/webui/service/LibraryVersionOrderingTest.java @@ -0,0 +1,175 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +package studio.webui.service; + +import io.vertx.core.json.JsonArray; +import io.vertx.core.json.JsonObject; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import studio.metadata.DatabaseMetadataService; + +import java.io.IOException; +import java.io.OutputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.FileTime; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import java.util.zip.CRC32; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * Which artefact of a story comes first when the library holds several versions of it. + * + *

The library groups artefacts by UUID and used to order each group by file modification time, + * newest first. The first entry is what the UI calls "latest", what the drop-on-device logic looks + * at first, and what the unofficial metadata (title, thumbnail) is refreshed from. A modification + * time says when a file was last written, not which version it holds: a v1 archive copied into the + * library after the v2 was saved sorts first, its title takes over the tile, and it is what the + * device would get. The story pack version is the one piece of identity the formats carry for this + * (see {@code FORMATS.md} §9), so it orders the group; the modification time only breaks ties + * between artefacts of the same version, which is what it was ever good for. + * + *

Same fixture as {@link LibraryCacheCoherenceTest}: a minimal {@code story.json} in a STORED + * zip, the metadata database outside {@code @TempDir} for the reason documented there. + */ +class LibraryVersionOrderingTest { + + private static final String UUID_UNDER_TEST = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"; + private static final long FIXED_ENTRY_TIME = 1_500_000_000_000L; + private static final long OLD = 1_600_000_000_000L; + private static final long NEW = 1_700_000_000_000L; + + @TempDir + Path root; + private Path library; + private Path metadataDatabaseFolder; + private LibraryService service; + private final Map previousProperties = new HashMap<>(); + + @BeforeEach + void setUp() throws IOException { + library = root.resolve("library"); + Files.createDirectories(library); + Files.createDirectories(root.resolve("tmp")); + metadataDatabaseFolder = Files.createTempDirectory("studio-metadata-db"); + metadataDatabaseFolder.toFile().deleteOnExit(); + setProperty(LibraryService.LOCAL_LIBRARY_PROP, library.toString() + java.io.File.separator); + setProperty(LibraryService.TMP_DIR_PROP, root.resolve("tmp").toString() + java.io.File.separator); + setProperty(DatabaseMetadataService.UNOFFICIAL_DB_PROP, metadataDatabaseFolder.resolve("unofficial.json").toString()); + service = new LibraryService(new DatabaseMetadataService(true)); + } + + @AfterEach + void tearDown() { + previousProperties.forEach((key, value) -> { + if (value == null) { + System.clearProperty(key); + } else { + System.setProperty(key, value); + } + }); + try { + Files.deleteIfExists(metadataDatabaseFolder.resolve("unofficial.json")); + Files.deleteIfExists(metadataDatabaseFolder); + } catch (IOException tolerated) { + // The metadata database reader may still hold the file; deleteOnExit has another go. + } + } + + @Test + @DisplayName("the highest version comes first, even when an older version's file is newer") + void highestVersionFirstRegardlessOfModificationTime() throws IOException { + writePack("story-v2.zip", 2, "Title v2", OLD); + writePack("story-v1.zip", 1, "Title v1", NEW); + + assertEquals(List.of(2, 1), exposedVersions()); + } + + @Test + @DisplayName("within one version, the most recently modified artefact comes first") + void modificationTimeOnlyBreaksTiesWithinAVersion() throws IOException { + writePack("story-v1-old.zip", 1, "Title", OLD); + writePack("story-v1-new.zip", 1, "Title", NEW); + writePack("story-v2.zip", 2, "Title", OLD); + + assertEquals(List.of("story-v2.zip", "story-v1-new.zip", "story-v1-old.zip"), exposedPaths()); + } + + @Test + @DisplayName("the story's title on the tile is the highest version's, not the newest file's") + void metadataIsRefreshedFromTheHighestVersion() throws IOException { + writePack("story-v2.zip", 2, "Title v2", OLD); + writePack("story-v1.zip", 1, "Title v1", NEW); + + // Every artefact of the group reports the group's title, which the unofficial database + // refreshed from the first entry. Before the fix that was the v1 file, being the newest. + JsonArray packs = exposedGroup().getJsonArray("packs"); + for (int i = 0; i < packs.size(); i++) { + assertEquals("Title v2", packs.getJsonObject(i).getString("title"), "artefact " + i); + } + } + + // ---------------------------------------------------------------- fixtures and helpers + + private void setProperty(String key, String value) { + previousProperties.put(key, System.getProperty(key)); + System.setProperty(key, value); + } + + private JsonObject exposedGroup() { + JsonArray groups = service.packs(); + assertEquals(1, groups.size(), "expected exactly one UUID group"); + return groups.getJsonObject(0); + } + + private List exposedVersions() { + JsonArray packs = exposedGroup().getJsonArray("packs"); + return IntStream.range(0, packs.size()).mapToObj(i -> packs.getJsonObject(i).getInteger("version")).collect(Collectors.toList()); + } + + private List exposedPaths() { + JsonArray packs = exposedGroup().getJsonArray("packs"); + return IntStream.range(0, packs.size()).mapToObj(i -> packs.getJsonObject(i).getString("path")).collect(Collectors.toList()); + } + + private void writePack(String name, int version, String title, long modifiedMillis) throws IOException { + String descriptor = "{\"version\":" + version + + ",\"title\":\"" + title + "\"" + + ",\"description\":\"A story pack built for a test\"" + + ",\"nightModeAvailable\":false" + + ",\"stageNodes\":[{\"uuid\":\"" + UUID_UNDER_TEST + "\"}]}"; + byte[] bytes = descriptor.getBytes(StandardCharsets.UTF_8); + CRC32 crc = new CRC32(); + crc.update(bytes); + ZipEntry entry = new ZipEntry("story.json"); + entry.setMethod(ZipEntry.STORED); + entry.setSize(bytes.length); + entry.setCompressedSize(bytes.length); + entry.setCrc(crc.getValue()); + entry.setTime(FIXED_ENTRY_TIME); + Path target = library.resolve(name); + try (OutputStream out = Files.newOutputStream(target); + ZipOutputStream zip = new ZipOutputStream(out)) { + zip.setMethod(ZipOutputStream.STORED); + zip.putNextEntry(entry); + zip.write(bytes); + zip.closeEntry(); + } + Files.setLastModifiedTime(target, FileTime.fromMillis(modifiedMillis)); + } +}