Loki-Afro / metalarchives

Unofficial Encyclopedia Metallum API
Other
44 stars 12 forks source link

getLyrics() returns null #12

Closed Aarkon closed 2 years ago

Aarkon commented 2 years ago

When calling getLyrics() on a given track, the result is always null.

Example:

import com.github.loki.afro.metallum.entity.Band;
import com.github.loki.afro.metallum.entity.Disc;
import com.github.loki.afro.metallum.entity.Track;
import com.github.loki.afro.metallum.search.API;
import com.github.loki.afro.metallum.search.query.entity.BandQuery;

import java.util.Collection;
import java.util.List;
import java.util.stream.Collectors;

public class Main {
    public static void main(String[] args) {
        for (final Band band : API.getBandsFully(BandQuery.byName("Mastodon", true))) {
            List<Track> tracks = band.getDiscs().stream().map(Disc::getTrackList).flatMap(Collection::stream).collect(Collectors.toList());
            List<String> lyrics = tracks.stream().map(Track::getLyrics).collect(Collectors.toList());
            System.out.println(lyrics);
        }
    }
}
// result:
// [null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null]

I've tried this with several bands in Java as well as in Scala.

Loki-Afro commented 2 years ago

meh, true. This has to do with how Partials were introduced and that i wanted to avoid additional calls to metalarchives for retrieving lyrics. Definitely a bug.

As a workaround however you can look here https://github.com/Loki-Afro/metalarchives/blob/be8272be53db09ba6ef80965cbf0e0e41e7e0417/src/test/java/com/github/loki/afro/metallum/search/service/advanced/DiscSearchServiceTest.java#L100

and modify your code to something like this as a mitigation

            List<Disc> discs = band.getDiscs();
            List<String> lyrics = discs.stream()
                    .map(disc -> new DiscSearchService(false, true).getById(disc.getId()))
                    .map(Disc::getTrackList)
                    .flatMap(Collection::stream)
                    .map(Track::getLyrics)
                    .collect(Collectors.toList());
Loki-Afro commented 2 years ago

@Aarkon i created a branch which fixes your issue, or better improves the api usage, the same improvement can be done for loading images as well, could you tell if you like this approach?

also when using parallelStream you can achieve much better results with this way

    @Test
    public void getLyrics() throws MetallumException {
        BandQuery query = BandQuery.builder().name("Mastodon")
                .exactBandNameMatch(true)
                .build();
        Iterable<Band> mastodon = API.getBandsFully(query);
        for (Band band : mastodon) {
            List<String> lyrics = band.getDiscs().parallelStream()
                    .map(Disc::getTrackList)
                    .flatMap(Collection::parallelStream)
                    .map(Track::getLyrics)
                    .filter(Optional::isPresent)
                    .map(Optional::get)
                    .collect(Collectors.toList());
            System.out.println(lyrics);
            break;

        }
    }
Aarkon commented 2 years ago

What would you like me to do exactly? I skimmed the code quickly, and it looks like you got rid of a lot - always a good sign! ;) Also, coming from Scala, I like that you adopt Optional types. Doing stuff in parallel is also a good idea when it comes to slow web scraping. I'd say: Go for it! :)

Loki-Afro commented 2 years ago

i would like that you build it yourself, try out if it works for in your case, cause if i go that path i will refactor more stuff in the same way

Aarkon commented 2 years ago

Ah, alright - I'll need to figure out how to use a local dependency from Scala for that though, because that is my use case. That may take some days because I'm really buisy with work and family, I hope that's OK for you!

Loki-Afro commented 2 years ago

sure, take your time

Aarkon commented 2 years ago

I tried this the other day, but I have to say that I fail in referencing this library from github with sbt as well as packaging it to a jar with all runtime dependencies. This is a library without a main method, so the usual maven plugins don't apply here. Any idea on how to do this (assuming you're not a Scala guy, I'd say how packaging to a full blown jar works)?

Aarkon commented 2 years ago

Actually, never mind, I've got it working doing this:

P.S.: You've got a failing because now lyrics are fetched and the assertion that they are null doesn't hold anymore. ;)

Loki-Afro commented 2 years ago

hi there, 1.1.0 was just released, containing the fix for lyrics and images like artworks or band photos. same "bug" however is still present for band

label

member