Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add file relocation and renaming api for v3 #1081

Draft
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

revam
Copy link
Member

@revam revam commented Aug 20, 2023

I've tested the new file endpoints to a degree (on linux using sqlite), moving files around, renaming them, creating hard and soft copies, and squashed a few bugs while I was at it.


So this PR adds renaming to the v3 api, and cleans up the move/rename internals, adapts the v1 api to the new internals, among other things.

- refactored VideoLocal_Place, moved some methods out, and organised the rest
  into regions.

- we now use request objects when moving/renaming and there is only one method
  for each operation.

- kept the retry logic we did on import, it will now retry regardless of where
  the operation was ran from (e.g. from the rename util in desktop, during
  import, from a RESTful client, etc.).

- Reworded some of the error messages

- Desktop now shows the full path if the option to move is selected, otherwise
  it will show just the file name.
and generate them dynamically for the v1 api
- don't allow a preview when directly relocating a file

- when a move/rename destinaiton is found, skip the actual operation
  if we're only previewing the destination.

- always display the full path in the v1 api

- add missing parameter usage in the v3 api for direct relocation

- add back logging of source and destination
add per-file toggles for auto-relocation and for auto-deletion

still WIP. missing database migrations and model mappings.
@revam revam changed the title Add renamer api v3 Add file relocation and renaming api for v3 Aug 20, 2023
@@ -15,5 +15,7 @@ public VideoLocal_PlaceMap()
Map(x => x.FilePath).Not.Nullable();
Map(x => x.ImportFolderID).Not.Nullable();
Map(x => x.ImportFolderType).Not.Nullable();
Map(x => x.AllowAutoRelocation).Not.Nullable();
Copy link
Member

Choose a reason for hiding this comment

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

Why? Per file overrides on this seem completely unnecessary

Copy link
Member Author

Choose a reason for hiding this comment

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

To prevent it from being automagically moved, e.g. you place a copy at one location for it to stay there, and not be moved away by shoko. If we had a better renaming system that allowed the user to specify all the paths you would want for a file then I don't it will be needed, no. But until then then I have a personal need for this per-file-location setting to exist.

Copy link
Member Author

@revam revam Aug 20, 2023

Choose a reason for hiding this comment

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

An good example is for Toriko EP1 / One Piece EP420(?), where I would want to place a hard copy in both directories, and have Shoko not touch them.

Copy link
Member

Choose a reason for hiding this comment

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

Well it won't get moved if it's in a destination. For predictability and reproducibility, you want your script to give it exactly one place it should go.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, I think i forgot to expose a way to toggle the per-file location settings (allow auto relocate and allow auto delete) in the api…

Copy link
Member

Choose a reason for hiding this comment

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

That's a fair example, I guess. It's a very niche feature.

Copy link
Member

Choose a reason for hiding this comment

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

This sounds a bit like over-engineering an issue that isn't really there.

@sdaqo
Copy link

sdaqo commented Sep 18, 2023

Will this PR make it possible to have one source import folder and another destination import folder where the files don't get moved but linked or copied instead? If yes awesome. Does it work well enough to test it?

@da3dsoul
Copy link
Member

No. Copying isn't really a good idea anyway. If you want copies, then use a copy on complete script in your downloader.
We can't implement hard linking because .net doesn't have an API for it. Shoko can move hard links if you make a hard link via a completion script in your downloader.

@revam
Copy link
Member Author

revam commented Sep 18, 2023

In case you didn't know @da3dsoul, i added and have tested adding hard linking support in this PR. They won't be made automatically by any renaming script, but it's possible to create them through the REST v3 API in this PR as of right now.

@revam
Copy link
Member Author

revam commented Sep 18, 2023

Also when hard linking (or just creating another location in general) then the two locations will automatically be excempt from auto deletion.

@revam revam marked this pull request as draft September 18, 2023 15:57
@revam
Copy link
Member Author

revam commented Sep 18, 2023

There are a few things to change up, so I've converted this PR to a draft for now, since I'm focusing on the other PR for adding TMDB support.

@revam
Copy link
Member Author

revam commented Sep 18, 2023

Will this PR make it possible to have one source import folder and another destination import folder where the files don't get moved but linked or copied instead? If yes awesome. Does it work well enough to test it?

While it might make that possible it won't make it possible with a traditional renamer, you would have to use the file events directly and also either use the REST api or reference the shoko dlls directly and not using the abstraction.

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.

None yet

4 participants