search

rmcrackan / Libation

Libation: Liberate your Library
GNU General Public License v3.0
2.63k stars 142 forks source link

Libation doesn't know when an item has been removed from the audible library #269

Closed Mbucari closed 1 year ago

Mbucari commented 2 years ago

Described here

If a book has been removed from the user's audible library (either by the user or by Audible) but is still in Libations database, Liberate All will always try and fail to liberate it.

My first though is to add a new LiberatedStatus enum value: Unavailable

rmcrackan commented 2 years ago

Sure. We could use a new Unavailable enum status instead of Error when that happens.

Mbucari commented 2 years ago

On a sort of but not really related note, should we make it possible to remove books from the library in a way that they are not added back on the next account sync?

Reason: There are some books that I bought and have no interest listening to or seeing. I'd probably prefer to not even own them, but automatic return is not available so I'd have to go through contacting audible for them to remove it. Even then, I may want access to the book again in the future. Also, there are some podcasts that I've added and for the life of me cannot figure out how to remove from my account. I just don't want to see them anymore!

Possible Solution: Add a flag in the db for the book to be ignored. Maybe an Ignore value in LiberateStatus. By default, no books with that flag are returned when fetching the library.

Problem How do I remove the "Ignored" flag once it's set? The book is ignored, so it's not in the grid for me to edit the UserDefinedItem. Maybe a separate dialog (like the soon-to-be-deleted RemoveBooksDialog) that lists all books with the Ignored flag and a checkbox to un-ignore?

Problem 2: How would we differentiate this type of "Remove Books" (book is not removed from the db) from the existing type of "Remove books" (book is removed from the db).

Mbucari commented 2 years ago

instead of Error.

Now that you mention Error, it got me thinking that a LiberateStatus is not the way to do this. If you have already liberated the book and the book has become unavailable, it should still show as liberated in your library. Additionally, It should not be automatically selected for removal when doing a Remove Books Scan. This one is pretty easy to solve: only scan NotLiberated books with the auto scan and remove tool. I've already made this change on my end.

On the other hand, if the book hasn't been liberated and has become unavailable, then... Not really sure what should happen in this case. Maybe just automatically remove that book from the library? Maybe automatically remove it only if there are no user defined tags?

But the more I think about it, the more this is beginning to sound related to https://github.com/rmcrackan/Libation/issues/204

wtanksleyjr commented 2 years ago

if the book hasn't been liberated and has become unavailable, then... Not really sure what should happen in this case. Maybe just automatically remove that book from the library?

Not automatically, please -- although this specific case would be solved by that, there are cases where having a book you owned vanish means you have the right to contact Audible's support and get a replacement credit. (If it's possible to tell whether the book had been "truly" owned, that could be automatic, since of course podcasts and borrowed books are different from purchased ones.)

rmcrackan commented 2 years ago

should we make it possible to remove books from the library in a way that they are not added back on the next account sync?

It's not a bad idea. Also not one I'm rushing to make because it could all be solved quicker with better documentation. You can delete permanently at https://www.amazon.com/mycd/ . You can hide without deleting via tagging and filtering. There's also a special tag hidden which will grey-out the row.

Error ... is not the way to do this

In the world of non-podcast book titles, "Error" == attempt to liberate resulted in an unknown error. Perhaps it should have been 'not liberated' and also create a new flag just for that. In the wake of podcast complications, it should probably be re-thought. New enum value 'not in library' ?

emu901 commented 2 years ago

On a sort of but not really related note, should we make it possible to remove books from the library in a way that they are not added back on the next account sync?

I have the same need, I would like to manually remove audio books from the list within Libation. The command "Import > Remove Library Books" presumably does that, however upon next sync, those removed books are re-added by Libation. I believe Libation should make an internal list of books that have been removed manually, and ignore them on the next sync. The current implementation of "remove library books" in my point of view is not working as I would expect it as a user.

Note: the removal of books only applies to the list of books stored within Libation.

rmcrackan commented 2 years ago

In the wake of podcast complications, it should probably be re-thought. New enum value 'not in library' ?

Bad idea. Something can be liberated and also no longer in library. Do not put orthogonal concepts into the same enum.

rmcrackan commented 2 years ago

@Mbucari

What do you think of this solution?

Start tracking if a book is still part of the library and store on LibraryBook. Update flag on each scan. Download can use this flag to not try unavailable books. Also if this is discovered during download attempt, should we update the flag?

Then it's a matter of how to inform the user. Popup? Different background color?

CharlieRussel commented 2 years ago

Different colour, with a right click for details.

