jez500 / chorus

A Web UI for XBMC, focused on user experience and music. Get the latest version here: https://github.com/xbmc/chorus2
357 stars 54 forks source link

Search function doesn't work consistently #2

Closed kopf closed 10 years ago

kopf commented 10 years ago

Browsing by artist, we see the album "Rock and roll night club":

Searching for the term "rock and roll" returns no hits:

jez500 commented 10 years ago

Hi kopf,

Search is still a bit flakey, but it could also be a limit was hit with the caches. Can you open a javascript console, type in "app.stores" and tell me how many artists/albums/songs are cached

kopf commented 10 years ago

OK, did the following:

albums: Array[0]
all: Array[0]
allAlbums: child
allArtists: child
artists: Array[0]
genres: Array[0]
songs: Array[0]
__proto__: Object
Uncaught RangeError: Maximum call stack size exceeded chorus.js:10503
_.extend.set chorus.js:10503
options.success chorus.js:10643
successCb chorus.js:11754
Rpc.onSuccess chorus.js:11479
(anonymous function) chorus.js:11530
fire chorus.js:3048
self.fireWith chorus.js:3160
done chorus.js:8235
callback chorus.js:8778

God knows what the last traceback is about - most likely something completely unrelated - but I thought I'd include it anyhow.

So, it looks like my cache isn't being populated at all. This is on Google Chrome Version 32.0.1700.77, OS X, using chorus 0.1.7 (installed from the zip in the repo)

jez500 commented 10 years ago

from a bit of testing I actually got a similar result: Array[0] however when I expanded, it had the correct length (and items), maybe chrome struggling with such a large array? anyway doing this gave me correct numbers:

app.stores.allAlbums.length
app.stores.allArtists.length

(all the other keys in app.stores are deprecated and will be removed)

Both of these get populated asynchronously on page load, to know if allAlbums is loaded, go to any album, or for allArtists, just wait for the sidebar to fill with artists. I think I will build it into the ui somewhere soon though.

If your allAlbums is populated maybe try and loop over each in the console and see if you can find it

  var key = "rock and roll",
  albums = app.stores.allAlbums.filter(function (element) {
    var label = element.attributes.label;
    return label.toLowerCase().indexOf(key.toLowerCase()) > -1;
  });
  console.log(albums);

I am worried that that second error means chrome can't handle the amount of data we are trying to shove into it, possibly it is truncating collections? Or as you say it could be unrelated.

kopf commented 10 years ago

from a bit of testing I actually got a similar result: Array[0] however when I expanded, it had the correct length (and items), maybe chrome struggling with such a large array?

Ah! Strange. OK, looks like it was indeed a cache size limit:

app.stores.allArtists.length
3295
app.stores.allAlbums.length
5000

By the way, if it would be a help, I can always make a dump of my MySQL mymusic32 database and you can load that in to an otherwise empty xbmc installation for testing.

jez500 commented 10 years ago

There is a max length on retrieved albums, and it was set to 5000, just pushed v0.1.8 with that increased to 15000 for albums and 10000 for artists, see how you go with that, hopefully your browser doesn't explode.

kopf commented 10 years ago

OK, with 0.1.8, I get 2 of the following messages upon loading the root resource http://htpc:10000/

Uncaught Error code: -32603 - message: Internal error. chorus.js:11666
kopf commented 10 years ago

Sorry. That must've been caused by something completely unrelated. I inspected the source, and was still on the old version of chorus (this bloody internal add-on package cache in xbmc's %appdata% folder caught me out again)

I now got the proper version installed. Search results are coming up as expected. Thanks!

I find myself wondering if it's a good idea to impose limits on the number of artists and albums and so on. If a user has an insanely large music collection, they can expect something like a browser crash the first time they try chorus. However, as shown by the fact that I created this issue, patchy search results can't be so easily explained. This is even worse now that the limit is at the high limit of 15k albums - meaning that cases where results are missing from a search happen very rarely and will therefore be all the more inexplicable.

jez500 commented 10 years ago

Yeah I was thinking the same thing as I was changing it, due to the query structure I kinda need to set a limit, but I might just make it stupidly high.

Hopefully that fixed your search issue in the interim.

kopf commented 10 years ago

Closing this issue, as upping the limit has fixed it.

Should keep this in mind in case such a high limit leads to problems in the future.