You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by arysin <gi...@git.apache.org> on 2017/05/19 19:26:46 UTC

[GitHub] lucene-solr pull request #206: LUCENE-7841: Normalize ghe with upturn

GitHub user arysin opened a pull request:

    https://github.com/apache/lucene-solr/pull/206

    LUCENE-7841: Normalize ghe with upturn

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/arysin/lucene-solr master

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/lucene-solr/pull/206.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #206
    
----
commit 9b1695bb82f790822cca6cf3e2689c9e7f33bb4e
Author: Andriy Rysin <ar...@gmail.com>
Date:   2017-05-19T19:22:37Z

    LUCENE-7841: Normalize ghe with upturn

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[GitHub] lucene-solr pull request: SOLR-8166 provide config for tika's Pars...

Posted by abinet <gi...@git.apache.org>.
Github user abinet commented on the pull request:

    https://github.com/apache/lucene-solr/pull/206#issuecomment-153773456
  
    All done. it it clean enough for merge?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[GitHub] lucene-solr pull request: SOLR-8166 provide config for tika's Pars...

Posted by uschindler <gi...@git.apache.org>.
Github user uschindler commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/206#discussion_r42311743
  
    --- Diff: solr/contrib/extraction/src/test/org/apache/solr/handler/extraction/ParseContextConfigTest.java ---
    @@ -0,0 +1,54 @@
    +package org.apache.solr.handler.extraction;
    +
    +/*
    + * 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.
    + */
    +
    +import org.apache.tika.parser.ParseContext;
    +import org.apache.tika.parser.pdf.PDFParserConfig;
    +import org.apache.xerces.dom.DocumentImpl;
    +import org.junit.Test;
    +import org.w3c.dom.Element;
    +
    +import static org.junit.Assert.*;
    +
    +public class ParseContextConfigTest {
    --- End diff --
    
    Please subclass SolrTestCaseJ4, dont create plain tests, as those don't use the test framework, which does additional checks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[GitHub] lucene-solr pull request: SOLR-8166 provide config for tika's Pars...

Posted by uschindler <gi...@git.apache.org>.
Github user uschindler commented on the pull request:

    https://github.com/apache/lucene-solr/pull/206#issuecomment-150497999
  
    Hi,
    a useful alternative to using commons-beanutils is using the JDK internal bean classes. See https://github.com/apache/lucene-solr/blob/trunk/solr/core/src/java/org/apache/solr/handler/admin/SystemInfoHandler.java#L166-L232 for an example. We read the properties of MXBeans in the SystemInfoHandler here. You can get a BeanInfo from the class and then use the property descriptors to get/set properties. And that is what you are doing.
    Because the JDK code is partly buggy for historical reasons, make sure to use the correct flags added with JDK 7 when inspecting and getting the property descriptors (disabling caches which are broken).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[GitHub] lucene-solr pull request: SOLR-8166 provide config for tika's Pars...

Posted by uschindler <gi...@git.apache.org>.
Github user uschindler commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/206#discussion_r42311723
  
    --- Diff: solr/contrib/extraction/src/test-files/log4j.properties ---
    @@ -0,0 +1,31 @@
    +#  Logging level
    --- End diff --
    
    Please revert changes to this file. They are unrelated.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[GitHub] lucene-solr pull request: SOLR-8166 provide config for tika's Pars...

Posted by abinet <gi...@git.apache.org>.
Github user abinet closed the pull request at:

    https://github.com/apache/lucene-solr/pull/206


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[GitHub] lucene-solr pull request: SOLR-8166 provide config for tika's Pars...

Posted by uschindler <gi...@git.apache.org>.
Github user uschindler commented on the pull request:

    https://github.com/apache/lucene-solr/pull/206#issuecomment-153793014
  
    Hey, yes exactly like that :-) I will review that later. Give me a day or two, I am looking in merging it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[GitHub] lucene-solr pull request: SOLR-8166 provide config for tika's Pars...

Posted by abinet <gi...@git.apache.org>.
Github user abinet commented on the pull request:

    https://github.com/apache/lucene-solr/pull/206#issuecomment-148731447
  
     	SOLR-8166 provide config for tika's ParseContext
    I would still prefer to modify existing ExtractingDocumentLoader rather then provide additional plugin with factory. 
    setAccessible is awoided now 
    And core.resourceClassloader is used


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[GitHub] lucene-solr pull request: SOLR-8166 provide config for tika's Pars...

