[mb-commits] r13717 - in search_server/trunk: index/src/main/java/org/musicbrainz/search/index index/src/test/java/org/musicbrainz/search/analysis index/src/test/java/org/musicbrainz/search/index servlet/src/test/java/org/musicbrainz/search/servlet

[email protected] Thu, 03 Oct 2013 11:10:32 +0000
Newsgroups gmane.comp.audio.musicbrainz.cvs
Message-ID <E1VRgng-0001Ca-JM@wiley>
Author: ijabz
Date: 2013-10-03 11:10:30 +0000 (Thu, 03 Oct 2013)
New Revision: 13717

Added:
   search_server/trunk/index/src/test/java/org/musicbrainz/search/analysis/IssueSearch258Test.java
Modified:
   search_server/trunk/index/src/main/java/org/musicbrainz/search/index/RecordingIndex.java
   search_server/trunk/index/src/main/java/org/musicbrainz/search/index/RecordingIndexField.java
   search_server/trunk/index/src/test/java/org/musicbrainz/search/index/RecordingIndexTest.java
   search_server/trunk/servlet/src/test/java/org/musicbrainz/search/servlet/FindRecordingTest.java
Log:
SEARCH-122:Recording Search now adds release artist credit if different to recording artist credit/ track artist

Modified: search_server/trunk/index/src/main/java/org/musicbrainz/search/index/RecordingIndex.java
===================================================================
--- search_server/trunk/index/src/main/java/org/musicbrainz/search/index/RecordingIndex.java	2013-10-02 15:01:43 UTC (rev 13716)
+++ search_server/trunk/index/src/main/java/org/musicbrainz/search/index/RecordingIndex.java	2013-10-03 11:10:30 UTC (rev 13717)
@@ -106,6 +106,7 @@
     }
 
     String releases;
+    String releaseArtistCredits;
     String releaseEvents;
     String releaseSecondaryTypes;
 
@@ -223,6 +224,22 @@
                         " AND a.artistId!='" + ArtistIndex.VARIOUS_ARTIST_MBID +"'" +
                         " AND a.artistId!='" + ArtistIndex.UNKNOWN_ARTIST_MBID  +"'" +
                         " ORDER BY r.id, a.pos, aa.name");
+
+        releaseArtistCredits =
+                "SELECT r.id as releaseKey, " +
+                        "  a.artist_credit, " +
+                        "  a.pos, " +
+                        "  a.joinphrase, " +
+                        "  a.artistId,  " +
+                        "  a.comment, " +
+                        "  a.artistName, " +
+                        "  a.artistCreditName, " +
+                        "  a.artistSortName " +
+                        " FROM release AS r " +
+                        "  INNER JOIN tmp_artistcredit a ON r.artist_credit=a.artist_credit " +
+                        " WHERE a.artistId!='" + ArtistIndex.VARIOUS_ARTIST_MBID +"'" +
+                        " AND r.id in ";
+
         releases =
                 "SELECT " +
                         "  id as releaseKey, gid as releaseid, name as releasename, type, " +
