Solr
  1. Solr
  2. SOLR-7877

TestAuthenticationFramework.testBasics to preserve/restore the original request(Username|Password)

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 5.3, 6.0
    • Fix Version/s: 5.3, 5.4, 6.0
    • Component/s: None
    • Labels:
      None

      Description

      Build: http://jenkins.thetaphi.de/job/Lucene-Solr-trunk-Linux/13742/
      Java: 32bit/jdk1.8.0_60-ea-b24 -server -XX:+UseG1GC
      
      1 tests failed.
      FAILED:  org.apache.solr.cloud.TestAuthenticationFramework.testCollectionCreateWithoutCoresThenDelete
      
      Error Message:
      Error from server at http://127.0.0.1:51573/solr: Expected mime type application/octet-stream but got text/html. <html> <head> <meta http-equiv="Content-Type" content="text/html;charset=ISO-8859-1"/> <title>Error 401 </title> </head> <body> <h2>HTTP ERROR: 401</h2> <p>Problem accessing /solr/admin/collections. Reason: <pre>    Unauthorized request</pre></p> <hr /><i><small>Powered by Jetty://</small></i> </body> </html>
      
      Stack Trace:
      org.apache.solr.client.solrj.impl.HttpSolrClient$RemoteSolrException: Error from server at http://127.0.0.1:51573/solr: Expected mime type application/octet-stream but got text/html. <html>
      <head>
      <meta http-equiv="Content-Type" content="text/html;charset=ISO-8859-1"/>
      <title>Error 401 </title>
      </head>
      <body>
      <h2>HTTP ERROR: 401</h2>
      <p>Problem accessing /solr/admin/collections. Reason:
      <pre>    Unauthorized request</pre></p>
      <hr /><i><small>Powered by Jetty://</small></i>
      </body>
      </html>
      
              at __randomizedtesting.SeedInfo.seed([A454441B503006EB:17918BDA5F48D5AA]:0)
              at org.apache.solr.client.solrj.impl.HttpSolrClient.executeMethod(HttpSolrClient.java:528)
              at org.apache.solr.client.solrj.impl.HttpSolrClient.request(HttpSolrClient.java:234)
              at org.apache.solr.client.solrj.impl.HttpSolrClient.request(HttpSolrClient.java:226)
              at org.apache.solr.client.solrj.impl.LBHttpSolrClient.doRequest(LBHttpSolrClient.java:376)
              at org.apache.solr.client.solrj.impl.LBHttpSolrClient.request(LBHttpSolrClient.java:328)
              at org.apache.solr.client.solrj.impl.CloudSolrClient.sendRequest(CloudSolrClient.java:1086)
              at org.apache.solr.client.solrj.impl.CloudSolrClient.requestWithRetryOnStaleState(CloudSolrClient.java:856)
              at org.apache.solr.client.solrj.impl.CloudSolrClient.request(CloudSolrClient.java:799)
              at org.apache.solr.client.solrj.SolrClient.request(SolrClient.java:1220)
              at org.apache.solr.cloud.MiniSolrCloudCluster.makeCollectionsRequest(MiniSolrCloudCluster.java:349)
              at org.apache.solr.cloud.MiniSolrCloudCluster.createCollection(MiniSolrCloudCluster.java:333)
              at org.apache.solr.cloud.TestMiniSolrCloudCluster.createCollection(TestMiniSolrCloudCluster.java:115)
              at org.apache.solr.cloud.TestMiniSolrCloudCluster.testCollectionCreateWithoutCoresThenDelete(TestMiniSolrCloudCluster.java:298)
      

        Issue Links

          Activity

          Hide
          Christine Poerschke added a comment -

          TestAuthenticationFramework overrides the testBasics method but does not override the testCollectionCreateWithoutCoresThenDelete method. Would this mean that if testCollectionCreateWithoutCoresThenDelete runs after testBasics the former is bound to fail because it will use the requestUsername and requestPassword last set by the latter?

            public class TestAuthenticationFramework extends TestMiniSolrCloudCluster {
            ...
            static String requestUsername = MockAuthenticationPlugin.expectedUsername;
            static String requestPassword = MockAuthenticationPlugin.expectedPassword;
            ...
            @Test
            @Override
            public void testBasics() throws Exception {
              requestUsername = MockAuthenticationPlugin.expectedUsername;
              requestPassword = MockAuthenticationPlugin.expectedPassword;
              
              final String collectionName = "testAuthenticationFrameworkCollection";
              
              // Should pass
              testCollectionCreateSearchDelete(collectionName);
              
              requestUsername = MockAuthenticationPlugin.expectedUsername;
              requestPassword = "junkpassword";
              
              // Should fail with 401
              try {
                testCollectionCreateSearchDelete(collectionName);
                fail("Should've returned a 401 error");
              } catch (Exception ex) {
                if (!ex.getMessage().contains("Error 401")) {
                  fail("Should've returned a 401 error");
                }
              }
            }
          
          Show
          Christine Poerschke added a comment - TestAuthenticationFramework overrides the testBasics method but does not override the testCollectionCreateWithoutCoresThenDelete method. Would this mean that if testCollectionCreateWithoutCoresThenDelete runs after testBasics the former is bound to fail because it will use the requestUsername and requestPassword last set by the latter? public class TestAuthenticationFramework extends TestMiniSolrCloudCluster { ... static String requestUsername = MockAuthenticationPlugin.expectedUsername; static String requestPassword = MockAuthenticationPlugin.expectedPassword; ... @Test @Override public void testBasics() throws Exception { requestUsername = MockAuthenticationPlugin.expectedUsername; requestPassword = MockAuthenticationPlugin.expectedPassword; final String collectionName = "testAuthenticationFrameworkCollection" ; // Should pass testCollectionCreateSearchDelete(collectionName); requestUsername = MockAuthenticationPlugin.expectedUsername; requestPassword = "junkpassword" ; // Should fail with 401 try { testCollectionCreateSearchDelete(collectionName); fail( "Should've returned a 401 error" ); } catch (Exception ex) { if (!ex.getMessage().contains( "Error 401" )) { fail( "Should've returned a 401 error" ); } } }
          Hide
          Christine Poerschke added a comment -

          Test result inspection confirms the theory of test execution ordering within TestAuthenticationFramework mattering. I will make a change so that TestAuthenticationFramework.testBasics restores requestUsername and requestPassword to the values they were at the beginning of the method.

          Show
          Christine Poerschke added a comment - Test result inspection confirms the theory of test execution ordering within TestAuthenticationFramework mattering. I will make a change so that TestAuthenticationFramework.testBasics restores requestUsername and requestPassword to the values they were at the beginning of the method.
          Hide
          ASF subversion and git services added a comment -

          Commit 1694314 from Christine Poerschke in branch 'dev/trunk'
          [ https://svn.apache.org/r1694314 ]

          SOLR-7877: TestAuthenticationFramework.testBasics to preserve/restore the original request(Username|Password)

          Show
          ASF subversion and git services added a comment - Commit 1694314 from Christine Poerschke in branch 'dev/trunk' [ https://svn.apache.org/r1694314 ] SOLR-7877 : TestAuthenticationFramework.testBasics to preserve/restore the original request(Username|Password)
          Hide
          ASF subversion and git services added a comment -

          Commit 1694322 from Christine Poerschke in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1694322 ]

          SOLR-7877: TestAuthenticationFramework.testBasics to preserve/restore the original request(Username|Password)

          Show
          ASF subversion and git services added a comment - Commit 1694322 from Christine Poerschke in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1694322 ] SOLR-7877 : TestAuthenticationFramework.testBasics to preserve/restore the original request(Username|Password)
          Hide
          ASF subversion and git services added a comment -

          Commit 1694333 from Christine Poerschke in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1694333 ]

          SOLR-7877: TestAuthenticationFramework.testBasics to preserve/restore the original request(Username|Password) - svn r1694322 missed out the svn:mergeinfo for branch_5x itself, oops, sorry.

          Show
          ASF subversion and git services added a comment - Commit 1694333 from Christine Poerschke in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1694333 ] SOLR-7877 : TestAuthenticationFramework.testBasics to preserve/restore the original request(Username|Password) - svn r1694322 missed out the svn:mergeinfo for branch_5x itself, oops, sorry.
          Hide
          Noble Paul added a comment - - edited

          Can we move that test out of TestMiniSolrCloudCluster . So many tests extend that and this will lead to many errors in the future as well
          I mean , this is not the right fix

          Show
          Noble Paul added a comment - - edited Can we move that test out of TestMiniSolrCloudCluster . So many tests extend that and this will lead to many errors in the future as well I mean , this is not the right fix
          Hide
          Gregory Chanan added a comment -

          Can we move that test out of TestMiniSolrCloudCluster . So many tests extend that and this will lead to many errors in the future as well.

          +1. One of the main motivations for the MiniSolrCloudCluster was that people could use it in a has-a manner (i.e. as a variable in the test) rather than as an is-a manner (i.e. deriving from some complicated test base). Demonstrating that was the motivation behind writing the TestMiniSolrCloudCluster (i.e. that you could have a SolrCloud test that exists outside of the SolrTest framework). It looks like folks find it useful to have it available in an is-a manner; that's fine, but deriving from a class that has actual test methods sounds like a bad idea. Maybe we should write a TestMiniSolrCloudClusterBase that has no test methods and convert all the tests that derive from TestMiniSolrCloudCluster to use that.

          Show
          Gregory Chanan added a comment - Can we move that test out of TestMiniSolrCloudCluster . So many tests extend that and this will lead to many errors in the future as well. +1. One of the main motivations for the MiniSolrCloudCluster was that people could use it in a has-a manner (i.e. as a variable in the test) rather than as an is-a manner (i.e. deriving from some complicated test base). Demonstrating that was the motivation behind writing the TestMiniSolrCloudCluster (i.e. that you could have a SolrCloud test that exists outside of the SolrTest framework). It looks like folks find it useful to have it available in an is-a manner; that's fine, but deriving from a class that has actual test methods sounds like a bad idea. Maybe we should write a TestMiniSolrCloudClusterBase that has no test methods and convert all the tests that derive from TestMiniSolrCloudCluster to use that.
          Hide
          Ramkumar Aiyengar added a comment -

          I agree, I think in general, deriving tests is a bad idea. I once pulled out `test` virtual method for this very reason from AbstractZkTestCase.. If many tests extend a particular test suite, then they should be changed to not do it.

          Show
          Ramkumar Aiyengar added a comment - I agree, I think in general, deriving tests is a bad idea. I once pulled out `test` virtual method for this very reason from AbstractZkTestCase.. If many tests extend a particular test suite, then they should be changed to not do it.
          Hide
          Christine Poerschke added a comment -

          +1 for refactoring TestMiniSolrCloudCluster - just created SOLR-7886 for it.

          Show
          Christine Poerschke added a comment - +1 for refactoring TestMiniSolrCloudCluster - just created SOLR-7886 for it.
          Hide
          Christine Poerschke added a comment -

          Hi Noble Paul - thanks for your comments and patch. I just created SOLR-7886 for the refactoring discussion and efforts. If it's possible, could i suggest transferring the patch attachment to that JIRA ticket (and removing it from this JIRA ticket to avoid confusion between the changes already committed for this JIRA as per above and the currently attached, subsequent, separate patch)? My reason for not yet resolving this ticket here is that the merge to branches/lucene_solr_5_3 is yet to happen, i will do that tomorrow/Friday morning London time if that's okay. Thank you. - Christine

          Show
          Christine Poerschke added a comment - Hi Noble Paul - thanks for your comments and patch. I just created SOLR-7886 for the refactoring discussion and efforts. If it's possible, could i suggest transferring the patch attachment to that JIRA ticket (and removing it from this JIRA ticket to avoid confusion between the changes already committed for this JIRA as per above and the currently attached, subsequent, separate patch)? My reason for not yet resolving this ticket here is that the merge to branches/lucene_solr_5_3 is yet to happen, i will do that tomorrow/Friday morning London time if that's okay. Thank you. - Christine
          Hide
          Christine Poerschke added a comment -

          to do still (tomorrow/Friday morning London time): merge branch_5x (or should it be trunk?) commit to branches/lucene_solr_5_3

          Show
          Christine Poerschke added a comment - to do still (tomorrow/Friday morning London time): merge branch_5x (or should it be trunk ?) commit to branches/lucene_solr_5_3
          Hide
          Noble Paul added a comment -

          sure, I'll remove that patch. It is committed to trunk, branch_5x already

          Show
          Noble Paul added a comment - sure, I'll remove that patch. It is committed to trunk, branch_5x already
          Hide
          ASF subversion and git services added a comment -

          Commit 1694689 from Christine Poerschke in branch 'dev/branches/lucene_solr_5_3'
          [ https://svn.apache.org/r1694689 ]

          svn merge for revisions 1694322 and 1694333 from branch_5x (corresponding to revision 1694314 from trunk)
          SOLR-7877: TestAuthenticationFramework.testBasics to preserve/restore the original request(Username|Password)

          Show
          ASF subversion and git services added a comment - Commit 1694689 from Christine Poerschke in branch 'dev/branches/lucene_solr_5_3' [ https://svn.apache.org/r1694689 ] svn merge for revisions 1694322 and 1694333 from branch_5x (corresponding to revision 1694314 from trunk) SOLR-7877 : TestAuthenticationFramework.testBasics to preserve/restore the original request(Username|Password)
          Hide
          Christine Poerschke added a comment -

          Change committed to trunk, branch_5x and lucene_solr_5_3. Please see SOLR-7886 re: TestMiniSolrCloudCluster refactoring discussion and efforts.

          Show
          Christine Poerschke added a comment - Change committed to trunk, branch_5x and lucene_solr_5_3. Please see SOLR-7886 re: TestMiniSolrCloudCluster refactoring discussion and efforts.
          Hide
          Shalin Shekhar Mangar added a comment -

          Bulk close for 5.3.0 release

          Show
          Shalin Shekhar Mangar added a comment - Bulk close for 5.3.0 release

            People

            • Assignee:
              Christine Poerschke
              Reporter:
              Noble Paul
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development