Sent from Outlookhttps://aka.ms/qtex0l on my phone. Please excuse any terseness or Autocorrect typos.

From: rmcrackan @.> Sent: Wednesday, July 20, 2022 9:15:31 AM To: rmcrackan/Libation @.> Cc: Subscribed @.***> Subject: Re: [rmcrackan/Libation] Libation doesn't know when an item has been removed from the audible library (Issue #269)

@Mbucarihttps://github.com/Mbucari

What do you think of this solution?

Start tracking if a book is still part of the library and store on LibraryBook. Update flag on each scan. Download can use this flag to not try unavailable books. Also if this is discovered during download attempt, should we update the flag?

Then it's a matter of how to inform the user. Popup? Different background color?

— Reply to this email directly, view it on GitHubhttps://github.com/rmcrackan/Libation/issues/269#issuecomment-1190482987, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AFAHCREJEFJ5TUR22FQIMALVVAQ2HANCNFSM5YPCGK7Q. You are receiving this because you are subscribed to this thread.Message ID: @.***>

andirapandi commented 2 years ago

I dont think its the same issue as 372. I have the same problem: 1000 books in library (well, not all of them bought, some come with the membership). Only single Liberation window (why the rush? :) ). After ~200 downloads, every single download fails. At the same time, the app is working, also with newly added books. I get in Log "Download & Decrypt: Validation failed". Maybe the virtual device that was created (for me it said iOS or iPhone or something) is blocked? Also, purchased books can still be downloaded even after above error comes up persistently.

Sorry, this meant to be a comment to issue 372.. though that is closed. Hope its not too confusing. Tracking already downloaded items might perhaps help - when really your virtual device is blocked, and one could generate a new one and add the account again but it notices that there already were downloads.

Mbucari commented 1 year ago

@rmcrackan I Found how to identify books that will be leaving audible soon.

dtoItems.Where(i => i.Plans.Any(p => p.PlanName == "US Minerva" && p.EndDate.HasValue && (p.EndDate.Value - DateTimeOffset.Now).TotalDays < 30)).ToList();

IDK what "US Minerva" is, but that query returns a list of 22 Items that exactly matches what Audible says is soon leaving my library.

rmcrackan commented 1 year ago

"Minerva" is one of a few things I found which correlated with the 'plus' catalog. Must have been an old code name which persisted in some corners of the codebase.

autryld commented 1 year ago

I read that on the Audible website that you can hide a book by marking it as "Finished". Will this work for the purpose of Libation?

rmcrackan commented 1 year ago

See also #499

rmcrackan commented 1 year ago

I read that on the Audible website that you can hide a book by marking it as "Finished". Will this work for the purpose of Libation?

No simple answer. We'd have to read that status which wouldn't be hard. Then we could provide that as a search parameter. I don't want that to auto-hide books though because an update would happen and people would reasonably wonder where most of their books went. We could allow for searching by that though.

Up until now I haven't done anything with 'finished' because the audible app on my phone has been flakey about marking books that way. I seem to almost always have to manually do it. On its own, the app leaves my completed book as 1 or 2 seconds remaining. So this just seemed like one more audible deficiency that was going to become my problem to provide support for.

Mbucari commented 1 year ago

@rmcrackan This is now the oldest open issue by a long shot, and I've lost the plot.

Our API Capabilities

Before I get into the problems and potential solutions, I want to summarize what info we have or can get through the API

What we can do:

What we MAY be able to do:

The Issues

I think this is actually 2 issues:

  1. Libation keeps trying to download books to which users no longer have rights.
  2. Users want to know if they have a liberated book that is not longer available to them in their library (due to it being returned, expiring plus title, Audible fuckup, or whatever).

Solutions

For Issue # 1, I really like your LibraryBook flag idea. It's easy to implement, requires no user intervention or settings, and will probably be robust.

I have no idea what to do about Issue # 2. I think on a technical level we have the data to do it, but I don't know how to make a UX that isn't deeply annoying to users (mostly me, the user I care the most about xD), such as a messagbox popup after each scan that lists unavailable titles still in the library. Thoughts?

rmcrackan commented 1 year ago

I don't think #2 is actually a problem, only #1. This came about because of errors when the user attempted to download a book which was no longer in their audible library. If it's already been downloaded, no one will notice or care. Especially now that there's a way to both hide and delete books (thank you again), the only thing to address that I see is to mark it with the new enum value 'unavailable'. The only possible challenge is: was it a legit error or a real denied license? I think the error msg will reveal that.

