Skip to content

Commit

Permalink
remove the ability to toggle newness via virtualfs rename
Browse files Browse the repository at this point in the history
  • Loading branch information
azuline committed Nov 19, 2023
1 parent 5965120 commit dbce3b5
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 90 deletions.
29 changes: 1 addition & 28 deletions docs/RELEASES.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ supported. All views in the virtual directory are supported as well.

## Toggle Release "new"-ness

Command line:
This operation is only supported on the command line.

```bash
$ cd $fuse_mount_dir
Expand All @@ -103,33 +103,6 @@ $ tree "2. Releases - New/"
└── LOOΠΔ - 2017. Kim Lip - Single [NEW]/...
```

Virtual filesystem:

_Note that if the release template produces the same path for new and non-new
releases, this operation is not possible via the virtual filesystem._

```bash
$ cd $fuse_mount_dir

# The specific path to rename to depends on the release path template. Here we
# are renaming the directory to what it would have been named if the release
# were new. In the default template, that means appending [NEW]. If you have a
# custom template, you will need to rename the directory in line with how
# "new"-ness is rendered in your template.
$ mv "1. Releases/LOOΠΔ ODD EYE CIRCLE - 2017. Mix & Match - EP" "1. Releases/LOOΠΔ ODD EYE CIRCLE - 2017. Mix & Match - EP [NEW]"

$ tree "2. Releases - New/"
2. Releases - New/
├── LOOΠΔ - 2017. Kim Lip - Single [NEW]/...
└── LOOΠΔ ODD EYE CIRCLE - 2017. Mix & Match - EP [NEW]/...

$ mv "1. Releases/LOOΠΔ ODD EYE CIRCLE - 2017. Mix & Match - EP [NEW]" "1. Releases/LOOΠΔ ODD EYE CIRCLE - 2017. Mix & Match - EP"

$ tree "2. Releases - New/"
2. Releases - New/
└── LOOΠΔ - 2017. Kim Lip - Single [NEW]/...
```

## Set Release Cover Art

_The filename of the cover art in the virtual filesystem will always appear as
Expand Down
51 changes: 5 additions & 46 deletions rose/virtualfs.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
import tempfile
import time
from collections.abc import Iterator
from copy import deepcopy
from dataclasses import dataclass
from pathlib import Path
from typing import Any, Generic, Literal, TypeVar
Expand Down Expand Up @@ -97,7 +96,6 @@
from rose.releases import (
delete_release,
set_release_cover_art,
toggle_release_new,
)
from rose.templates import PathTemplate, eval_release_template, eval_track_template

Expand Down Expand Up @@ -345,11 +343,8 @@ def __init__(self, config: Config):
# behavior as specified in the class docstring.
self._release_store: TTLCache[tuple[VirtualPath, str], str] = TTLCache(ttl_seconds=60 * 15)
self._track_store: TTLCache[tuple[VirtualPath, str], str] = TTLCache(ttl_seconds=60 * 15)
# We also track the what the "inverse" new-ness paths are for a release. This allows us to
# handle toggle-new operations by comparing the new directory name against the inverse.
self._inverse_new_store: TTLCache[tuple[VirtualPath, str], str] = TTLCache(ttl_seconds=60 * 15)
# Cache template evaluations because they're expensive.
self._release_template_eval_cache: dict[tuple[VirtualPath, PathTemplate, str, str | None], tuple[str, str]] = {}
self._release_template_eval_cache: dict[tuple[VirtualPath, PathTemplate, str, str | None], str] = {}
self._track_template_eval_cache: dict[tuple[VirtualPath, PathTemplate, str, str | None], str] = {}
# fmt: on

