Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-8992

Restore Schema API GET method functionality removed by SOLR-8736

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 6.0.1, 6.1, 7.0
    • Component/s: None
    • Labels:
      None

      Description

      The following schema API GET functionality was removed under SOLR-8736; some of this functionality should be restored:

      • schema/copyfields:
        • The following information is no longer output:
          • destDynamicBase: the matching dynamic field pattern for the destination
          • sourceDynamicBase: the matching dynamic field pattern for the source
        • The following request parameters are no longer supported:
          • dest.fl: include only copyFields that have one of these as a destination
          • source.fl: include only copyFields that have one of these as a source
      • schema/dynamicfields:
        • The following request parameters are no longer supported:
          • fl: a comma and/or space separated list of dynamic field patterns to include
      • schema/fields and schema/fields/fieldname:
        • The following information is no longer output:
          • dynamicBase: the matching dynamic field pattern, if the includeDynamic param is given (see below)
        • The following request parameters are no longer supported:
          • fl: (only supported without /fieldname): a comma and/or space separated list of fields to include
          • includeDynamic: output the matching dynamic field pattern as dynamicBase, if fieldname, or field(s) listed in fl param, are not explicitly declared in the schema
      • schema/fieldtypes and schema/fieldtypes/typename:
        • The following information is no longer output:
          • fields: the fields with the given field type
          • dynamicFields: the dynamic fields with the given field type
      1. SOLR-8992.patch
        13 kB
        Noble Paul
      2. SOLR-8992.patch
        6 kB
        Noble Paul
      3. SOLR-8992.patch
        22 kB
        Noble Paul
      4. SOLR-8992-branch_6_0-missing-TestFieldCollectionResource-tests.patch
        2 kB
        Steve Rowe

        Issue Links

          Activity

          Hide
          steve_rowe Steve Rowe added a comment -

          Also, TestCloudManagedSchemaConcurrent was @Ignored under SOLR-8736, but no test was substituted. I haven't looked into whether the bulk schema API has concurrent cloud tests.

          Show
          steve_rowe Steve Rowe added a comment - Also, TestCloudManagedSchemaConcurrent was @Ignored under SOLR-8736 , but no test was substituted. I haven't looked into whether the bulk schema API has concurrent cloud tests.
          Hide
          noble.paul Noble Paul added a comment -

          TestBulkSchemaConcurrent is a what you are looking for

          Show
          noble.paul Noble Paul added a comment - TestBulkSchemaConcurrent is a what you are looking for
          Hide
          noble.paul Noble Paul added a comment -

          which of these are essential and must be restored ?

          Show
          noble.paul Noble Paul added a comment - which of these are essential and must be restored ?
          Hide
          steve_rowe Steve Rowe added a comment -

          I think asking which of these is "essential" is the wrong question.

          I think it's bad that we advertized these query parameters - and still do in the ref guide to become 6.0's! - and then just dropped them. I vote for putting them all back.

          I'm ambivalent about the removed output information (identified with "The following information is no longer output" above). +0 to not restore it.

          Show
          steve_rowe Steve Rowe added a comment - I think asking which of these is "essential" is the wrong question. I think it's bad that we advertized these query parameters - and still do in the ref guide to become 6.0's! - and then just dropped them. I vote for putting them all back. I'm ambivalent about the removed output information (identified with "The following information is no longer output" above). +0 to not restore it.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit e8cc19eb885c46d25b56fdd681825712516050c9 in lucene-solr's branch refs/heads/master from Noble Paul
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=e8cc19e ]

          SOLR-8992: Restore Schema API GET method functionality removed in 6.0

          Show
          jira-bot ASF subversion and git services added a comment - Commit e8cc19eb885c46d25b56fdd681825712516050c9 in lucene-solr's branch refs/heads/master from Noble Paul [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=e8cc19e ] SOLR-8992 : Restore Schema API GET method functionality removed in 6.0
          Hide
          steve_rowe Steve Rowe added a comment -

          Thanks for the patch Noble.

          A few things I noticed:

          1. Two SchemaProps.Handler enum values (SOLR_QUERY_PARSER and SIMILARITY) can have null lambdas (when they are not explicitly configured in the schema), but you don't guard against that in SchemaProps.toMap(), so NPEs are waiting to happen. There are no tests for these cases but there should be.
          2. In the previous IndexSchema.getNamedPropertyValues(), fields were output before dynamicFields, but the way you iterate your new SchemaProps.Handler enum in SchemaProps.toMap(), DYNAMIC_FIELDS will be called before FIELDS; as a result, the way whole schema output is ordered will change. I think that's bad.
          3. In the SchemaProps ctor there is repeated field list extraction code (for source.fl, dest.fl and fl params) that could be pulled out into a method and called from each of those places, e.g. here's the fl version:
            String[] fields = flParam.trim().split("[,\\s]+");
            if (fields.length > 0)
              requestedFields = new LinkedHashSet<>(Stream.of(fields)
                .filter(it -> !it.trim().isEmpty())
                .collect(Collectors.toList()));
            
          4. There are a couple new unused imports in IndexSchema.java: com.google.common.base.Functions and org.apache.solr.common.util.Pair.
          Show
          steve_rowe Steve Rowe added a comment - Thanks for the patch Noble. A few things I noticed: Two SchemaProps.Handler enum values ( SOLR_QUERY_PARSER and SIMILARITY ) can have null lambdas (when they are not explicitly configured in the schema), but you don't guard against that in SchemaProps.toMap() , so NPEs are waiting to happen. There are no tests for these cases but there should be. In the previous IndexSchema.getNamedPropertyValues() , fields were output before dynamicFields , but the way you iterate your new SchemaProps.Handler enum in SchemaProps.toMap() , DYNAMIC_FIELDS will be called before FIELDS ; as a result, the way whole schema output is ordered will change. I think that's bad. In the SchemaProps ctor there is repeated field list extraction code (for source.fl , dest.fl and fl params) that could be pulled out into a method and called from each of those places, e.g. here's the fl version: String [] fields = flParam.trim().split( "[,\\s]+" ); if (fields.length > 0) requestedFields = new LinkedHashSet<>(Stream.of(fields) .filter(it -> !it.trim().isEmpty()) .collect(Collectors.toList())); There are a couple new unused imports in IndexSchema.java : com.google.common.base.Functions and org.apache.solr.common.util.Pair .
          Hide
          noble.paul Noble Paul added a comment -

          In the previous IndexSchema.getNamedPropertyValues(), fields were output before dynamicFields,

          did we ever guarantee the order?

          Show
          noble.paul Noble Paul added a comment - In the previous IndexSchema.getNamedPropertyValues(), fields were output before dynamicFields, did we ever guarantee the order?
          Hide
          steve_rowe Steve Rowe added a comment -

          did we ever guarantee the order?

          No. But it will cost you nearly nothing to leave things the way they were.

          Show
          steve_rowe Steve Rowe added a comment - did we ever guarantee the order? No. But it will cost you nearly nothing to leave things the way they were.
          Hide
          noble.paul Noble Paul added a comment - - edited

          but you don't guard against that in SchemaProps.toMap(), so NPEs are waiting to happen.

          I think I have copied the code verbatim and it is guarded against NPEs

          Object val = it.fun.apply(this);
            if (val != null) topLevel.put(it.name, val);
          
          Show
          noble.paul Noble Paul added a comment - - edited but you don't guard against that in SchemaProps.toMap(), so NPEs are waiting to happen. I think I have copied the code verbatim and it is guarded against NPEs Object val = it.fun.apply( this ); if (val != null ) topLevel.put(it.name, val);
          Hide
          steve_rowe Steve Rowe added a comment -

          but you don't guard against that in SchemaProps.toMap(), so NPEs are waiting to happen.

          I think I have copied the code verbatim and it is guarded against NPEs

          Object val = it.fun.apply(this);
            if (val != null) topLevel.put(it.name, val);
          

          Which code are you talking about copying verbatim? The code below looks completely new to me? And it.fun is never checked for null.

              @Override
              public Map<String, Object> toMap() {
                Map<String, Object> topLevel = new LinkedHashMap<>();
                Stream.of(Handler.values())
                    .filter(it -> name == null || it.name.equals(name))
                    .forEach(it -> {
                      Object val = it.fun.apply(this);
                      if (val != null) topLevel.put(it.name, val);
                    });
                return topLevel;
              }
            }
          
          Show
          steve_rowe Steve Rowe added a comment - but you don't guard against that in SchemaProps.toMap(), so NPEs are waiting to happen. I think I have copied the code verbatim and it is guarded against NPEs Object val = it.fun.apply( this ); if (val != null ) topLevel.put(it.name, val); Which code are you talking about copying verbatim? The code below looks completely new to me? And it.fun is never checked for null. @Override public Map< String , Object > toMap() { Map< String , Object > topLevel = new LinkedHashMap<>(); Stream.of(Handler.values()) .filter(it -> name == null || it.name.equals(name)) .forEach(it -> { Object val = it.fun.apply( this ); if (val != null ) topLevel.put(it.name, val); }); return topLevel; } }
          Hide
          steve_rowe Steve Rowe added a comment -

          Crap, my misunderstanding about it.fun needing a null check - those lambdas will never be null, but they may return null. Sorry for the noise.

          Show
          steve_rowe Steve Rowe added a comment - Crap, my misunderstanding about it.fun needing a null check - those lambdas will never be null, but they may return null. Sorry for the noise.
          Hide
          steve_rowe Steve Rowe added a comment - - edited

          Noble,

          Looks like you didn't put back the ability to use schema/fields/fieldname&includeDynamic=true? When I put back this test you removed under SOLR-8736, it fails with error No such path /schema/fields/some_crazy_name_i:

          TestFieldResource.java
            public void testGetFieldIncludeDynamic() throws Exception {
              assertQ("/schema/fields/some_crazy_name_i?indent=on&wt=xml&includeDynamic=true",
                  "/response/lst[@name='field']/str[@name='name'] = 'some_crazy_name_i'",
                  "/response/lst[@name='field']/str[@name='dynamicBase'] = '*_i'");
            }
          
          Show
          steve_rowe Steve Rowe added a comment - - edited Noble, Looks like you didn't put back the ability to use schema/fields/ fieldname &includeDynamic=true ? When I put back this test you removed under SOLR-8736 , it fails with error No such path /schema/fields/some_crazy_name_i : TestFieldResource.java public void testGetFieldIncludeDynamic() throws Exception { assertQ( "/schema/fields/some_crazy_name_i?indent=on&wt=xml&includeDynamic= true " , "/response/lst[@name='field']/str[@name='name'] = 'some_crazy_name_i'" , "/response/lst[@name='field']/str[@name='dynamicBase'] = '*_i'" ); }
          Hide
          noble.paul Noble Paul added a comment -

          added the missing testcase

          Show
          noble.paul Noble Paul added a comment - added the missing testcase
          Hide
          hossman Hoss Man added a comment -

          every jenkins build that makes it far enough to run the solrj tests is failin on this test...

          ant test -Dtestcase=SchemaTest -Dtests.method=testSchemaRequestAccuracy -Dtests.seed=B07EE1D8472F7C65 -Dtests.slow=true -Dtests.locale=es-VE -Dtests.timezone=Europe/Nicosia -Dtests.asserts=true -Dtests.file.encoding=ISO-8859-1

          ...taht seed (along with every other seed i tried) is failing reliably for me...

          java.lang.ClassCastException: java.util.LinkedHashMap cannot be cast to org.apache.solr.common.util.NamedList
                  at __randomizedtesting.SeedInfo.seed([B07EE1D8472F7C65:3782EE7649D781E2]:0)
                  at org.apache.solr.client.solrj.response.schema.SchemaResponse.setResponse(SchemaResponse.java:252)
                  at org.apache.solr.client.solrj.SolrRequest.process(SolrRequest.java:149)
                  at org.apache.solr.client.solrj.SolrRequest.process(SolrRequest.java:166)
                  at org.apache.solr.client.solrj.request.SchemaTest.testSchemaRequestAccuracy(SchemaTest.java:123)
          

          the problem seems to be e8cc19eb885c46d25b56fdd681825712516050c9, revert to the previous SHA (2ee8426) and it passes

          Show
          hossman Hoss Man added a comment - every jenkins build that makes it far enough to run the solrj tests is failin on this test... ant test -Dtestcase=SchemaTest -Dtests.method=testSchemaRequestAccuracy -Dtests.seed=B07EE1D8472F7C65 -Dtests.slow=true -Dtests.locale=es-VE -Dtests.timezone=Europe/Nicosia -Dtests.asserts=true -Dtests.file.encoding=ISO-8859-1 ...taht seed (along with every other seed i tried) is failing reliably for me... java.lang.ClassCastException: java.util.LinkedHashMap cannot be cast to org.apache.solr.common.util.NamedList at __randomizedtesting.SeedInfo.seed([B07EE1D8472F7C65:3782EE7649D781E2]:0) at org.apache.solr.client.solrj.response.schema.SchemaResponse.setResponse(SchemaResponse.java:252) at org.apache.solr.client.solrj.SolrRequest.process(SolrRequest.java:149) at org.apache.solr.client.solrj.SolrRequest.process(SolrRequest.java:166) at org.apache.solr.client.solrj.request.SchemaTest.testSchemaRequestAccuracy(SchemaTest.java:123) the problem seems to be e8cc19eb885c46d25b56fdd681825712516050c9, revert to the previous SHA (2ee8426) and it passes
          Hide
          steve_rowe Steve Rowe added a comment -

          +1 to the latest patch. It did take me a while to grok the default case in SchemaHandler.handleGET() though - the local variable names (realName, fieldName, name, parts) are confusing. I suggest rethinking them so it's clearer what's going on.

          Noble, if you can't fix the SchemaTest failures right away, we should revert your e8cc19e commit until a fix is in place.

          Show
          steve_rowe Steve Rowe added a comment - +1 to the latest patch. It did take me a while to grok the default case in SchemaHandler.handleGET() though - the local variable names (realName, fieldName, name, parts) are confusing. I suggest rethinking them so it's clearer what's going on. Noble, if you can't fix the SchemaTest failures right away, we should revert your e8cc19e commit until a fix is in place.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 827c670d189f9da56259b541db9b94113d3ca9a0 in lucene-solr's branch refs/heads/master from Noble Paul
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=827c670 ]

          SOLR-8992: Restore Schema API GET method functionality removed in 6.0 added back another test and fixed test errors

          Show
          jira-bot ASF subversion and git services added a comment - Commit 827c670d189f9da56259b541db9b94113d3ca9a0 in lucene-solr's branch refs/heads/master from Noble Paul [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=827c670 ] SOLR-8992 : Restore Schema API GET method functionality removed in 6.0 added back another test and fixed test errors
          Hide
          noble.paul Noble Paul added a comment -

          Yeah, I need to clean it up

          Show
          noble.paul Noble Paul added a comment - Yeah, I need to clean it up
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 7fefc35dcfb84c74fec2ea1a8abd6dea1289bd18 in lucene-solr's branch refs/heads/branch_6x from Noble Paul
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=7fefc35 ]

          SOLR-8992: Restore Schema API GET method functionality removed in 6.0

          Show
          jira-bot ASF subversion and git services added a comment - Commit 7fefc35dcfb84c74fec2ea1a8abd6dea1289bd18 in lucene-solr's branch refs/heads/branch_6x from Noble Paul [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=7fefc35 ] SOLR-8992 : Restore Schema API GET method functionality removed in 6.0
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 6b7e965b85873ffa11d9b1c443748257c8767659 in lucene-solr's branch refs/heads/branch_6x from Noble Paul
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=6b7e965 ]

          SOLR-8992: Restore Schema API GET method functionality removed in 6.0

          Show
          jira-bot ASF subversion and git services added a comment - Commit 6b7e965b85873ffa11d9b1c443748257c8767659 in lucene-solr's branch refs/heads/branch_6x from Noble Paul [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=6b7e965 ] SOLR-8992 : Restore Schema API GET method functionality removed in 6.0
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 6c459779c58a49b685fe84fede737a40769d2d88 in lucene-solr's branch refs/heads/branch_6x from Noble Paul
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=6c45977 ]

          SOLR-8992: Restore Schema API GET method functionality removed in 6.0 added back another test and fixed test errors

          Show
          jira-bot ASF subversion and git services added a comment - Commit 6c459779c58a49b685fe84fede737a40769d2d88 in lucene-solr's branch refs/heads/branch_6x from Noble Paul [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=6c45977 ] SOLR-8992 : Restore Schema API GET method functionality removed in 6.0 added back another test and fixed test errors
          Hide
          yseeley@gmail.com Yonik Seeley added a comment -

          Has this issue been completed?

          Show
          yseeley@gmail.com Yonik Seeley added a comment - Has this issue been completed?
          Hide
          steve_rowe Steve Rowe added a comment -

          I think this should be backported to branch_6_0 for a 6.0.1 release.

          Show
          steve_rowe Steve Rowe added a comment - I think this should be backported to branch_6_0 for a 6.0.1 release.
          Hide
          noble.paul Noble Paul added a comment -

          will do that

          Show
          noble.paul Noble Paul added a comment - will do that
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit f837216b3bd06c6e914206562b3c8c4cfdf8c1bd in lucene-solr's branch refs/heads/branch_6_0 from Noble Paul
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=f837216 ]

          SOLR-8992: Restore Schema API GET method functionality removed in 6.0

          Show
          jira-bot ASF subversion and git services added a comment - Commit f837216b3bd06c6e914206562b3c8c4cfdf8c1bd in lucene-solr's branch refs/heads/branch_6_0 from Noble Paul [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=f837216 ] SOLR-8992 : Restore Schema API GET method functionality removed in 6.0
          Hide
          hossman Hoss Man added a comment -

          Manually correcting fixVersion per Step #S5 of LUCENE-7271

          Show
          hossman Hoss Man added a comment - Manually correcting fixVersion per Step #S5 of LUCENE-7271
          Hide
          steve_rowe Steve Rowe added a comment -

          In backporting SOLR-8985 to branch_6_0, I looked at its interaction with this issue (already backported by Noble Paul), and noticed that some of the tests in TestFieldCollectionResource on branch_6x and master didn't make it branch_6_0.

          Reopening to get these tests in; I'll post a patch.

          Show
          steve_rowe Steve Rowe added a comment - In backporting SOLR-8985 to branch_6_0, I looked at its interaction with this issue (already backported by Noble Paul ), and noticed that some of the tests in TestFieldCollectionResource on branch_6x and master didn't make it branch_6_0. Reopening to get these tests in; I'll post a patch.
          Hide
          steve_rowe Steve Rowe added a comment -

          Patch with missing tests.

          Committing shortly.

          Show
          steve_rowe Steve Rowe added a comment - Patch with missing tests. Committing shortly.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 2c6f0282b4a29c69c5a5182d8babc52298d0f807 in lucene-solr's branch refs/heads/branch_6_0 from Steve Rowe
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=2c6f028 ]

          SOLR-8992: branch_6_0: restore missing TestFieldCollectionResource tests

          Show
          jira-bot ASF subversion and git services added a comment - Commit 2c6f0282b4a29c69c5a5182d8babc52298d0f807 in lucene-solr's branch refs/heads/branch_6_0 from Steve Rowe [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=2c6f028 ] SOLR-8992 : branch_6_0: restore missing TestFieldCollectionResource tests
          Hide
          steve_rowe Steve Rowe added a comment -

          Committed the missing tests. I checked the diffs between branch_6_0 and master, and between branch_6_0 and branch_6x, of all other files touched by this issue and SOLR-8985, and nothing else is missing.

          Show
          steve_rowe Steve Rowe added a comment - Committed the missing tests. I checked the diffs between branch_6_0 and master, and between branch_6_0 and branch_6x, of all other files touched by this issue and SOLR-8985 , and nothing else is missing.
          Hide
          steve_rowe Steve Rowe added a comment -

          Bulk close issues included in the 6.0.1 release.

          Show
          steve_rowe Steve Rowe added a comment - Bulk close issues included in the 6.0.1 release.

            People

            • Assignee:
              noble.paul Noble Paul
              Reporter:
              steve_rowe Steve Rowe
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development