Tech details: I believe that Unavailables should not be attempted again and switching their status ad hoc or in batch should be subject to the same rules as Error. A user can choose to set error/unavailable to downloaded or not downloaded, but error and unavailable are only set-able by Libation. Oh yeah, and this new state will need 1 or 3 icons. 1 icon if all unavailable books use the same icon. 3 if we want 'unavailable, no pdf', 'unavailable, pdf downloaded, book isn't', 'unavailable, book downloaded, pdf isn't'. My vote is just 1 icon, just as Error does.

Thoughts?

manalive commented 1 year ago

I care about #2 because I have a great many books I've already downloaded with badly split chapters (or still have "this is audible" intros) that I need to redownload, but I only want to delete and try to redownload books that are still available.

Mbucari commented 1 year ago

@rmcrackan I was working on this last night, and I think I have a solution that I'd like to run by you.

After bookImporter.Import runs in LibraryBookImporter.DoImport, all DbContext.LibraryBooks with null Book are books that were not found in the library scan. A simple null check is enough to determine if they're no longer in the library.

Determining if a Plus title is no longer available is a bit trickier. Here's a query I've found works:

i.DtoItem.IsAyce is true &&
!i.DtoItem.Plans.Any(p => p.PlanName.Contains("Minerva") || p.PlanName.Contains("Free Tier"))

Basically, if a Book had been added to your library as an audible Plus book, IsAyce is true regardless of whether it's still available. But in order to listen to it, you need rights under the "Minerva" or "Free Tier" plans. If you don't have either of those plans then it will show as "Unavailable" in your Audible library.

This query has a 100% success rate and 0% false negative rate on my library, but I have no idea if it will work in other regions. The Audible Plus plan is called "US Minerva", so I'm assuming that other regions are in the format "[COUNTRY] Minerva"

Question: We can cover books removed from the library and plus books no longer free. Are there any other ways that an audiobook may become unavailable to download?

This is what I currently have to detect and flag unavailable books.

protected override int DoImport(IEnumerable<ImportItem> importItems)
{
    bookImporter.Import(importItems);

    var qtyNew = upsertLibraryBooks(importItems);

    foreach (var item in DbContext.LibraryBooks)
    {
        //This SEEMS to work to detect plus titles which are no longer available.
        //I have my doubts it won't yield false negatives, but I have more
        //confidence that it won't give many/any false positives.
        var unavailable
            = importItems
            .Any(i =>
                i.DtoItem.ProductId == item.Book?.AudibleProductId &&
                i.AccountId == item.Account &&
                i.DtoItem.IsAyce is true &&
                !i.DtoItem.Plans.Any(p => p.PlanName.Contains("Minerva") || p.PlanName.Contains("Free Tier")));

        item.PresentInLastScan = item.Book is not null && !unavailable;
    }

    return qtyNew;
}

To display this info to users, I've slightly grayed out the stoplight icon added tooltip text to the "Liberate" button (only if the Book status is not "Liberated"):

This book cannot be downloaded
because it wasn't found during
the last library scan

Unfortunately there's no easy way to "disable" buttons in the grid view, but I was able to simulate the look. Here's what it looks like in WinForms.

Libation_IxXgRjlocY

Thoughts/Notes?

Mbucari commented 1 year ago

By the way, why does this query yield no results

var nullBooks = DbContext.LibraryBooks.Where(lb => lb.Book == null);

But iterating over DbContext.LibraryBooks does find null books?

Do you know of an article explaining Linq limitations on IQueryable<T>?

EDIT*

So using the Where<T>(IQueryable<T>, Expression<Func<T, bool>>) overload yields an empty set, but using the Where<T>(Enumerable<T>, Func<T, bool>) overload returns the expected items. What gives???

Edit 2**

Calling Where<T>(IQueryable<T>, Expression<Func<T, bool>>) on DbContext.LibraryBooks doesn't read any of the LibraryBooks. I put a breakpoint on LibraryBook.get_Book(), and it's never even called. Really, wtf?

rmcrackan commented 1 year ago

Unfortunately there's no easy way to "disable" buttons in the grid view, but I was able to simulate the look. Here's what it looks like in WinForms.

Those buttons are conditional logic anyway. We can extend the logic for 'is unavailable and not downloaded' to do nothing or to show a pop-up.

Where(IQueryable, Expression<Func<T, bool>>) overload yields an empty set

What happens if you 'ToList' it to bypass deferred execution?

rmcrackan commented 1 year ago