@@ -417,6 +434,42 @@
     }
 
     /**
+     * Get Release Artist Credit
+     *
+     * @param min min recording id
+     * @param max max recording id
+     * @return A map of matches
+     * @throws SQLException if sql problem
+     * @throws IOException  if io exception
+     */
+    private Map<Integer, ArtistCreditWrapper> loadReleaseArtists(Map<Integer, Release> releases,int min, int max) throws SQLException, IOException {
+
+        //Add release artists
+        PreparedStatement stmt = createReleaseArtistCreditsStatement(releases.size());
+        int count = 1;
+        for (Integer key : releases.keySet()) {
+            stmt.setInt(count, key);
+            count++;
+        }
+
+        ResultSet rs = stmt.executeQuery();
+        Map<Integer, ArtistCreditWrapper> releaseArtistCredits
+                = ArtistCreditHelper.completeArtistCreditFromDbResults
+                (rs,
+                        "releaseKey",
+                        "artist_Credit",
+                        "artistId",
+                        "artistName",
+                        "artistSortName",
+                        "comment",
+                        "joinphrase",
+                        "artistCreditName"
+                );
+        rs.close();
+        return  releaseArtistCredits;
+    }
+
+    /**
      * Get track  information for recordings
      * <p/>
      * One recording can be linked to by multiple tracks
@@ -529,6 +582,31 @@
     }
 
     /**
+     * Create the release artist credits statement
+     *
+     * @param noOfElements
+     * @return
+     * @throws SQLException
+     */
+    private PreparedStatement createReleaseArtistCreditsStatement(int noOfElements) throws SQLException {
+        StringBuilder inClause = new StringBuilder();
+        boolean firstValue = true;
+        for (int i = 0; i < noOfElements; i++) {
+            if (firstValue) {
+                firstValue = false;
+            } else {
+                inClause.append(',');
+            }
+            inClause.append('?');
+        }
+        PreparedStatement stmt = dbConnection.prepareStatement(
+                releaseArtistCredits + "(" + inClause.toString() + ')');
+        return stmt;
+
+    }
+
+
+    /**
      * Get release information for recordings
      *
      * @param tracks
@@ -590,7 +668,8 @@
             ml.setTrackCount(BigInteger.valueOf(rs.getInt("tracks")));
             release.setReleaseGroup(rg);
             release.setMediumList(ml);
-
+            //Manually create various artist artists they are not retrieved from the database
+            //for performance reasons
             if (rs.getInt("artist_credit") == VARIOUS_ARTIST_CREDIT_ID) {
                 ArtistCredit ac = createVariousArtistsCredit();
                 release.setArtistCredit(ac);
@@ -598,6 +677,7 @@
         }
         rs.close();
 
+
         //Add ReleaseEvents for each Release
         stmt = createReleaseEventStatement(releaseKeys.size());
         count = 1;
@@ -667,6 +747,7 @@
         Map<Integer, ArtistCreditWrapper>   trackArtistCredits  = updateTrackArtistCreditWithAliases(loadTrackArtists(min, max), min, max);
         Map<Integer, List<TrackWrapper>>    tracks              = loadTracks(min, max);
         Map<Integer, Release>               releases            = loadReleases(tracks);
+        Map<Integer, ArtistCreditWrapper>   releaseArtists      = loadReleaseArtists(releases, min, max);
 
         PreparedStatement st = getPreparedStatement("RECORDINGS");
         st.setInt(1, min);
@@ -675,7 +756,7 @@
         ResultSet rs = st.executeQuery();
         recordingClock.suspend();
         while (rs.next()) {
-            indexWriter.addDocument(documentFromResultSet(rs, tags, isrcs, artistCredits, trackArtistCredits, tracks, releases));
+            indexWriter.addDocument(documentFromResultSet(rs, tags, isrcs, artistCredits, trackArtistCredits, tracks, releases, releaseArtists));
         }
         rs.close();
 
@@ -687,7 +768,8 @@
                                           Map<Integer, ArtistCreditWrapper> artistCredits,
                                           Map<Integer, ArtistCreditWrapper> trackArtistCredits,
                                           Map<Integer, List<TrackWrapper>> tracks,
-                                          Map<Integer, Release> releases) throws SQLException {
+                                          Map<Integer, Release> releases,
+                                          Map<Integer, ArtistCreditWrapper>   releaseArtists) throws SQLException {
 
         buildClock.resume();
         Set<Integer> durations = new HashSet<Integer>();
@@ -772,6 +854,10 @@
 
 
                 if (origRelease != null) {
+
+                    //Get Artist Credit for Track
+                    ArtistCreditWrapper taw = trackArtistCredits.get(trackWrapper.getTrackId());
+
                     //This release instance will be shared by all recordings that have a track on the release so we need
                     //to copy details so we can append track specific details
                     Release release = of.createRelease();
@@ -780,13 +866,33 @@
                     MediumList ml = of.createMediumList();
                     release.setReleaseGroup(origRelease.getReleaseGroup());
                     release.setStatus(origRelease.getStatus());
+                    //This will be a va release
                     if(origRelease.getArtistCredit()!=null)
                     {
                         release.setArtistCredit(origRelease.getArtistCredit());
-                        doc.addField(RecordingIndexField.RELEASE_AC_VA, "1");
                     }
-                    else {
-                        doc.addField(RecordingIndexField.RELEASE_AC_VA, Index.NO_VALUE);
+                    //Only add if release artist credit is different to track artist, or if that not set
+                    //because same as recording artist only set if different to recording artist
+                    else
+                    {
+                        ArtistCreditWrapper racWrapper = releaseArtists.get(trackWrapper.getReleaseId());
+                        if(racWrapper!=null)
+                        {
+                            if(taw!=null)
+                            {
+                                if(taw.getArtistCreditId()!=racWrapper.getArtistCreditId())
+                                {
+                                    release.setArtistCredit(racWrapper.getArtistCredit());
+                                }
+                            }
+                            else if(ac!=null)
+                            {
+                                if(ac.getArtistCreditId()!=racWrapper.getArtistCreditId())
+                                {
+                                    release.setArtistCredit(racWrapper.getArtistCredit());
+                                }
+                            }
+                        }
                     }
 
                     ml.setTrackCount(origRelease.getMediumList().getTrackCount());
@@ -879,8 +985,6 @@
                     doc.addField(RecordingIndexField.POSITION, String.valueOf(trackWrapper.getMediumPosition()));
                     doc.addFieldOrNoValue(RecordingIndexField.FORMAT, trackWrapper.getMediumFormat());
 
-                    //Get Artist Credit for Track
-                    ArtistCreditWrapper taw = trackArtistCredits.get(trackWrapper.getTrackId());
                     //If different to the Artist Credit for the recording
                     if (taw != null &&
                             (

Modified: search_server/trunk/index/src/main/java/org/musicbrainz/search/index/RecordingIndexField.java
===================================================================
--- search_server/trunk/index/src/main/java/org/musicbrainz/search/index/RecordingIndexField.java	2013-10-02 15:01:43 UTC (rev 13716)
+++ search_server/trunk/index/src/main/java/org/musicbrainz/search/index/RecordingIndexField.java	2013-10-03 11:10:30 UTC (rev 13717)
@@ -36,7 +36,6 @@
     RECORDING_ACCENT        ("recordingaccent",     MusicBrainzFieldTypes.TEXT_NOT_STORED_ANALYZED, new MusicbrainzKeepAccentsAnalyzer()),
     RELEASE				    ("release",		        MusicBrainzFieldTypes.TEXT_NOT_STORED_ANALYZED, new TitleWithPosGapAnalyzer()),
     RELEASE_DATE	        ("date",		        MusicBrainzFieldTypes.TEXT_NOT_STORED_NOT_ANALYZED_NO_NORMS, new KeywordAnalyzer()),
-    RELEASE_AC_VA           ("release_ac_va",       MusicBrainzFieldTypes.TEXT_STORED_NOT_INDEXED),
     RELEASE_ID			    ("reid",		        MusicBrainzFieldTypes.TEXT_NOT_STORED_NOT_ANALYZED_NO_NORMS, new KeywordAnalyzer()),
     RELEASE_PRIMARY_TYPE    ("primarytype",		    MusicBrainzFieldTypes.TEXT_NOT_STORED_ANALYZED_NO_NORMS, new CaseInsensitiveKeywordAnalyzer()),
     RELEASE_SECONDARY_TYPE  ("secondarytype",       MusicBrainzFieldTypes.TEXT_NOT_STORED_ANALYZED_NO_NORMS, new CaseInsensitiveKeywordAnalyzer()),

Modified: search_server/trunk/index/src/test/java/org/musicbrainz/search/index/RecordingIndexTest.java
===================================================================
--- search_server/trunk/index/src/test/java/org/musicbrainz/search/index/RecordingIndexTest.java	2013-10-02 15:01:43 UTC (rev 13716)
+++ search_server/trunk/index/src/test/java/org/musicbrainz/search/index/RecordingIndexTest.java	2013-10-03 11:10:30 UTC (rev 13717)
@@ -47,9 +47,9 @@
                 " VALUES (16153, 'ccd4879c-5e88-4385-b131-bf65296bf245', 'Echo & The Bunnymen', 'Echo and The Bunnymen','')");
         stmt.addBatch("INSERT INTO artist_alias(id, artist, name, sort_name, locale, edits_pending, last_updated) VALUES (1, 16153, 'Echo & Bunneymen','Bunneymen & Echo', 'en',1,null)");
 
-        stmt.addBatch("INSERT INTO artist_credit (id, name, artist_count, ref_count) VALUES (1,  'Echo & The Bunnymen', 1, 1)");
+        stmt.addBatch("INSERT INTO artist_credit (id, name, artist_count, ref_count) VALUES (3,  'Echo & The Bunnymen', 1, 1)");
         stmt.addBatch("INSERT INTO artist_credit_name (artist_credit, position, artist, name, join_phrase) " +
-                " VALUES (1, 0, 16153, 'Echo & The Bunnymen', '')");
+                " VALUES (3, 0, 16153, 'Echo & The Bunnymen', '')");
 
         stmt.addBatch("INSERT INTO release_group (id, gid, name, artist_credit, type)" +
                 " VALUES (491240, 'efd2ace2-b3b9-305f-8a53-9803595c0e37', 'Crocodiles', 1, 1)");
@@ -59,7 +59,7 @@
 
         stmt.addBatch("INSERT INTO release (id, gid, name, artist_credit, release_group, status, packaging, " +
                 "  language, script) " +
-                " VALUES (491240, 'c3b8dbc9-c1ff-4743-9015-8d762819134e', 'Crocodiles (bonus disc)', 1, 491240, 1, 1, 1, 1)");
+                " VALUES (491240, 'c3b8dbc9-c1ff-4743-9015-8d762819134e', 'Crocodiles (bonus disc)', 3, 491240, 1, 1, 1, 1)");
 
         stmt.addBatch("INSERT INTO release_country (release, country, date_year, date_month, date_day) values (491240, 221, 1970,1,1)");
         stmt.addBatch("INSERT INTO area (id, gid, name, sort_name) VALUES (221, 'c3b8dbc9-c1ff-4743-9015-8d762819134g','United Kingdom','Kingdom of UK')");
@@ -80,13 +80,11 @@
         stmt.addBatch("INSERT INTO track (id, gid, recording, medium, position, number, name, artist_credit, length) "
                 + " VALUES (1, 'c3b8dbc9-c1ff-4743-9015-8d762819134e', 1, 1, 4, 'A4', 'Do It Cleans', 1, 33100)");
         stmt.addBatch("INSERT INTO recording (id, gid, name, artist_credit, length, comment, video)"
-                + " VALUES (1, '2f250ed2-6285-40f1-aa2a-14f1c05e9765', 'Do It Clean', 1, 33000, 'demo', true)");
+                + " VALUES (1, '2f250ed2-6285-40f1-aa2a-14f1c05e9765', 'Do It Clean', 3, 33000, 'demo', true)");
 
         stmt.addBatch("INSERT INTO isrc (id, recording, isrc) VALUES (1, 1, 'FRAAA9000038')");
         stmt.addBatch("INSERT INTO isrc (id, recording, isrc) VALUES (2, 1, 'FRAAA9100082')");
 
-        stmt.addBatch("INSERT INTO puid (id, puid) VALUES (1, 'efd2ace2-b3b9-305f-8a53-9803595c0e38')");
-        stmt.addBatch("INSERT INTO recording_puid (id, puid, recording) VALUES (1, 1, 1)");
 
         stmt.addBatch("INSERT INTO area (id, name) VALUES (1, 'United Kingdom')");
         stmt.addBatch("INSERT INTO iso_3166_1 (area, code) VALUES (1, 'UK')");
@@ -96,7 +94,7 @@
     }
 
     /**
-     * All Basic Fields Plus Release Events and different track artist to recording artist
+     * All Basic Fields Plus Release Events and different release artist and track artist to recording artist
      *
      * @throws Exception exception
      */
