You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2021/08/16 14:48:54 UTC
[GitHub] [solr] epugh opened a new pull request #264: SOLR-10887: Append .xml to managed-schema file
epugh opened a new pull request #264:
URL: https://github.com/apache/solr/pull/264
https://issues.apache.org/jira/browse/SOLR-10887
<!--
_(If you are a project committer then you may remove some/all of the following template.)_
Before creating a pull request, please file an issue in the ASF Jira system for Solr:
* https://issues.apache.org/jira/projects/SOLR
You will need to create an account in Jira in order to create an issue.
The title of the PR should reference the Jira issue number in the form:
* SOLR-####: <short description of problem or changes>
SOLR must be fully capitalized. A short description helps people scanning pull requests for items they can work on.
Properly referencing the issue in the title ensures that Jira is correctly updated with code review comments and commits. -->
# Description
Migrate any `managed-schema` file found in ZK to `managed-schema.xml`, and make the default be `managed-schema.xml`.
# Solution
I basically checked where we load the managed schema, and if the name matches that of the legacy schema, `managed-schema`, then rename it.
I am NOT very confident that this was the right way to do this, and would love feedback/suggestions.
For example maybe I should have tagged onto the logic in ManageSchemaIndexFactory where it decides `shouldUpgrade` and then maybe used the method `zkUgradeToManagedSchema` to handle the `managed-schema` to `managed-schema.xml` change?
# Tests
Please describe the tests you've developed or run to confirm this patch implements the feature or solves the problem.
# Checklist
Please review the following and check all that apply:
- [ ] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
- [ ] I have created a Jira issue and added the issue ID to my pull request title.
- [ ] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [ ] I have developed this patch against the `main` branch.
- [ ] I have run `./gradlew check`.
- [ ] I have added tests for my changes.
- [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r698422058
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +95,63 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file if the legacy file exists.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ */
+ public String lookupZKManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
+ final ZkController zkController = zkLoader.getZkController();
+ final SolrZkClient zkClient = zkController.getZkClient();
+ String managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + managedSchemaResourceName;
+ final String legacyManagedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME;
+ try {
+ // check if we are using the legacy managed-schema file name.
+ if (zkClient.exists(legacyManagedSchemaPath, true)){
+ log.debug("Legacy managed schema resource {} found - loading legacy managed schema instead of {} file."
+ , ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME, managedSchemaResourceName);
+ managedSchemaPath = legacyManagedSchemaPath;
+ }
+ } catch (KeeperException e) {
+ throw new RuntimeException(e);
+ } catch (InterruptedException e) {
+ // Restore the interrupted status
+ Thread.currentThread().interrupt();
+ throw new RuntimeException(e);
+ }
+ return managedSchemaPath;
+ }
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file if the legacy file exists.
+ */
+ public Path lookupLocalManagedSchemaPath() {
+ final Path legacyManagedSchemaPath = Paths.get(loader.getConfigPath().toString(), ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME);
+
+ Path managedSchemaPath = Paths.get(loader.getConfigPath().toString(), managedSchemaResourceName);
+
+ // check if we are using the legacy managed-schema file name.
+ if (Files.exists(legacyManagedSchemaPath)){
+ log.debug("Legacy managed schema resource {} found - loading legacy managed schema instead of {} file."
+ , ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME, managedSchemaResourceName);
+ managedSchemaPath = legacyManagedSchemaPath;
+ }
+
+ File parentDir = managedSchemaPath.toFile().getParentFile(); // Do we need this clause? A managed file that is in a dir that doesn't exist?
Review comment:
Redone!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r694333269
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -74,12 +75,12 @@ public void init(NamedList<?> args) {
args.remove("mutable");
managedSchemaResourceName = params.get(MANAGED_SCHEMA_RESOURCE_NAME, DEFAULT_MANAGED_SCHEMA_RESOURCE_NAME);
args.remove(MANAGED_SCHEMA_RESOURCE_NAME);
- if (SCHEMA_DOT_XML.equals(managedSchemaResourceName)) {
+ if (SCHEMA_DOT_XML.equals(managedSchemaResourceName)) { // TODO Still needed?
Review comment:
I agree; I think a user should be able to configure it to be the same if they wish. But I would make such a change in a separate issue, and also see that there is no problem with it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r693484821
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +92,33 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ * @see org.apache.solr.schema.ManagedIndexSchema#lookupManagedSchemaPath
+ */
+ public String lookupManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
+ final ZkController zkController = zkLoader.getZkController();
+ final SolrZkClient zkClient = zkController.getZkClient();
+ String managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + managedSchemaResourceName;
+ try {
+ // check if we are using the legacy managed-schema file name.
+ if (!zkClient.exists(managedSchemaPath, true)){
+ log.info("Managed schema resource {} not found - loading legacy managed schema {} file instead"
+ , managedSchemaResourceName, ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME);
+ managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME;
+ }
+ } catch (KeeperException e) {
+ throw new RuntimeException(e);
+ } catch (InterruptedException e) {
+ throw new RuntimeException(e);
Review comment:
Done.... I don't know how you knew to set that however! Wonder if there is a check that could have flagged this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r694332486
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -195,7 +223,16 @@ private InputStream readSchemaLocally() {
InputStream schemaInputStream = null;
try {
// Attempt to load the managed schema
- schemaInputStream = loader.openResource(managedSchemaResourceName);
+ try {
+ schemaInputStream = loader.openResource(managedSchemaResourceName);
+ }
+ catch (IOException e) {
Review comment:
Uh.... SolrResourceNotFoundException extends IOException so it should be possible -- just catch it before IOException. If you give up, I can submit to your PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
epugh commented on pull request #264:
URL: https://github.com/apache/solr/pull/264#issuecomment-901966202
Doing some manual testing, and in non-solr cloud mode, if you have a `managed-schema` file, and you modify the schema via SchemaAPI, then you now have a `managed-schema.xml` along side the original `managed-schema`. Is this okay? Or do we need to honour the fact that they started with `managed-schema` and update that file?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r698411663
##########
File path: solr/core/src/java/org/apache/solr/schema/SchemaManager.java
##########
@@ -428,17 +428,22 @@ private ManagedIndexSchema getFreshManagedSchema(SolrCore core) throws IOExcepti
SolrResourceLoader resourceLoader = core.getResourceLoader();
String name = core.getLatestSchema().getResourceName();
Review comment:
Done!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r694640465
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +95,66 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ */
+ public String lookupZKManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
+ final ZkController zkController = zkLoader.getZkController();
+ final SolrZkClient zkClient = zkController.getZkClient();
+ String managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + managedSchemaResourceName;
+ final String legacyManagedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME;
+ try {
+ // check if we are using the legacy managed-schema file name.
+ if (zkClient.exists(legacyManagedSchemaPath, true)){
+ log.debug("Legacy managed schema resource {} found - loading legacy managed schema instead of {} file."
+ , ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME, managedSchemaResourceName);
+ managedSchemaPath = legacyManagedSchemaPath;
+ }
+ } catch (KeeperException e) {
+ throw new RuntimeException(e);
+ } catch (InterruptedException e) {
+ // Restore the interrupted status
+ Thread.currentThread().interrupt();
+ throw new RuntimeException(e);
+ }
+ return managedSchemaPath;
+ }
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchemaFactory.
+ * @see org.apache.solr.schema.ManagedIndexSchemaFactory#lookupZKManagedSchemaPath
+ */
+ public Path lookupLocalManagedSchemaPath() {
+ final Path legacyManagedSchemaPath = Paths.get(loader.getConfigPath().toString(), ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME);
+
+ Path managedSchemaPath = Paths.get(loader.getConfigPath().toString(), managedSchemaResourceName);
Review comment:
Can you guard this with a call to `CoreContainer#assertPathAllowed(path)`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r694768304
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +95,66 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ */
+ public String lookupZKManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
+ final ZkController zkController = zkLoader.getZkController();
+ final SolrZkClient zkClient = zkController.getZkClient();
+ String managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + managedSchemaResourceName;
+ final String legacyManagedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME;
+ try {
+ // check if we are using the legacy managed-schema file name.
+ if (zkClient.exists(legacyManagedSchemaPath, true)){
+ log.debug("Legacy managed schema resource {} found - loading legacy managed schema instead of {} file."
+ , ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME, managedSchemaResourceName);
+ managedSchemaPath = legacyManagedSchemaPath;
+ }
+ } catch (KeeperException e) {
+ throw new RuntimeException(e);
+ } catch (InterruptedException e) {
+ // Restore the interrupted status
+ Thread.currentThread().interrupt();
+ throw new RuntimeException(e);
+ }
+ return managedSchemaPath;
+ }
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchemaFactory.
+ * @see org.apache.solr.schema.ManagedIndexSchemaFactory#lookupZKManagedSchemaPath
+ */
+ public Path lookupLocalManagedSchemaPath() {
+ final Path legacyManagedSchemaPath = Paths.get(loader.getConfigPath().toString(), ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME);
+
+ Path managedSchemaPath = Paths.get(loader.getConfigPath().toString(), managedSchemaResourceName);
Review comment:
i checked the javadocs for `assertPathAllowed`, and it only confirms if the file is in the SOLR_HOME... Are y ou thinking this is to help with the `PATH_TRAVERSAL_IN` issue?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh closed pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
epugh closed pull request #264:
URL: https://github.com/apache/solr/pull/264
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
epugh commented on pull request #264:
URL: https://github.com/apache/solr/pull/264#issuecomment-918220552
Closing in favour of an updated PR https://github.com/apache/solr/pull/279
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r694780800
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +95,66 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ */
+ public String lookupZKManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
+ final ZkController zkController = zkLoader.getZkController();
+ final SolrZkClient zkClient = zkController.getZkClient();
+ String managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + managedSchemaResourceName;
+ final String legacyManagedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME;
+ try {
+ // check if we are using the legacy managed-schema file name.
+ if (zkClient.exists(legacyManagedSchemaPath, true)){
+ log.debug("Legacy managed schema resource {} found - loading legacy managed schema instead of {} file."
+ , ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME, managedSchemaResourceName);
+ managedSchemaPath = legacyManagedSchemaPath;
+ }
+ } catch (KeeperException e) {
+ throw new RuntimeException(e);
+ } catch (InterruptedException e) {
+ // Restore the interrupted status
+ Thread.currentThread().interrupt();
+ throw new RuntimeException(e);
+ }
+ return managedSchemaPath;
+ }
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchemaFactory.
+ * @see org.apache.solr.schema.ManagedIndexSchemaFactory#lookupZKManagedSchemaPath
+ */
+ public Path lookupLocalManagedSchemaPath() {
+ final Path legacyManagedSchemaPath = Paths.get(loader.getConfigPath().toString(), ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME);
+
+ Path managedSchemaPath = Paths.get(loader.getConfigPath().toString(), managedSchemaResourceName);
Review comment:
That was my thinking, but as the only way to insert `managedSchemaResourceName` is through solrconfig (or perhaps config api), then perhaps this assertion is not needed, instead try to mute sonatype-lift :) ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r693444308
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +92,33 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ * @see org.apache.solr.schema.ManagedIndexSchema#lookupManagedSchemaPath
+ */
+ public String lookupManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
Review comment:
ignore
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
epugh commented on pull request #264:
URL: https://github.com/apache/solr/pull/264#issuecomment-903250748
@janhoy I wonder if there will ever be enough use cases that we would want to support some sort of "migrations" concept in Solr... A way of assisting someone in an upgrade by running bits of code to redo things, like this specific use case ;-).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r694320618
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +92,33 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ * @see org.apache.solr.schema.ManagedIndexSchema#lookupManagedSchemaPath
+ */
+ public String lookupManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
Review comment:
@dsmiley do y ou think this method lookupZKManagedSchemaPath should actually return a `Path` class? Wish there was a single way of representing both in ZK and on the local filesystem....
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] sonatype-lift[bot] commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r694349095
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +92,66 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ */
+ public String lookupZKManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
+ final ZkController zkController = zkLoader.getZkController();
+ final SolrZkClient zkClient = zkController.getZkClient();
+ String managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + managedSchemaResourceName;
+ final String legacyManagedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME;
+ try {
+ // check if we are using the legacy managed-schema file name.
+ if (zkClient.exists(legacyManagedSchemaPath, true)){
+ log.debug("Legacy managed schema resource {} found - loading legacy managed schema instead of {} file."
+ , ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME, managedSchemaResourceName);
+ managedSchemaPath = legacyManagedSchemaPath;
+ }
+ } catch (KeeperException e) {
+ throw new RuntimeException(e);
+ } catch (InterruptedException e) {
+ // Restore the interrupted status
+ Thread.currentThread().interrupt();
+ throw new RuntimeException(e);
+ }
+ return managedSchemaPath;
+ }
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchemaFactory.
+ * @see org.apache.solr.schema.ManagedIndexSchemaFactory#lookupZKManagedSchemaPath
+ */
+ public File lookupLocalManagedSchemaPath() {
+ final File legacyManagedSchemaPath = new File(loader.getConfigPath().toFile(), ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME);
+
+ File managedSchemaFile = new File(loader.getConfigPath().toFile(), managedSchemaResourceName);
Review comment:
*PATH_TRAVERSAL_IN:* This API (java/io/File.<init>(Ljava/io/File;Ljava/lang/String;)V) reads a file whose location might be specified by user input [(details)](https://find-sec-bugs.github.io/bugs.htm#PATH_TRAVERSAL_IN)
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +92,66 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `ManagedIndexSchemaFactory.getSchemaResourceName(...)` reads without synchronization from `this.managedSchemaResourceName`. Potentially races with write in method `ManagedIndexSchemaFactory.create(...)`.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +92,66 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ */
+ public String lookupZKManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
+ final ZkController zkController = zkLoader.getZkController();
+ final SolrZkClient zkClient = zkController.getZkClient();
+ String managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + managedSchemaResourceName;
+ final String legacyManagedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME;
+ try {
+ // check if we are using the legacy managed-schema file name.
+ if (zkClient.exists(legacyManagedSchemaPath, true)){
+ log.debug("Legacy managed schema resource {} found - loading legacy managed schema instead of {} file."
+ , ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME, managedSchemaResourceName);
+ managedSchemaPath = legacyManagedSchemaPath;
+ }
+ } catch (KeeperException e) {
+ throw new RuntimeException(e);
+ } catch (InterruptedException e) {
+ // Restore the interrupted status
+ Thread.currentThread().interrupt();
+ throw new RuntimeException(e);
+ }
+ return managedSchemaPath;
+ }
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchemaFactory.
+ * @see org.apache.solr.schema.ManagedIndexSchemaFactory#lookupZKManagedSchemaPath
+ */
+ public File lookupLocalManagedSchemaPath() {
+ final File legacyManagedSchemaPath = new File(loader.getConfigPath().toFile(), ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME);
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `ManagedIndexSchemaFactory.lookupLocalManagedSchemaPath()` reads without synchronization from `this.loader`. Potentially races with write in method `ManagedIndexSchemaFactory.create(...)`.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +92,66 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ */
+ public String lookupZKManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
+ final ZkController zkController = zkLoader.getZkController();
+ final SolrZkClient zkClient = zkController.getZkClient();
+ String managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + managedSchemaResourceName;
+ final String legacyManagedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME;
+ try {
+ // check if we are using the legacy managed-schema file name.
+ if (zkClient.exists(legacyManagedSchemaPath, true)){
+ log.debug("Legacy managed schema resource {} found - loading legacy managed schema instead of {} file."
+ , ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME, managedSchemaResourceName);
+ managedSchemaPath = legacyManagedSchemaPath;
+ }
+ } catch (KeeperException e) {
+ throw new RuntimeException(e);
+ } catch (InterruptedException e) {
+ // Restore the interrupted status
+ Thread.currentThread().interrupt();
+ throw new RuntimeException(e);
+ }
+ return managedSchemaPath;
+ }
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchemaFactory.
+ * @see org.apache.solr.schema.ManagedIndexSchemaFactory#lookupZKManagedSchemaPath
+ */
+ public File lookupLocalManagedSchemaPath() {
+ final File legacyManagedSchemaPath = new File(loader.getConfigPath().toFile(), ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME);
+
+ File managedSchemaFile = new File(loader.getConfigPath().toFile(), managedSchemaResourceName);
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `ManagedIndexSchemaFactory.lookupLocalManagedSchemaPath()` reads without synchronization from `this.managedSchemaResourceName`. Potentially races with write in method `ManagedIndexSchemaFactory.create(...)`.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +92,66 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ */
+ public String lookupZKManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `ManagedIndexSchemaFactory.lookupZKManagedSchemaPath()` reads without synchronization from `this.loader`. Potentially races with write in method `ManagedIndexSchemaFactory.create(...)`.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +92,66 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ */
+ public String lookupZKManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
+ final ZkController zkController = zkLoader.getZkController();
+ final SolrZkClient zkClient = zkController.getZkClient();
+ String managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + managedSchemaResourceName;
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `ManagedIndexSchemaFactory.lookupZKManagedSchemaPath()` reads without synchronization from `this.managedSchemaResourceName`. Potentially races with write in method `ManagedIndexSchemaFactory.create(...)`.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
dsmiley commented on pull request #264:
URL: https://github.com/apache/solr/pull/264#issuecomment-899824276
Thanks for working on this!
I don't think I like the automagic rename. What if that configSet was designated as immutable; this would violate that metadata? And it works with only ZK, and doesn't support the file system or a custom ConfigSetService either (a recent option).
Can't we just check for both -- check the new name first and if it doesn't exist (will become increasingly uncommon) then try the old name? Needn't even warn about it, I think.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r694316602
##########
File path: solr/core/src/test/org/apache/solr/handler/admin/ShowFileRequestHandlerTest.java
##########
@@ -84,7 +84,7 @@ public void testDirList() throws SolrServerException, IOException {
public void testGetRawFile() throws SolrServerException, IOException {
SolrClient client = getSolrClient();
//assertQ(req("qt", "/admin/file")); TODO file bug that SolrJettyTestBase extends SolrTestCaseJ4
- QueryRequest request = new QueryRequest(params("file", "managed-schema.xml"));
+ QueryRequest request = new QueryRequest(params("file", "managed-schema"));
Review comment:
Yeah, for now, till I write a better integration style test. I discovered that while the unit tests might all pass perfectly, when I fired up solr in cloud mode and non cloud mode it wasn't working ;-( So I went back to the old way to make srue I was properly testing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r693439374
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -195,7 +223,16 @@ private InputStream readSchemaLocally() {
InputStream schemaInputStream = null;
try {
// Attempt to load the managed schema
- schemaInputStream = loader.openResource(managedSchemaResourceName);
+ try {
+ schemaInputStream = loader.openResource(managedSchemaResourceName);
+ }
+ catch (IOException e) {
Review comment:
Not for any IOException, only when the resource isn't found. Looking at the code, we'll through `SolrResourceNotFoundException`
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +92,33 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ * @see org.apache.solr.schema.ManagedIndexSchema#lookupManagedSchemaPath
+ */
+ public String lookupManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
+ final ZkController zkController = zkLoader.getZkController();
+ final SolrZkClient zkClient = zkController.getZkClient();
+ String managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + managedSchemaResourceName;
+ try {
+ // check if we are using the legacy managed-schema file name.
+ if (!zkClient.exists(managedSchemaPath, true)){
+ log.info("Managed schema resource {} not found - loading legacy managed schema {} file instead"
Review comment:
IMO should be debug level
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -74,12 +75,12 @@ public void init(NamedList<?> args) {
args.remove("mutable");
managedSchemaResourceName = params.get(MANAGED_SCHEMA_RESOURCE_NAME, DEFAULT_MANAGED_SCHEMA_RESOURCE_NAME);
args.remove(MANAGED_SCHEMA_RESOURCE_NAME);
- if (SCHEMA_DOT_XML.equals(managedSchemaResourceName)) {
+ if (SCHEMA_DOT_XML.equals(managedSchemaResourceName)) { // TODO Still needed?
Review comment:
I don't think this check is out-dated, if that's why you added this comment.
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -195,7 +223,16 @@ private InputStream readSchemaLocally() {
InputStream schemaInputStream = null;
try {
// Attempt to load the managed schema
- schemaInputStream = loader.openResource(managedSchemaResourceName);
+ try {
+ schemaInputStream = loader.openResource(managedSchemaResourceName);
+ }
+ catch (IOException e) {
+ // Attempt to load the managed schema with the legacy name.
+ log.info("The schema is configured as managed, but managed schema resource {} not found - looking for legacy {} instead"
Review comment:
IMO this should be debug level
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +92,33 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ * @see org.apache.solr.schema.ManagedIndexSchema#lookupManagedSchemaPath
+ */
+ public String lookupManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
+ final ZkController zkController = zkLoader.getZkController();
+ final SolrZkClient zkClient = zkController.getZkClient();
+ String managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + managedSchemaResourceName;
+ try {
+ // check if we are using the legacy managed-schema file name.
+ if (!zkClient.exists(managedSchemaPath, true)){
+ log.info("Managed schema resource {} not found - loading legacy managed schema {} file instead"
+ , managedSchemaResourceName, ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME);
+ managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME;
+ }
+ } catch (KeeperException e) {
+ throw new RuntimeException(e);
+ } catch (InterruptedException e) {
+ throw new RuntimeException(e);
Review comment:
Set interruption status on the current thread
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchema.java
##########
@@ -111,13 +111,41 @@
this.schemaUpdateLock = schemaUpdateLock;
}
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchemaFactory.
+ * @see org.apache.solr.schema.ManagedIndexSchemaFactory#lookupManagedSchemaPath
+ */
+ public String lookupManagedSchemaPath() {
Review comment:
I don't think this method belongs here; it's already on the factory which should be sufficient. It seems you added it so that MIS.persist... is called, that it knows where to persist it. But I think the schema already knows it's own name/path: field `managedSchemaResourceName`. If the problem is that this is never simply `managed-schema` then it seems you need to fix that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
epugh commented on pull request #264:
URL: https://github.com/apache/solr/pull/264#issuecomment-901354917
I backed out the magic rename, but to get the tests to pass, I have to check in THREE places the naming logic... ;-(.
I'm going to try and DRY it up, and would love any pointers you might have!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r694311373
##########
File path: solr/core/src/test/org/apache/solr/handler/admin/ShowFileRequestHandlerTest.java
##########
@@ -84,7 +84,7 @@ public void testDirList() throws SolrServerException, IOException {
public void testGetRawFile() throws SolrServerException, IOException {
SolrClient client = getSolrClient();
//assertQ(req("qt", "/admin/file")); TODO file bug that SolrJettyTestBase extends SolrTestCaseJ4
- QueryRequest request = new QueryRequest(params("file", "managed-schema.xml"));
+ QueryRequest request = new QueryRequest(params("file", "managed-schema"));
Review comment:
Changed your mind?
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -223,15 +257,9 @@ private InputStream readSchemaLocally() {
InputStream schemaInputStream = null;
try {
// Attempt to load the managed schema
- try {
- schemaInputStream = loader.openResource(managedSchemaResourceName);
- }
- catch (IOException e) {
- // Attempt to load the managed schema with the legacy name.
- log.info("The schema is configured as managed, but managed schema resource {} not found - looking for legacy {} instead"
- , managedSchemaResourceName, LEGACY_MANAGED_SCHEMA_RESOURCE_NAME);
- schemaInputStream = loader.openResource(LEGACY_MANAGED_SCHEMA_RESOURCE_NAME);
- }
+ final String managedSchemaPath = lookupLocalManagedSchemaPath().toString();
+ schemaInputStream = loader.openResource(lookupLocalManagedSchemaPath().toString());
+ managedSchemaResourceName = managedSchemaPath.substring(managedSchemaPath.lastIndexOf("/")+1); // not loving this
Review comment:
> // not loving this
Me neither! Firstly, please avoid File in Java; use Path henceforth. `lookupLocalManagedSchemaPath` ought to return a *Path* not *File*. Path gives you methods to get the last piece via `getName(getNameCount()-1)`
Also, I see no reason to call `lookupLocalManagedSchemaPath` twice as you have done.
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +92,33 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ * @see org.apache.solr.schema.ManagedIndexSchema#lookupManagedSchemaPath
+ */
+ public String lookupManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
Review comment:
@sonatype-lift ignore
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +92,33 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ * @see org.apache.solr.schema.ManagedIndexSchema#lookupManagedSchemaPath
+ */
+ public String lookupManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
+ final ZkController zkController = zkLoader.getZkController();
+ final SolrZkClient zkClient = zkController.getZkClient();
+ String managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + managedSchemaResourceName;
+ try {
+ // check if we are using the legacy managed-schema file name.
+ if (!zkClient.exists(managedSchemaPath, true)){
+ log.info("Managed schema resource {} not found - loading legacy managed schema {} file instead"
+ , managedSchemaResourceName, ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME);
+ managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME;
+ }
+ } catch (KeeperException e) {
+ throw new RuntimeException(e);
+ } catch (InterruptedException e) {
+ throw new RuntimeException(e);
Review comment:
> I don't know how you knew to set that however!
This is a standard Java practice that is easy to miss. I wish Sonatype Lift / would raise alarms about this in case I'm not a code reviewer :-). There is one bug pattern used by error-prone (Sonatype Lift calls it) -- https://errorprone.info/bugpattern/InterruptedExceptionSwallowed but you didn't _quite_ run a-fowl of that pattern since you did explicitly catch InterruptedException. Error-prone may be assuming that because you explicitly caught it, that you know what you are doing ;-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r693484231
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -195,7 +223,16 @@ private InputStream readSchemaLocally() {
InputStream schemaInputStream = null;
try {
// Attempt to load the managed schema
- schemaInputStream = loader.openResource(managedSchemaResourceName);
+ try {
+ schemaInputStream = loader.openResource(managedSchemaResourceName);
+ }
+ catch (IOException e) {
Review comment:
Lookis I am stuck with IOException, it won't let me specify SolrResourceNotFoundException.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r693485131
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -74,12 +75,12 @@ public void init(NamedList<?> args) {
args.remove("mutable");
managedSchemaResourceName = params.get(MANAGED_SCHEMA_RESOURCE_NAME, DEFAULT_MANAGED_SCHEMA_RESOURCE_NAME);
args.remove(MANAGED_SCHEMA_RESOURCE_NAME);
- if (SCHEMA_DOT_XML.equals(managedSchemaResourceName)) {
+ if (SCHEMA_DOT_XML.equals(managedSchemaResourceName)) { // TODO Still needed?
Review comment:
Reading through this again... I see that we have decided that schema.xml can't be the managed schema file... Which I guess is fine, but if you wanted it to be? Is tehre a use case for being able to override the default managed schema name? I was wondering if it could just be hardcoded to "managed-schema.xml" (wiht the legacy fall back to "managed-schema". Is there ever a reason to have a different name?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r694305186
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +92,33 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ * @see org.apache.solr.schema.ManagedIndexSchema#lookupManagedSchemaPath
+ */
+ public String lookupManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
+ final ZkController zkController = zkLoader.getZkController();
+ final SolrZkClient zkClient = zkController.getZkClient();
+ String managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + managedSchemaResourceName;
+ try {
+ // check if we are using the legacy managed-schema file name.
+ if (!zkClient.exists(managedSchemaPath, true)){
+ log.info("Managed schema resource {} not found - loading legacy managed schema {} file instead"
+ , managedSchemaResourceName, ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME);
+ managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME;
+ }
+ } catch (KeeperException e) {
+ throw new RuntimeException(e);
+ } catch (InterruptedException e) {
+ throw new RuntimeException(e);
Review comment:
> I don't know how you knew to set that however!
This is a standard Java practice that is easy to miss. I wish Sonatype Lift / would raise alarms about this in case I'm not a code reviewer :-). There is one bug pattern used by error-prone (Sonatype Lift calls it) -- https://errorprone.info/bugpattern/InterruptedExceptionSwallowed but you didn't _quite_ run a-fowl of that pattern since you did explicitly catch InterruptedException. Error-prone may be assuming that because you explicitly caught it, that you know what you are doing ;-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r694936746
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +95,66 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ */
+ public String lookupZKManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
+ final ZkController zkController = zkLoader.getZkController();
+ final SolrZkClient zkClient = zkController.getZkClient();
+ String managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + managedSchemaResourceName;
+ final String legacyManagedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME;
+ try {
+ // check if we are using the legacy managed-schema file name.
+ if (zkClient.exists(legacyManagedSchemaPath, true)){
+ log.debug("Legacy managed schema resource {} found - loading legacy managed schema instead of {} file."
+ , ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME, managedSchemaResourceName);
+ managedSchemaPath = legacyManagedSchemaPath;
+ }
+ } catch (KeeperException e) {
+ throw new RuntimeException(e);
+ } catch (InterruptedException e) {
+ // Restore the interrupted status
+ Thread.currentThread().interrupt();
+ throw new RuntimeException(e);
+ }
+ return managedSchemaPath;
+ }
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchemaFactory.
+ * @see org.apache.solr.schema.ManagedIndexSchemaFactory#lookupZKManagedSchemaPath
+ */
+ public Path lookupLocalManagedSchemaPath() {
+ final Path legacyManagedSchemaPath = Paths.get(loader.getConfigPath().toString(), ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME);
+
+ Path managedSchemaPath = Paths.get(loader.getConfigPath().toString(), managedSchemaResourceName);
Review comment:
@sonatype-lift ignore
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
janhoy commented on pull request #264:
URL: https://github.com/apache/solr/pull/264#issuecomment-901988377
> ...Or do we need to honour the fact that they started with `managed-schema` and update that file?
Of course it was not as easy as just caring for the read side :) Looks like each core will need to decide which of the two names to use as default - depending on what it finds in the configset? The alternative is "rename-on-write" which would be just as surprising as "rename-on-boot" I guess?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
epugh commented on pull request #264:
URL: https://github.com/apache/solr/pull/264#issuecomment-910236316
Thank you everyone who reviewed code. I have spent the past few days trying to figure out why tests are failing (that shouldn't have!!), and started a NEW branch, where all of the code worked;-(. I have been trying to compare the two versions over and over, and didn't have luck figureing out the difference. I am now porting changes over to https://github.com/apache/solr/pull/279 and then running `./gradlew check`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r693484355
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -195,7 +223,16 @@ private InputStream readSchemaLocally() {
InputStream schemaInputStream = null;
try {
// Attempt to load the managed schema
- schemaInputStream = loader.openResource(managedSchemaResourceName);
+ try {
+ schemaInputStream = loader.openResource(managedSchemaResourceName);
+ }
+ catch (IOException e) {
+ // Attempt to load the managed schema with the legacy name.
+ log.info("The schema is configured as managed, but managed schema resource {} not found - looking for legacy {} instead"
Review comment:
Will do.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r694349334
##########
File path: solr/core/src/java/org/apache/solr/schema/SchemaManager.java
##########
@@ -428,17 +428,22 @@ private ManagedIndexSchema getFreshManagedSchema(SolrCore core) throws IOExcepti
SolrResourceLoader resourceLoader = core.getResourceLoader();
String name = core.getLatestSchema().getResourceName();
Review comment:
I don't love the name `name`, maybe `schemaResourceName`? kind of verbose.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r698409242
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -195,7 +223,16 @@ private InputStream readSchemaLocally() {
InputStream schemaInputStream = null;
try {
// Attempt to load the managed schema
- schemaInputStream = loader.openResource(managedSchemaResourceName);
+ try {
+ schemaInputStream = loader.openResource(managedSchemaResourceName);
+ }
+ catch (IOException e) {
Review comment:
Okay, just pushed up an edit, is this what you were thinking?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
epugh commented on pull request #264:
URL: https://github.com/apache/solr/pull/264#issuecomment-903103237
Can I just say that all of this logic is very convoluted? We're constantly checking what type of loader we have, we constantly manually create the paths to lookup the schema resource file, and there seem to be a ton of "fallback" checks that I wonder if they are actually needed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
epugh commented on pull request #264:
URL: https://github.com/apache/solr/pull/264#issuecomment-901966934
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r693485382
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchema.java
##########
@@ -111,13 +111,41 @@
this.schemaUpdateLock = schemaUpdateLock;
}
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchemaFactory.
+ * @see org.apache.solr.schema.ManagedIndexSchemaFactory#lookupManagedSchemaPath
+ */
+ public String lookupManagedSchemaPath() {
Review comment:
Yeah, I definitly saw this as a code smell, and yeah, I couldn't quite figure out how to make sure `managedSchemaResourceName` was ever changed from the default to `managed-schema.xml` to the `managed-schema` at the right time. I've also experimented with moving this whole method over to ZkSolrResourceLoader, with the idea that it is the right class to do the fall back lookup....
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r697653753
##########
File path: solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
##########
@@ -342,7 +342,7 @@ private Path checkPathIsSafe(Path pathToCheck) throws IOException {
*/
@Override
public InputStream openResource(String resource) throws IOException {
-
Review comment:
revert since you didn't need to touch this file
##########
File path: solr/core/src/test/org/apache/solr/schema/TestFallbackToLegacyManagedSchema.java
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.schema;
+
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.util.Map;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.solr.common.util.Utils;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.SolrResourceNotFoundException;
+import org.apache.solr.util.RestTestBase;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.google.common.collect.Lists;
+
+/**
+ * Test updating a Managed Schema that is the legacy "managed-schema" instead of the
+ * default "managed-schema.xml" file properly falls back to "managed-schema"
+ */
+
+public class TestFallbackToLegacyManagedSchema extends RestTestBase {
Review comment:
Why RestTestBase? Oh, because, to easily post schema changes. Maybe SolrJ does a fine job with that already? (not sure). BTW SolrJ clients can be easily had via `SolrClient client = new EmbeddedSolrServer(h.getCoreContainer(), h.coreName);` in Solr tests.
Eventually I'm hoping more of our tests might be SolrJ based in general.
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchema.java
##########
@@ -89,13 +89,13 @@
@Override public boolean isMutable() { return isMutable; }
final String managedSchemaResourceName;
-
Review comment:
It appears all changes in this file, ManagedIndexSchema, are merely formatting so please revert.
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +95,63 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file if the legacy file exists.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ */
+ public String lookupZKManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
+ final ZkController zkController = zkLoader.getZkController();
+ final SolrZkClient zkClient = zkController.getZkClient();
+ String managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + managedSchemaResourceName;
+ final String legacyManagedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME;
+ try {
+ // check if we are using the legacy managed-schema file name.
+ if (zkClient.exists(legacyManagedSchemaPath, true)){
+ log.debug("Legacy managed schema resource {} found - loading legacy managed schema instead of {} file."
+ , ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME, managedSchemaResourceName);
+ managedSchemaPath = legacyManagedSchemaPath;
+ }
+ } catch (KeeperException e) {
+ throw new RuntimeException(e);
+ } catch (InterruptedException e) {
+ // Restore the interrupted status
+ Thread.currentThread().interrupt();
+ throw new RuntimeException(e);
+ }
+ return managedSchemaPath;
+ }
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file if the legacy file exists.
+ */
+ public Path lookupLocalManagedSchemaPath() {
+ final Path legacyManagedSchemaPath = Paths.get(loader.getConfigPath().toString(), ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME);
+
+ Path managedSchemaPath = Paths.get(loader.getConfigPath().toString(), managedSchemaResourceName);
+
+ // check if we are using the legacy managed-schema file name.
+ if (Files.exists(legacyManagedSchemaPath)){
+ log.debug("Legacy managed schema resource {} found - loading legacy managed schema instead of {} file."
+ , ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME, managedSchemaResourceName);
+ managedSchemaPath = legacyManagedSchemaPath;
+ }
+
+ File parentDir = managedSchemaPath.toFile().getParentFile(); // Do we need this clause? A managed file that is in a dir that doesn't exist?
Review comment:
Can you please not convert to File; isn't necessary.
##########
File path: solr/core/src/java/org/apache/solr/schema/SchemaManager.java
##########
@@ -428,17 +428,22 @@ private ManagedIndexSchema getFreshManagedSchema(SolrCore core) throws IOExcepti
SolrResourceLoader resourceLoader = core.getResourceLoader();
String name = core.getLatestSchema().getResourceName();
Review comment:
I also prefer your suggestion; not too verbose I think.
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +95,63 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file if the legacy file exists.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ */
+ public String lookupZKManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
+ final ZkController zkController = zkLoader.getZkController();
+ final SolrZkClient zkClient = zkController.getZkClient();
+ String managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + managedSchemaResourceName;
+ final String legacyManagedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME;
+ try {
+ // check if we are using the legacy managed-schema file name.
+ if (zkClient.exists(legacyManagedSchemaPath, true)){
+ log.debug("Legacy managed schema resource {} found - loading legacy managed schema instead of {} file."
+ , ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME, managedSchemaResourceName);
+ managedSchemaPath = legacyManagedSchemaPath;
+ }
+ } catch (KeeperException e) {
+ throw new RuntimeException(e);
+ } catch (InterruptedException e) {
+ // Restore the interrupted status
+ Thread.currentThread().interrupt();
+ throw new RuntimeException(e);
+ }
+ return managedSchemaPath;
+ }
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file if the legacy file exists.
+ */
+ public Path lookupLocalManagedSchemaPath() {
+ final Path legacyManagedSchemaPath = Paths.get(loader.getConfigPath().toString(), ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME);
+
+ Path managedSchemaPath = Paths.get(loader.getConfigPath().toString(), managedSchemaResourceName);
+
+ // check if we are using the legacy managed-schema file name.
+ if (Files.exists(legacyManagedSchemaPath)){
+ log.debug("Legacy managed schema resource {} found - loading legacy managed schema instead of {} file."
+ , ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME, managedSchemaResourceName);
+ managedSchemaPath = legacyManagedSchemaPath;
+ }
+
+ File parentDir = managedSchemaPath.toFile().getParentFile(); // Do we need this clause? A managed file that is in a dir that doesn't exist?
Review comment:
Oh and I agree with your comment... but can't we simply check that the Path exists as a simplification (no need to look at parent dir in particular)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
epugh commented on pull request #264:
URL: https://github.com/apache/solr/pull/264#issuecomment-903249838
THanks for the comments @dsmiley I will go through and make edits. I *think* when I am done with this change, I'd like to revisit the ManagedSchema codebase and see if there was some way to refactor it to be cleaner. It just feels very convoluted, and it seems conceptually like it shouldn't be so complex! SOmething for another JIRA, I am trying hard NOT to start pulling on the refactoring thread in this JIRA.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
janhoy commented on pull request #264:
URL: https://github.com/apache/solr/pull/264#issuecomment-900204661
Think I agree with @dsmiley here. Change the default for new versions, but look support fallback to old going forward. No surprises. Users can then easily do a manual rename of the file in zk or on local disk if they choose to.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] sonatype-lift[bot] commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r692205926
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +92,33 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ * @see org.apache.solr.schema.ManagedIndexSchema#lookupManagedSchemaPath
+ */
+ public String lookupManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `ManagedIndexSchemaFactory.lookupManagedSchemaPath()` reads without synchronization from `this.loader`. Potentially races with write in method `ManagedIndexSchemaFactory.create(...)`.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
dsmiley commented on pull request #264:
URL: https://github.com/apache/solr/pull/264#issuecomment-904158990
> do y ou think this method lookupZKManagedSchemaPath should actually return a Path class? Wish there was a single way of representing both in ZK and on the local filesystem....
No; should be a String.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] sonatype-lift[bot] commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r694355510
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +95,66 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ */
+ public String lookupZKManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
+ final ZkController zkController = zkLoader.getZkController();
+ final SolrZkClient zkClient = zkController.getZkClient();
+ String managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + managedSchemaResourceName;
+ final String legacyManagedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME;
+ try {
+ // check if we are using the legacy managed-schema file name.
+ if (zkClient.exists(legacyManagedSchemaPath, true)){
+ log.debug("Legacy managed schema resource {} found - loading legacy managed schema instead of {} file."
+ , ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME, managedSchemaResourceName);
+ managedSchemaPath = legacyManagedSchemaPath;
+ }
+ } catch (KeeperException e) {
+ throw new RuntimeException(e);
+ } catch (InterruptedException e) {
+ // Restore the interrupted status
+ Thread.currentThread().interrupt();
+ throw new RuntimeException(e);
+ }
+ return managedSchemaPath;
+ }
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchemaFactory.
+ * @see org.apache.solr.schema.ManagedIndexSchemaFactory#lookupZKManagedSchemaPath
+ */
+ public Path lookupLocalManagedSchemaPath() {
+ final Path legacyManagedSchemaPath = Paths.get(loader.getConfigPath().toString(), ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME);
+
+ Path managedSchemaPath = Paths.get(loader.getConfigPath().toString(), managedSchemaResourceName);
Review comment:
*PATH_TRAVERSAL_IN:* This API (java/nio/file/Paths.get(Ljava/lang/String;[Ljava/lang/String;)Ljava/nio/file/Path;) reads a file whose location might be specified by user input [(details)](https://find-sec-bugs.github.io/bugs.htm#PATH_TRAVERSAL_IN)
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +95,66 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ */
+ public String lookupZKManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
+ final ZkController zkController = zkLoader.getZkController();
+ final SolrZkClient zkClient = zkController.getZkClient();
+ String managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + managedSchemaResourceName;
+ final String legacyManagedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME;
+ try {
+ // check if we are using the legacy managed-schema file name.
+ if (zkClient.exists(legacyManagedSchemaPath, true)){
+ log.debug("Legacy managed schema resource {} found - loading legacy managed schema instead of {} file."
+ , ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME, managedSchemaResourceName);
+ managedSchemaPath = legacyManagedSchemaPath;
+ }
+ } catch (KeeperException e) {
+ throw new RuntimeException(e);
+ } catch (InterruptedException e) {
+ // Restore the interrupted status
+ Thread.currentThread().interrupt();
+ throw new RuntimeException(e);
+ }
+ return managedSchemaPath;
+ }
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchemaFactory.
+ * @see org.apache.solr.schema.ManagedIndexSchemaFactory#lookupZKManagedSchemaPath
+ */
+ public Path lookupLocalManagedSchemaPath() {
+ final Path legacyManagedSchemaPath = Paths.get(loader.getConfigPath().toString(), ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME);
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `ManagedIndexSchemaFactory.lookupLocalManagedSchemaPath()` reads without synchronization from `this.loader`. Potentially races with write in method `ManagedIndexSchemaFactory.create(...)`.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
##########
@@ -91,6 +95,66 @@ public void init(NamedList<?> args) {
public String getSchemaResourceName(String cdResourceName) {
return managedSchemaResourceName; // actually a guess; reality depends on the actual files in the config set :-(
}
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchema.
+ */
+ public String lookupZKManagedSchemaPath() {
+ final ZkSolrResourceLoader zkLoader = (ZkSolrResourceLoader)loader;
+ final ZkController zkController = zkLoader.getZkController();
+ final SolrZkClient zkClient = zkController.getZkClient();
+ String managedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + managedSchemaResourceName;
+ final String legacyManagedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME;
+ try {
+ // check if we are using the legacy managed-schema file name.
+ if (zkClient.exists(legacyManagedSchemaPath, true)){
+ log.debug("Legacy managed schema resource {} found - loading legacy managed schema instead of {} file."
+ , ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME, managedSchemaResourceName);
+ managedSchemaPath = legacyManagedSchemaPath;
+ }
+ } catch (KeeperException e) {
+ throw new RuntimeException(e);
+ } catch (InterruptedException e) {
+ // Restore the interrupted status
+ Thread.currentThread().interrupt();
+ throw new RuntimeException(e);
+ }
+ return managedSchemaPath;
+ }
+
+ /**
+ * Lookup the path to the managed schema, dealing with falling back to the
+ * legacy managed-schema file, instead of the expected managed-schema.xml file.
+ *
+ * This method is duplicated in ManagedIndexSchemaFactory.
+ * @see org.apache.solr.schema.ManagedIndexSchemaFactory#lookupZKManagedSchemaPath
+ */
+ public Path lookupLocalManagedSchemaPath() {
+ final Path legacyManagedSchemaPath = Paths.get(loader.getConfigPath().toString(), ManagedIndexSchemaFactory.LEGACY_MANAGED_SCHEMA_RESOURCE_NAME);
+
+ Path managedSchemaPath = Paths.get(loader.getConfigPath().toString(), managedSchemaResourceName);
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `ManagedIndexSchemaFactory.lookupLocalManagedSchemaPath()` reads without synchronization from `this.managedSchemaResourceName`. Potentially races with write in method `ManagedIndexSchemaFactory.create(...)`.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on a change in pull request #264: SOLR-10887: Append .xml to managed-schema file
Posted by GitBox <gi...@apache.org>.
epugh commented on a change in pull request #264:
URL: https://github.com/apache/solr/pull/264#discussion_r698414812
##########
File path: solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
##########
@@ -342,7 +342,7 @@ private Path checkPathIsSafe(Path pathToCheck) throws IOException {
*/
@Override
public InputStream openResource(String resource) throws IOException {
-
Review comment:
Just learned how to do this properly! Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org