Good sleuthing above. I didn't know about "Free Tier". I was just thinking we needed another damn synonym ::sigh:: I like your handling of this. 1 point of minutia: you'll want to use ContainsInsensitive or similar so you don't get bit if a different region uses 'minerva' instead of 'Minerva' or lower-case t in 'Free tier' or something else which would be similarly frustrating to debug.

Mbucari commented 1 year ago

What happens if you 'ToList' it to bypass deferred execution?

That works as does calling AsEnumerable() first. It just really bugs me that I have no idea why defered IQuereyable.Where isn't working. Grrr!

I didn't know about "Free Tier".

For the most part, items in the "Free Tier" are podcasts. However there are at least some full-length audiobooks that "Free Tier" and not "Minerva". One example is this copy of War and Peace which is not "Minerva" but is "Ad Enabled Free Tier".

rmcrackan commented 1 year ago

why defered IQuereyable.Where isn't working

IQueryable is a set of rules on how to query data. It isn't yet data. You need the final step to go from queryable rules to queried information set. IQuereyable.Where is still building those rules up which will be executed once you run AsEnumerable, ToList, Count, etc

Mbucari commented 1 year ago

IQuereyable.Where is still building those rules up which will be executed once you run AsEnumerable, ToList, Count, etc

Sorry, I misunderstood your question about ToList.

Calling ToList or AsEnumerable before calling where works. Calling them after Where does not work.

This still returns an empty set.

DbContext.LibraryBooks.Where(lb => lb.Book == null).ToList();
rmcrackan commented 1 year ago

Probably an issue that you aren't yet Including Book. It's querying LibraryBooks and they have the book ID. However, the Book object isn't linked unless you do an include. Look at my library queries which load everything with includes. Related: no LibraryBook should exist without a linked book. Library book is a record of when an account got a book -- makes no sense without a book. I bet it's just the lack of loading.

rmcrackan commented 1 year ago

https://github.com/rmcrackan/Libation/blob/790319ed98c8dbb90989585b91848288da23fe4c/Source/DataLayer/QueryObjects/LibraryBookQueries.cs#L53-L58

Mbucari commented 1 year ago

Probably an issue that you aren't yet Including Book.

I don't think that's the issue because this query returns all books with 'Shakespear' in the title

DbContext.LibraryBooks.Where(lb => lb.Book.Title.Contains("Shakespear"));

I'm running these queries after bookImporter.Import(importItems); has run, so that should have linked the books. If a LibraryBook's Book wasn't included in importItems, then after the book importer runs those LibraryBooks' Books should be null. That's exactly what I find when I convert DbContext.LibraryBooks to IEnumerable, but when it's still IQueryable none of the Books are null.

Anyway, it's not important because I've got it working. I'm just still confused.

Edit* and I think I just answered my own question here. The book importer links the books to library books, but it only links books that are in the importItems set. Once I convert it to IEnumerable, any remaining library books are evaluated and show up with null books.

Thanks for the help!

How should these unavailable books be counted? Should unavailable books be included in PendingBooks, or should we subtract them off?

https://github.com/rmcrackan/Libation/blob/790319ed98c8dbb90989585b91848288da23fe4c/Source/ApplicationServices/LibraryCommands.cs#L456-L463

My thinking is we don't change how any of these stats are counted. The book is still in the library, and if it's not liberated it's still not liberated regardless of whether a user is able to download it. The only difference will be that you can't queue these books for download.

EDIT* Nevermind. Now I think we should treat them as error books for the purposes of counting and queuing.

rmcrackan commented 1 year ago

If a LibraryBook's Book wasn't included in importItems, then after the book importer runs those LibraryBooks' Books should be null.

Sorry, I can't make this sentence click for me. If a book wasn't included in importItems, then it doesn't make a new LibraryBooks. Where would the null LibraryBook come from?

How should these unavailable books be counted?

Good question. It doesn't feel like an error. Error is: not downloaded but for exceptional reasons. Downloaded vs available is orthogonal and you can have any combo. Clearly we won't enqueue an unavailable book. To me this feels like a new count type. If you have 3 books: 1 downloaded and avail, 1 downloaded and unavail, 1 not downloaded and avail: it feels right to have '2 downloaded, 2 not downloaded, 1 unavailable'. That's accurate but will it confuse the user. Maybe a new pipe section: BACKUPS: No progress: 1 In process: 0 Fully backed up: 2 | 2 of these are unavailable. Thoughts?

rmcrackan commented 1 year ago

Fixed in latest :)

manalive commented 1 year ago

how can users find unavailable books?

Mbucari commented 1 year ago

how can users find unavailable books?

Sort the "Liberate" column. All unavailable books will group together at the top or bottom and will be grayed out.