@@ -104,24 +102,34 @@
 
         Statement stmt = conn.createStatement();
 
+        //Recording
         stmt.addBatch("INSERT INTO artist (id, gid, name, sort_name, comment)" +
                 " VALUES (16153, 'ccd4879c-5e88-4385-b131-bf65296bf245', 'Echo & The Bunnymen', 1, 'a comment')");
         stmt.addBatch("INSERT INTO artist_credit (id, name, artist_count, ref_count) VALUES (1, 'Echo & The Bunnymen', 1, 1)");
         stmt.addBatch("INSERT INTO artist_credit_name (artist_credit, position, artist, name, join_phrase) " +
                 " VALUES (1, 0, 16153, 'Echo & The Bunnymen','')");
 
+        //Track
         stmt.addBatch("INSERT INTO artist (id, gid, name, sort_name, comment)" +
                 " VALUES (2, 'ddd4879c-5e88-4385-b131-bf65296bf245', 'Pixies','Pixies', 'a comment')");
         stmt.addBatch("INSERT INTO artist_credit (id, name, artist_count, ref_count) VALUES (2, 'Pixies', 1, 1)");
         stmt.addBatch("INSERT INTO artist_credit_name (artist_credit, position, artist, name) " +
                 " VALUES (2, 0, 2, 'Pixies')");
 
