edwellbrook / node-tvdb

Node.js library for accessing TheTVDB API
https://edwellbrook.github.io/node-tvdb/
MIT License
66 stars 28 forks source link

ZIP for getSeriesAllById #7

Closed watdafox closed 9 years ago

watdafox commented 9 years ago

I would really love to have the hability to use the zip instead of the xml for getSeriesAllById.

Example for "The Big Bang Theory" Series with the 'fr' lang request: 238KB of xml, 47KB of zip.

edwellbrook commented 9 years ago

nice idea! would you like to be handling the zipped data yourself, or for the data to be unzipped by node-tvdb and then that data parsed and presented back to you?

watdafox commented 9 years ago

The way I see it, but that's me and my lazyness talking, it would be something like:

  1. User requests zip
  2. (node-tvdb) Get localized zip
  3. (node-tvdb) Unzip the xml file
  4. (node-tvdb) Parse xml
  5. (node-tvdb) return object
  6. User gets readable data.series.episode

Basically the user could have the same response as getSeriesAllById, but would reduce brandwidth impact (and more local calculations/process). Benefit mostly to TVDB servers.

edwellbrook commented 9 years ago

yup, thought so. looking into this all now, and ideally that's how we'd want to do it. I'll take a look at this when i have some time, but please feel free to submit a PR if you want to :smile:

watdafox commented 9 years ago

I looked a little before submitting but simply looking at

fs.createWriteStream() -> .pipe(require('unzip')())

or

var pack = new AdmZip(zip) -> pack.extractAllToAsync(dir, true, function())

makes me itchy^^

watdafox commented 9 years ago

you're the real MVP

edwellbrook commented 9 years ago

:smile: should be updated on npm as v1.3.0 as well now!

watdafox commented 9 years ago

mmmh, I don't seem able to have it working.

in responseOk(error, resp, data), the data is undefined.

edwellbrook commented 9 years ago

can you give me some example code to reproduce?

edwellbrook commented 9 years ago

or whatever you used that caused the error :)

watdafox commented 9 years ago
var Client = require('node-tvdb');
var tvdb = new Client('5B757353E14FA17B', 'fr');
tvdb.getSeriesAllById('80379').then(function (localization) {
    console.log(localization.Overview);
});

1

The error is within the module, not in my code as far as i can tell

watdafox commented 9 years ago

got it !

in sendRequest function:

            if (!responseOk(error, resp, data.toString('utf8'))) {
                if (!error) {
                    error = new Error("Could not complete the request");
                }
                error.statusCode = resp.statusCode;

                return (callback ? callback : reject)(error);

It's a buffer when using RESPONSE_TYPE.ZIP

edwellbrook commented 9 years ago

does this happen every time for you? worked fine first time for me (although i did spot a couple of other bugs)

watdafox commented 9 years ago

data is a buffer and you pass it to responseOk() that doesn't seem to take a buffer

watdafox commented 9 years ago

probably better, handle the buffer in responseOk with:

if (data.toString().indexOf("404 Not Found") !== -1) return false;

or add something to check if it's a buffer or not.

edwellbrook commented 9 years ago

yep, writing a fix to handle buffers in responseOk

watdafox commented 9 years ago

imo you can just:

function responseOk(error, resp, data) {
    if (error) return false;
    if (resp.statusCode !== 200) return false;
    if (!data) return false;
    if (data === "") return false;
    if (data.toString().indexOf("404 Not Found") !== -1) return false;

    return true;
}

because even if it's not a buffer, it will work. And it won't error if no "data" is passed because of your if (!data) return false.

Not sure you have to check anything

edwellbrook commented 9 years ago

check my latest code. if it works, i'll update on npm :)

watdafox commented 9 years ago

works fine here, thanks

edwellbrook commented 9 years ago

v1.3.1 on npm. please let me know if this issue comes up again or if you come across anything else! :)

watdafox commented 9 years ago

so far I only use getAllSeriesById^^ but probably switching our image calls from trakt to tvdb as trakt is slower

edwellbrook commented 9 years ago

ah nice. awesome stuff :) do let me know if there's anything that you need/want improved when you do that!

watdafox commented 9 years ago

well, if you're into webapps in js, android apps, ios apps... we always can use an extra hand at git.popcorntime.io, don't hesitate to step by and say hello, or on the forums discuss.popcorntime.io