Posted by uschindler <gi...@git.apache.org>.
Github user uschindler commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/206#discussion_r42311748
  
    --- Diff: solr/contrib/extraction/src/java/org/apache/solr/handler/extraction/ParseContextConfig.java ---
    @@ -0,0 +1,113 @@
    +package org.apache.solr.handler.extraction;
    +
    +/*
    + * 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.
    + */
    +
    +import javax.xml.parsers.DocumentBuilder;
    +import javax.xml.parsers.DocumentBuilderFactory;
    +import java.io.File;
    +import java.lang.reflect.Field;
    +import java.lang.reflect.Method;
    +import java.util.HashMap;
    +import java.util.Map;
    +
    +import org.apache.tika.parser.ParseContext;
    +import org.w3c.dom.Document;
    +import org.w3c.dom.Element;
    +import org.w3c.dom.NamedNodeMap;
    +import org.w3c.dom.Node;
    +import org.w3c.dom.NodeList;
    +
    +public class ParseContextConfig {
    +  private Map<Class, Object> entries = new HashMap<>();
    +
    +  public ParseContextConfig() {
    +  }
    +
    +  public ParseContextConfig(Element element, ClassLoader loader) throws Exception {
    +    extract(element, loader);
    +  }
    +
    +  public ParseContextConfig(Document document, ClassLoader loader) throws Exception {
    +    this(document.getDocumentElement(), loader);
    +  }
    +
    +  public ParseContextConfig(String fileName, ClassLoader loader) throws Exception {
    +    this(getBuilder().parse(fileName), loader);
    +  }
    +
    +  public ParseContextConfig(File file, ClassLoader loader) throws Exception {
    +    this(getBuilder().parse(file), loader);
    +  }
    +
    +  private static DocumentBuilder getBuilder() throws Exception {
    +      return DocumentBuilderFactory.newInstance().newDocumentBuilder();
    +  }
    +
    +  private void extract(Element element, ClassLoader loader) throws Exception {
    +    final NodeList xmlEntries = element.getElementsByTagName("entry");
    +    for (int i = 0; i < xmlEntries.getLength(); i++) {
    +      final NamedNodeMap xmlEntryAttributes = xmlEntries.item(i).getAttributes();
    +      final String className = xmlEntryAttributes.getNamedItem("class").getNodeValue();
    +      final String implementationName = xmlEntryAttributes.getNamedItem("value").getNodeValue();
    +
    +      final NodeList xmlProperties = ((Element)xmlEntries.item(i)).getElementsByTagName("property");
    +
    +        final Class<?> interfaceClass = loader.loadClass(className);
    +        final Class<?> implementationClass = loader.loadClass(implementationName);
    +        final Object instance = implementationClass.newInstance();
    +
    +        for (int j = 0; j < xmlProperties.getLength(); j++) {
    +          final Node xmlProperty = xmlProperties.item(j);
    +          final NamedNodeMap xmlPropertyAttributes = xmlProperty.getAttributes();
    +
    +          final String propertyName = xmlPropertyAttributes.getNamedItem("name").getNodeValue();
    +          final String propertyValue = xmlPropertyAttributes.getNamedItem("value").getNodeValue();
    +
    +          final Field declaredField = interfaceClass.getDeclaredField(propertyName);
    +          final Class<?> type = declaredField.getType();
    +          final Method declaredMethod = interfaceClass.getDeclaredMethod("set" + propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1, propertyName.length()), type);
    --- End diff --
    
    This does not work in Turkey! :-) Don't use String#toUpper/LowerCase() without giving a Locale (Locale.ROOT is needed here)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[GitHub] lucene-solr pull request: SOLR-8166 provide config for tika's Pars...

Posted by abinet <gi...@git.apache.org>.
Github user abinet commented on the pull request:

    https://github.com/apache/lucene-solr/pull/206#issuecomment-153851266
  
    Hi, thank you for support and cooperation. I'll close pull request.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[GitHub] lucene-solr pull request: SOLR-8166 provide config for tika's Pars...

Posted by uschindler <gi...@git.apache.org>.
Github user uschindler commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/206#discussion_r42311693
  
    --- Diff: solr/contrib/extraction/src/java/org/apache/solr/handler/extraction/ExtractingRequestHandler.java ---
    @@ -79,6 +81,20 @@ public void inform(SolrCore core) {
               throw new SolrException(ErrorCode.SERVER_ERROR, e);
             }
           }
    +
    +      String parseContextConfigLoc = (String) initArgs.get(PARSE_CONTEXT_CONFIG);
    +      if (parseContextConfigLoc != null) {
    +        File parseContextConfigFile = new File(parseContextConfigLoc);
    +        if (parseContextConfigFile.isAbsolute() == false) {
    +          parseContextConfigFile = new File(core.getResourceLoader().getConfigDir(), parseContextConfigFile.getPath());
    +        }
    +        try {
    +          parseContextConfig = new ParseContextConfig(parseContextConfigFile, core.getResourceLoader().getClassLoader());
    --- End diff --
    
    I would directly pass the resourceloader and not the classloader. Resourceloader has easy-to-use methods to load classes, too.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[GitHub] lucene-solr pull request: SOLR-8166 provide config for tika's Pars...

Posted by abinet <gi...@git.apache.org>.
Github user abinet commented on the pull request:

    https://github.com/apache/lucene-solr/pull/206#issuecomment-150487320
  
    I fixed all issues you mentioned. It is also possible to use PropertyUtils from Apache commons-beanutils but we need to introduce new dependency then.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[GitHub] lucene-solr pull request: SOLR-8166 provide config for tika's Pars...

Posted by uschindler <gi...@git.apache.org>.
Github user uschindler commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/206#discussion_r42311681
  
    --- Diff: solr/contrib/extraction/src/java/org/apache/solr/handler/extraction/ExtractingRequestHandler.java ---
    @@ -79,6 +81,20 @@ public void inform(SolrCore core) {
               throw new SolrException(ErrorCode.SERVER_ERROR, e);
             }
           }
    +
    +      String parseContextConfigLoc = (String) initArgs.get(PARSE_CONTEXT_CONFIG);
    +      if (parseContextConfigLoc != null) {
    +        File parseContextConfigFile = new File(parseContextConfigLoc);
    +        if (parseContextConfigFile.isAbsolute() == false) {
    +          parseContextConfigFile = new File(core.getResourceLoader().getConfigDir(), parseContextConfigFile.getPath());
    --- End diff --
    
    Please dont create a file instance here, because with Zookeeper there is no config directory available. Just use the ResourceLoader methods to load the config file as InputStream (using RL#getResource()). Just pass this input stream to the XML parser.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[GitHub] lucene-solr pull request: SOLR-8166 provide config for tika's Pars...

Posted by uschindler <gi...@git.apache.org>.
Github user uschindler commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/206#discussion_r42311704
  
    --- Diff: solr/contrib/extraction/src/java/org/apache/solr/handler/extraction/ParseContextConfig.java ---
    @@ -0,0 +1,113 @@
    +package org.apache.solr.handler.extraction;
    +
    +/*
    + * 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.
    + */
    +
    +import javax.xml.parsers.DocumentBuilder;
    +import javax.xml.parsers.DocumentBuilderFactory;
    +import java.io.File;
    +import java.lang.reflect.Field;
    +import java.lang.reflect.Method;
    +import java.util.HashMap;
    +import java.util.Map;
    +
    +import org.apache.tika.parser.ParseContext;
    +import org.w3c.dom.Document;
    +import org.w3c.dom.Element;
    +import org.w3c.dom.NamedNodeMap;
    +import org.w3c.dom.Node;
    +import org.w3c.dom.NodeList;
    +
    +public class ParseContextConfig {
    +  private Map<Class, Object> entries = new HashMap<>();
    +
    +  public ParseContextConfig() {
    --- End diff --
    
    Please remove unused constructors.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[GitHub] lucene-solr pull request: SOLR-8166 provide config for tika's Pars...

Posted by uschindler <gi...@git.apache.org>.
Github user uschindler commented on the pull request:

    https://github.com/apache/lucene-solr/pull/206#issuecomment-153850780
  
    Hi, this was merged into SVN. Can you close the pull request, the automatic close did not work...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org