+        //Release
+        stmt.addBatch("INSERT INTO artist (id, gid, name, sort_name, comment)" +
+                " VALUES (3, 'eee4879c-5e88-4385-b131-bf65296bf245', 'Poxies','Poxies', 'a comment')");
+        stmt.addBatch("INSERT INTO artist_credit (id, name, artist_count, ref_count) VALUES (3, 'The Poxies', 1, 1)");
+        stmt.addBatch("INSERT INTO artist_credit_name (artist_credit, position, artist, name) " +
+                " VALUES (3, 0, 3, 'The Poxies')");
+
+
         stmt.addBatch("INSERT INTO release_group (id, gid, name, artist_credit)" +
                 " VALUES (491240, 'efd2ace2-b3b9-305f-8a53-9803595c0e37',  'Crocodiles', 1)");
 
         stmt.addBatch("INSERT INTO release (id, gid, name, artist_credit, release_group, packaging, " +
                 "  language, script) " +
-                " VALUES (491240, 'c3b8dbc9-c1ff-4743-9015-8d762819134e', 'Crocodiles (bonus disc)', 1, 491240, 1, 1, 1)");
+                " VALUES (491240, 'c3b8dbc9-c1ff-4743-9015-8d762819134e', 'Crocodiles (bonus disc)', 3, 491240, 1, 1, 1)");
         stmt.addBatch("INSERT INTO medium (id, track_count, release, position, format) VALUES (1, 2, 491240, 1, 7)");
         stmt.addBatch("INSERT INTO track (id, gid, recording, medium, position, name, artist_credit, length) "
                 + " VALUES (1, 'c3b8dbc9-c1ff-4743-9015-8d762819134e', 1, 1, 4, 'Do It Clean', 2, 33100)");