Expand Down Expand Up @@ -401,40 +396,31 @@ def list_release_paths(
time_start = time.time()
cachekey = (release_parent, template, release.metahash, position)
try:
vname, inverse_new_vname = self._release_template_eval_cache[cachekey]
vname = self._release_template_eval_cache[cachekey]
logger.debug(
f"VNAMES: Reused cached virtual dirname {vname} for release {logtext} in {time.time()-time_start} seconds"
)
except KeyError:
vname = eval_release_template(template, release, position)
vname = sanitize_dirname(vname, False)
# Generate the inverse new virtual name for toggle new checking.
inverse_new_release = deepcopy(release)
inverse_new_release.new = not release.new
inverse_new_vname = eval_release_template(template, inverse_new_release, position)
inverse_new_vname = sanitize_dirname(inverse_new_vname, False)
self._release_template_eval_cache[cachekey] = vname, inverse_new_vname
self._release_template_eval_cache[cachekey] = vname
logger.debug(
f"VNAMES: Generated virtual dirname {vname} for release {logtext} in {time.time()-time_start} seconds"
)

# Handle name collisions by appending a unique discriminator to the end.
original_vname = vname
original_inverse_new_vname = inverse_new_vname
collision_no = 2
while True:
if vname not in seen:
break
logger.debug(f"Virtual dirname collision: {vname=} {seen=}")
vname = f"{original_vname} [{collision_no}]"
inverse_new_vname = f"{original_inverse_new_vname} [{collision_no}]"
collision_no += 1
logger.debug(f"VNAMES: Added collision number to virtual dirname {vname}")

# Store the generated release name in the cache.
time_start = time.time()
self._release_store[(release_parent, vname)] = release.id
self._inverse_new_store[(release_parent, vname)] = inverse_new_vname
seen.add(vname)
logger.debug(
f"VNAMES: Time cost of storing the virtual dirname: {time.time()-time_start=} seconds"
Expand Down Expand Up @@ -547,23 +533,6 @@ def lookup_track(self, p: VirtualPath) -> str | None:
logger.debug(f"VNAMES: Failed to resolve track virtual name {p}")
return None

def check_toggle_new(self, old: VirtualPath, new: VirtualPath) -> bool:
"""
Check whether the two directory names indicate a "toggle new" operation. The toggle new
operation occurs when the "new" marker is removed or added to the directory name. However,
the position and formatting of the marker depends on the path template, so we generate the
new and not-new path templates for the release and assert that each of the inputs matches
them.
"""
logger.debug(f"VNAMES: Checking toggle new validity for {old=} and {new=}")
if not old.release or not new.release or old.release == new.release:
return False
try:
inverse_path = self._inverse_new_store[(old.release_parent, old.release)]
except KeyError:
return False
return old.release_parent == new.release_parent and new.release == inverse_path


class CanShower:
"""
Expand Down Expand Up @@ -1014,18 +983,8 @@ def rename(self, old: VirtualPath, new: VirtualPath) -> None:
logger.debug(f"LOGICAL: Received rename for {old=} {new=}")

# Possible actions:
# 1. Toggle a release's new status.
# 2. Rename a collage.
# 3. Rename a playlist.
# TODO: Consider allowing renaming artist/genre/label here?
if (
(old.release and new.release)
and (not old.file and not new.file)
and (release_id := self.vnames.lookup_release(old))
and self.vnames.check_toggle_new(old, new)
):
toggle_release_new(self.config, release_id)
return
# 1. Rename a collage.
# 2. Rename a playlist.
if (
old.view == "Collages"
and new.view == "Collages"
Expand Down
16 changes: 0 additions & 16 deletions rose/virtualfs_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -370,22 +370,6 @@ def test_virtual_filesystem_read_from_deleted_file(config: Config, source_dir: P
assert (config.fuse_mount_dir / "1. Releases").is_dir()


def test_virtual_filesystem_toggle_new(config: Config, source_dir: Path) -> None: # noqa: ARG001
dirname = "NewJeans - 1990. I Love NewJeans"
root = config.fuse_mount_dir
with start_virtual_fs(config):
(root / "1. Releases" / dirname).rename(root / "1. Releases" / f"{dirname} [NEW]")
assert (root / "1. Releases" / f"{dirname} [NEW]").is_dir()
assert (root / "1. Releases" / dirname) not in set((root / "1. Releases").iterdir())
(root / "1. Releases" / f"{dirname} [NEW]").rename(root / "1. Releases" / dirname)
assert (root / "1. Releases" / dirname).is_dir()
assert (root / "1. Releases" / f"{dirname} [NEW]") not in set(
(root / "1. Releases").iterdir()
)
with pytest.raises(OSError): # noqa: PT011
(root / "1. Releases" / dirname).rename(root / "1. Releases" / "lalala")


@pytest.mark.usefixtures("seeded_cache")
def test_virtual_filesystem_blacklist(config: Config) -> None:
new_config = dataclasses.replace(
Expand Down

0 comments on commit dbce3b5

Please sign in to comment.