@@ -573,6 +581,93 @@
      * @throws Exception exception
      */
     @Test
+    public void testReleaseArtist() throws Exception {
+
+        addTrackTwo();
+        RAMDirectory ramDir = new RAMDirectory();
+        createIndex(ramDir);
+
+        IndexReader ir = DirectoryReader.open(ramDir);
+        assertEquals(2, ir.numDocs());
+        {
+            Document doc = ir.document(1);
+
+
+            Recording recording = (Recording) MMDSerializer.unserialize(doc.get(RecordingIndexField.RECORDING_STORE.getName()), Recording.class);
+            ArtistCredit ac = recording.getArtistCredit();
+            assertNotNull(ac);
+            assertEquals("Echo & The Bunnymen", ac.getNameCredit().get(0).getArtist().getName());
+
+
+            ac = recording.getReleaseList().getRelease().get(0).getArtistCredit();
+            assertNotNull(ac);
+            assertEquals("Poxies", ac.getNameCredit().get(0).getArtist().getName());
+            assertEquals("The Poxies", ac.getNameCredit().get(0).getName());
+        }
+        ir.close();
+    }
+
+    /**
+     * @throws Exception exception
+     */
+    @Test
+    public void testNoReleaseArtistBecauseSameAsRecordingArtist() throws Exception {
+
+        addTrackOne();
+        RAMDirectory ramDir = new RAMDirectory();
+        createIndex(ramDir);
+
+        IndexReader ir = DirectoryReader.open(ramDir);
+        assertEquals(2, ir.numDocs());
+        {
+            Document doc = ir.document(1);
+
+
+            Recording recording = (Recording) MMDSerializer.unserialize(doc.get(RecordingIndexField.RECORDING_STORE.getName()), Recording.class);
+            ArtistCredit ac = recording.getArtistCredit();
+            assertNotNull(ac);
+            assertEquals("Echo & The Bunnymen", ac.getNameCredit().get(0).getArtist().getName());
+
+
+            ac = recording.getReleaseList().getRelease().get(0).getArtistCredit();
+            assertNull(ac);
+        }
+        ir.close();
+    }
+
+    /**
+     * @throws Exception exception
+     */
+    @Test
+    public void testNoTrackArtistBecauseSameAsRecordingArtist() throws Exception {
+
+        addTrackOne();
+        RAMDirectory ramDir = new RAMDirectory();
+        createIndex(ramDir);
+
+        IndexReader ir = DirectoryReader.open(ramDir);
+        assertEquals(2, ir.numDocs());
+        {
+            Document doc = ir.document(1);
+
+
+            Recording recording = (Recording) MMDSerializer.unserialize(doc.get(RecordingIndexField.RECORDING_STORE.getName()), Recording.class);
+            ArtistCredit ac = recording.getArtistCredit();
+            assertNotNull(ac);
+            assertEquals("Echo & The Bunnymen", ac.getNameCredit().get(0).getArtist().getName());
+
+
+            ac = recording.getReleaseList().getRelease().get(0).getMediumList().getMedium().get(0).getTrackList().getDefTrack().get(0).getArtistCredit();
+            assertNull(ac);
+        }
+        ir.close();
+    }
+
+
+    /**
+     * @throws Exception exception
+     */
+    @Test
     public void testTag() throws Exception {
 
         addTrackTwo();

Modified: search_server/trunk/servlet/src/test/java/org/musicbrainz/search/servlet/FindRecordingTest.java
===================================================================
--- search_server/trunk/servlet/src/test/java/org/musicbrainz/search/servlet/FindRecordingTest.java	2013-10-02 15:01:43 UTC (rev 13716)
+++ search_server/trunk/servlet/src/test/java/org/musicbrainz/search/servlet/FindRecordingTest.java	2013-10-03 11:10:30 UTC (rev 13717)
@@ -179,7 +179,6 @@
             tagList.getTag().add(tag);
             release.setTagList(tagList);
 
-            doc.addField(RecordingIndexField.RELEASE_AC_VA, "1");
             Artist vaArtist = of.createArtist();
             vaArtist.setId("89ad4ac3-39f7-470e-963a-56509c546377");
             vaArtist.setName("Various Artists");