[GitHub] jena pull request: JENA-878 Avoid exposing xerces.impl.dv
Github user afs commented on a diff in the pull request: https://github.com/apache/jena/pull/31#discussion_r24904425 --- Diff: jena-core/src/main/java/com/hp/hpl/jena/datatypes/xsd/impl/XSDGenericType.java --- @@ -1,66 +0,0 @@ -/* --- End diff -- Why is this moved to XSDDatatype? Seems to be just stylistic. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: JENA-878 Avoid exposing xerces.impl.dv
Github user afs commented on a diff in the pull request: https://github.com/apache/jena/pull/31#discussion_r24904627 --- Diff: jena-core/src/main/java/com/hp/hpl/jena/datatypes/xsd/XSDDatatype.java --- @@ -18,34 +18,52 @@ package com.hp.hpl.jena.datatypes.xsd; -import java.io.Reader ; -import java.math.BigDecimal ; -import java.math.BigInteger ; +import java.io.Reader; +import java.math.BigDecimal; +import java.math.BigInteger; import java.net.URI; -import java.util.ArrayList ; -import java.util.List ; - -import org.apache.xerces.impl.dv.* ; -import org.apache.xerces.impl.dv.util.Base64 ; -import org.apache.xerces.impl.dv.util.HexBin ; -import org.apache.xerces.impl.dv.xs.DecimalDV ; -import org.apache.xerces.impl.dv.xs.XSSimpleTypeDecl ; -import org.apache.xerces.impl.validation.ValidationState ; -import org.apache.xerces.parsers.XMLGrammarPreparser ; -import org.apache.xerces.util.SymbolHash ; -import org.apache.xerces.xni.grammars.XMLGrammarDescription ; -import org.apache.xerces.xni.grammars.XSGrammar ; -import org.apache.xerces.xni.parser.XMLInputSource ; -import org.apache.xerces.xs.XSConstants ; -import org.apache.xerces.xs.XSNamedMap ; -import org.apache.xerces.xs.XSTypeDefinition ; - -import com.hp.hpl.jena.datatypes.BaseDatatype ; -import com.hp.hpl.jena.datatypes.DatatypeFormatException ; -import com.hp.hpl.jena.datatypes.RDFDatatype ; -import com.hp.hpl.jena.datatypes.TypeMapper ; -import com.hp.hpl.jena.datatypes.xsd.impl.* ; -import com.hp.hpl.jena.graph.impl.LiteralLabel ; +import java.util.ArrayList; +import java.util.List; + +import org.apache.xerces.impl.dv.InvalidDatatypeValueException; +import org.apache.xerces.impl.dv.SchemaDVFactory; +import org.apache.xerces.impl.dv.ValidatedInfo; +import org.apache.xerces.impl.dv.ValidationContext; +import org.apache.xerces.impl.dv.XSSimpleType; +import org.apache.xerces.impl.dv.util.Base64; +import org.apache.xerces.impl.dv.util.HexBin; +import org.apache.xerces.impl.dv.xs.DecimalDV; +import org.apache.xerces.impl.dv.xs.XSSimpleTypeDecl; +import org.apache.xerces.impl.validation.ValidationState; +import org.apache.xerces.parsers.XMLGrammarPreparser; +import org.apache.xerces.util.SymbolHash; +import org.apache.xerces.xni.grammars.XMLGrammarDescription; +import org.apache.xerces.xni.grammars.XSGrammar; +import org.apache.xerces.xni.parser.XMLInputSource; +import org.apache.xerces.xs.XSConstants; +import org.apache.xerces.xs.XSNamedMap; +import org.apache.xerces.xs.XSTypeDefinition; + +import com.hp.hpl.jena.datatypes.BaseDatatype; +import com.hp.hpl.jena.datatypes.DatatypeFormatException; +import com.hp.hpl.jena.datatypes.RDFDatatype; +import com.hp.hpl.jena.datatypes.TypeMapper; +import com.hp.hpl.jena.datatypes.xsd.impl.XSDBaseNumericType; +import com.hp.hpl.jena.datatypes.xsd.impl.XSDBaseStringType; +import com.hp.hpl.jena.datatypes.xsd.impl.XSDByteType; +import com.hp.hpl.jena.datatypes.xsd.impl.XSDDateTimeType; +import com.hp.hpl.jena.datatypes.xsd.impl.XSDDateType; +import com.hp.hpl.jena.datatypes.xsd.impl.XSDDayType; +import com.hp.hpl.jena.datatypes.xsd.impl.XSDDouble; +import com.hp.hpl.jena.datatypes.xsd.impl.XSDDurationType; +import com.hp.hpl.jena.datatypes.xsd.impl.XSDFloat; +import com.hp.hpl.jena.datatypes.xsd.impl.XSDMonthDayType; +import com.hp.hpl.jena.datatypes.xsd.impl.XSDMonthType; +import com.hp.hpl.jena.datatypes.xsd.impl.XSDPlainType; +import com.hp.hpl.jena.datatypes.xsd.impl.XSDTimeType; +import com.hp.hpl.jena.datatypes.xsd.impl.XSDYearMonthType; +import com.hp.hpl.jena.datatypes.xsd.impl.XSDYearType; +import com.hp.hpl.jena.graph.impl.LiteralLabel; --- End diff -- Gnerally, we havn't enforced style of imports. These will get lost is anyone uses Eclipse to reformat and has * collapse on (which it is by default). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: JENA-878 Avoid exposing xerces.impl.dv
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/31#discussion_r24980025
--- Diff:
jena-core/src/main/java/com/hp/hpl/jena/datatypes/xsd/XSDDatatype.java ---
@@ -251,7 +269,7 @@ public XSDDatatype(String typeName, Class javaClass)
{
* @param xstype the XSSimpleType definition to be wrapped
* @param namespace the namespace for the type (used because the
grammar loading doesn't seem to keep that)
*/
-public XSDDatatype(XSSimpleType xstype, String namespace) {
+XSDDatatype(XSSimpleType xstype, String namespace) {
--- End diff --
Jena provides all the meaningful XSD types (just some XML specific types
are missing) so it is unlikely that external code is calling this public
constructor. Howver, it is public. If the aim is to hide Xerces XSSimpleType,
then hop
about:
1. Make this protected.
2. Add `public XSDDatatype(Object xstype, String namespace)` and cast to
`XSSimpleType`
I thhnk that will keep OSGi happy(ier).
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-901 Upper bound on LPDRuleEngine cache
Github user afs commented on a diff in the pull request: https://github.com/apache/jena/pull/44#discussion_r26858470 --- Diff: jena-core/pom.xml --- @@ -45,6 +45,13 @@ test + + com.carrotsearch + java-sizeof + 0.0.4 + test + + --- End diff -- What is this? What is the license? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: JENA-901 Upper bound on LPDRuleEngine cache
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/44#discussion_r26859236
--- Diff: jena-core/src/main/java/com/hp/hpl/jena/util/BoundedLRUMap.java
---
@@ -0,0 +1,58 @@
+/*
+ * 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 com.hp.hpl.jena.util;
+
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+/**
+ * Bounded Map with a maximum size.
+ *
+ * On insertion of entries beyond the maximum size, the eldest accessed
entry is
+ * removed.
+ *
+ * @param
+ *Type of keys
+ * @param
+ *Type of values
+ */
+public class BoundedLRUMap extends LinkedHashMap implements
Map {
+
+ private static final long serialVersionUID = -1424511852972661771L;
+ private int maxEntries;
+
+ /**
+* Construct a BoundedLRUMap
+*
+* @param maxEntries Maximum number of entries
+*/
+ public BoundedLRUMap(int maxEntries) {
+ super(Math.max(maxEntries/16, 16), 0.75f, true);
+ if (maxEntries <= 0) {
+ throw new IllegalArgumentException("maxEntries <= 0");
+ }
+ this.maxEntries = maxEntries;
+ }
+
+ @Override
+ protected boolean removeEldestEntry(java.util.Map.Entry eldest) {
+ return size() > maxEntries;
+ }
+}
--- End diff --
Seems to duplicate existing oaj.atlas.lib.cache.CacheLRU. At a minimum
move and use that once so it is then findable for any later upgrade to Guava
rather than leave yet another separate cache implementation around. Better
still the cache discussion on the dev@ list. Putting in ad-hoc fixes drives up
the long term maintence overhead.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: Guava as shaded dependency
Github user afs commented on a diff in the pull request: https://github.com/apache/jena/pull/45#discussion_r26902155 --- Diff: jena-shadowed-ext/pom.xml --- @@ -0,0 +1,107 @@ + + + +http://maven.apache.org/POM/4.0.0"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd";> + + 4.0.0 + +org.apache.jena +jena-parent +13-SNAPSHOT +../jena-parent + + jena-shadowed-ext + 2.14.0-SNAPSHOT + bundle + Apache Jena - Shadowed external libraries + +This module shadows some external libraries like Guava, +which we don't want to be depending on in their regular +package names, as they are likely to change/upgrade +externally and also be used in other libraries +in potentially conflicting versions. + +This module uses the Shade plugin to re-package them +under the package name +org.apache.jena.ext and is also a valid OSGi bundle +(but should only be used by used by other Jena modules). + + + + + + + com.google.guava + guava + + 18.0 + + + + + + + +org.apache.maven.plugins +maven-shade-plugin + + +package + + shade + + + + + + + com.google.guava:guava + + + + + com.google + org.apache.jena.ext.com.google + + + + + +org.apache.felix --- End diff -- I don't understand why this needed. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Guava as shaded dependency
Github user afs commented on a diff in the pull request: https://github.com/apache/jena/pull/45#discussion_r26902175 --- Diff: jena-shadowed-ext/pom.xml --- @@ -0,0 +1,107 @@ + + + +http://maven.apache.org/POM/4.0.0"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd";> + + 4.0.0 + +org.apache.jena +jena-parent +13-SNAPSHOT +../jena-parent + + jena-shadowed-ext + 2.14.0-SNAPSHOT + bundle + Apache Jena - Shadowed external libraries + +This module shadows some external libraries like Guava, +which we don't want to be depending on in their regular +package names, as they are likely to change/upgrade +externally and also be used in other libraries +in potentially conflicting versions. + +This module uses the Shade plugin to re-package them +under the package name +org.apache.jena.ext and is also a valid OSGi bundle +(but should only be used by used by other Jena modules). + + + + + + + com.google.guava + guava + + 18.0 + + + + + + + +org.apache.maven.plugins +maven-shade-plugin + + +package + + shade + + + + + + + com.google.guava:guava + + + + + com.google + org.apache.jena.ext.com.google --- End diff -- More focused is to shade com.google.common and com.google.thirdparty --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Guava as shaded dependency
Github user afs commented on a diff in the pull request: https://github.com/apache/jena/pull/45#discussion_r26977608 --- Diff: jena-shadowed-ext/pom.xml --- @@ -0,0 +1,107 @@ + + + +http://maven.apache.org/POM/4.0.0"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd";> + + 4.0.0 + +org.apache.jena +jena-parent +13-SNAPSHOT +../jena-parent + + jena-shadowed-ext + 2.14.0-SNAPSHOT + bundle + Apache Jena - Shadowed external libraries + +This module shadows some external libraries like Guava, +which we don't want to be depending on in their regular +package names, as they are likely to change/upgrade +externally and also be used in other libraries +in potentially conflicting versions. + +This module uses the Shade plugin to re-package them +under the package name +org.apache.jena.ext and is also a valid OSGi bundle +(but should only be used by used by other Jena modules). + + + + + + + com.google.guava + guava + + 18.0 + + + + + + + +org.apache.maven.plugins +maven-shade-plugin + + +package + + shade + + + + + + + com.google.guava:guava + + + + + com.google + org.apache.jena.ext.com.google --- End diff -- I was wodnering what if there is a com.google, not related to guava unrelated that is not to be shaded. I have been trying out: com.google.common then com.google.thirdparty --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Re-enable jena-osgi-tests
Github user afs commented on the pull request: https://github.com/apache/jena/pull/49#issuecomment-88627481 This does not work: `mvn clean verify` when run either in jena-osgi-test or for the whole system give test failures: (stack trace removed) ``` Running org.apache.jena.osgi.test.JenaOSGITest [org.ops4j.pax.exam.nat.internal.NativeTestContainer] : Bundle [org.apache.jena.osgi_2.13.1.SNAPSHOT [15]] is not resolved ERROR: Bundle org.apache.jena.osgi [15] Error starting mvn:org.apache.jena/jena-osgi/2.13.1-SNAPSHOT (org.osgi.framework.BundleException: Unresolved constraint in bundle org.apache.jena.osgi [15]: Unable to resolve 15.0: missing requirement [15.0] osgi.wiring.package; (osgi.wiring.package=sun.misc)) org.osgi.framework.BundleException: Unresolved constraint in bundle org.apache.jena.osgi [15]: Unable to resolve 15.0: missing requirement [15.0] osgi.wiring.package; (osgi.wiring.package=sun.misc) at org.apache.felix.framework.Felix.resolveBundleRevision(Felix.java:4097) at org.apache.felix.framework.Felix.startBundle(Felix.java:2114) at org.apache.felix.framework.Felix.setActiveStartLevel(Felix.java:1368) at org.apache.felix.framework.FrameworkStartLevelImpl.run(FrameworkStartLevelImpl.java:308) at java.lang.Thread.run(Thread.java:745) [org.ops4j.pax.exam.nat.internal.NativeTestContainer] : Bundle [org.apache.jena.osgi [15]] is not resolved Tests run: 4, Failures: 0, Errors: 4, Skipped: 0, Time elapsed: 1.354 sec <<< FAILURE! - in org.apache.jena.osgi.test.JenaOSGITest testJenaArq(org.apache.jena.osgi.test.JenaOSGITest) Time elapsed: 0.008 sec <<< ERROR! java.lang.ClassNotFoundException: com.hp.hpl.jena.rdf.model.RDFNode not found by PAXEXAM-PROBE-455bf793-53a1-40e6-99a9-a7fb4ad5cbf0 [29] ... testJenaIRI(org.apache.jena.osgi.test.JenaOSGITest) Time elapsed: 0.002 sec <<< ERROR! java.lang.ClassNotFoundException: com.hp.hpl.jena.rdf.model.RDFNode not found by PAXEXAM-PROBE-455bf793-53a1-40e6-99a9-a7fb4ad5cbf0 [29] ... testJenaTdb(org.apache.jena.osgi.test.JenaOSGITest) Time elapsed: 0.001 sec <<< ERROR! java.lang.ClassNotFoundException: com.hp.hpl.jena.rdf.model.RDFNode not found by PAXEXAM-PROBE-455bf793-53a1-40e6-99a9-a7fb4ad5cbf0 [29] ... testJenaCore(org.apache.jena.osgi.test.JenaOSGITest) Time elapsed: 0.001 sec <<< ERROR! java.lang.ClassNotFoundException: com.hp.hpl.jena.rdf.model.RDFNode not found by PAXEXAM-PROBE-455bf793-53a1-40e6-99a9-a7fb4ad5cbf0 [29] ... Results : Tests in error: JenaOSGITest.testJenaArq » ClassNotFound com.hp.hpl.jena.rdf.model.RDFNode not... JenaOSGITest.testJenaIRI » ClassNotFound com.hp.hpl.jena.rdf.model.RDFNode not... JenaOSGITest.testJenaTdb » ClassNotFound com.hp.hpl.jena.rdf.model.RDFNode not... JenaOSGITest.testJenaCore » ClassNotFound com.hp.hpl.jena.rdf.model.RDFNode no... ``` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Re-enable jena-osgi-tests
Github user afs commented on the pull request: https://github.com/apache/jena/pull/49#issuecomment-88628699 After pulling this, I get errors with mvn clean verify in jena-osgi-test (ditto a top level rebuild). ``` Running org.apache.jena.osgi.test.JenaOSGITest [org.ops4j.pax.exam.nat.internal.NativeTestContainer] : Bundle [org.apache.jena.osgi_2.13.1.SNAPSHOT [15]] is not resolved ERROR: Bundle org.apache.jena.osgi [15] Error starting mvn:org.apache.jena/jena-osgi/2.13.1-SNAPSHOT (org.osgi.framework.BundleException: Unresolved constraint in bundle org.apache.jena.osgi [15]: Unable to resolve 15.0: missing requirement [15.0] osgi.wiring.package; (osgi.wiring.package=sun.misc)) org.osgi.framework.BundleException: Unresolved constraint in bundle org.apache.jena.osgi [15]: Unable to resolve 15.0: missing requirement [15.0] osgi.wiring.package; (osgi.wiring.package=sun.misc) at org.apache.felix.framework.Felix.resolveBundleRevision(Felix.java:4097) at org.apache.felix.framework.Felix.startBundle(Felix.java:2114) at org.apache.felix.framework.Felix.setActiveStartLevel(Felix.java:1368) at org.apache.felix.framework.FrameworkStartLevelImpl.run(FrameworkStartLevelImpl.java:308) at java.lang.Thread.run(Thread.java:745) [org.ops4j.pax.exam.nat.internal.NativeTestContainer] : Bundle [org.apache.jena.osgi [15]] is not resolved Tests run: 4, Failures: 0, Errors: 4, Skipped: 0, Time elapsed: 1.354 sec <<< FAILURE! - in org.apache.jena.osgi.test.JenaOSGITest testJenaArq(org.apache.jena.osgi.test.JenaOSGITest) Time elapsed: 0.008 sec <<< ERROR! java.lang.ClassNotFoundException: com.hp.hpl.jena.rdf.model.RDFNode not found by PAXEXAM-PROBE-455bf793-53a1-40e6-99a9-a7fb4ad5cbf0 [29] ... testJenaIRI(org.apache.jena.osgi.test.JenaOSGITest) Time elapsed: 0.002 sec <<< ERROR! java.lang.ClassNotFoundException: com.hp.hpl.jena.rdf.model.RDFNode not found by PAXEXAM-PROBE-455bf793-53a1-40e6-99a9-a7fb4ad5cbf0 [29] ... testJenaTdb(org.apache.jena.osgi.test.JenaOSGITest) Time elapsed: 0.001 sec <<< ERROR! java.lang.ClassNotFoundException: com.hp.hpl.jena.rdf.model.RDFNode not found by PAXEXAM-PROBE-455bf793-53a1-40e6-99a9-a7fb4ad5cbf0 [29] ... testJenaCore(org.apache.jena.osgi.test.JenaOSGITest) Time elapsed: 0.001 sec <<< ERROR! java.lang.ClassNotFoundException: com.hp.hpl.jena.rdf.model.RDFNode not found by PAXEXAM-PROBE-455bf793-53a1-40e6-99a9-a7fb4ad5cbf0 [29] ... Results : Tests in error: JenaOSGITest.testJenaArq » ClassNotFound com.hp.hpl.jena.rdf.model.RDFNode not... JenaOSGITest.testJenaIRI » ClassNotFound com.hp.hpl.jena.rdf.model.RDFNode not... JenaOSGITest.testJenaTdb » ClassNotFound com.hp.hpl.jena.rdf.model.RDFNode not... JenaOSGITest.testJenaCore » ClassNotFound com.hp.hpl.jena.rdf.model.RDFNode no... ``` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Re-enable jena-osgi-tests
Github user afs commented on the pull request: https://github.com/apache/jena/pull/49#issuecomment-88641136 I did try mvn in apache-jena-osgi which by dependency linkage should not need the install. But it seems OSGi is different, or the lifecycle isn't aligned, and it now works. However, it then says "org.apache.taverna.ext" -- change to "jena" --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: jena-text multilingual indexing
Github user afs commented on the pull request: https://github.com/apache/jena/pull/51#issuecomment-90649959 Hi there - thank you very much for the pull request. It looks very interesting. Can I ask a couple of things: 1. Who owns the copyright on the code? if you work for a company or institution, often the company or institution owns the copyright. 1. This is based on jena 2.12.1? I tried to apply it to the current codebase but there have been 2 significant contributions since then and the pull request does not align with the codebase nowadays. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: jena-text multilingual indexing
Github user afs commented on the pull request: https://github.com/apache/jena/pull/51#issuecomment-91190852 Hi Alexis, I see that Comète is GPL3-licensed. That means that the project can incorporate source code licenced under the Apache License and still make the overall license GPL3, this is what required by GPL3. The Free Software Foundation and the Apache Software Foundation both recognize this. The other way isn't possible - if software, or changes, are licenced under GPL3, then an Apache License system would need to become GPL3 to fulfil the requirements of GPL3. If the changes can be separated from the rest of the GPL3-codebase and if you are able to license them as Apache License, then at least the licensing issues are sorted out. I acknowledge that they might not be possible because of the way Comète is setup. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: jena-text multilingual indexing (take 2)
Github user afs commented on the pull request: https://github.com/apache/jena/pull/52#issuecomment-98085046 Hi - this is looking good. Merge conflicts arise because of the Jena3 migration. Any packages "com.hp.hpl.jena" have become "org.apache.jena". Don't worry about that, we can fix that up easily enough. I just checked the current state and replacing "com.hp.hpl.jena" by "org.apache.jena" resulted in a valid patch. About tests and documentation - it's quite important to have these so that future changes in and around jena-text don't accidentally break things and so people can find the feature. The documentation does not need to be large. I've create [JENA-928](https://issues.apache.org/jira/browse/JENA-928) to track this. @amiara514 Would it help if I created a branch in the codebase so that we're working against that as a reference point? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: JENA-909 - Docker image for Fuseki 2
Github user afs commented on the pull request: https://github.com/apache/jena/pull/50#issuecomment-98122275 It would be great if this could be tested by someone. The README describes a registry.hub.docker.com image "stain/jena-fuseki" and the document says "stain/jena-fuseki" in quite a few places. What is being delivered by this module or is it more an example that can be pointed to? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: JENA-901 LPDRuleEngine cache Guava from jena-sh...
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/47#discussion_r29499092
--- Diff:
jena-core/src/main/java/com/hp/hpl/jena/reasoner/rulesys/impl/LPBRuleEngine.java
---
@@ -52,16 +66,22 @@
protected boolean recordDerivations;
/** List of engine instances which are still processing queries */
-protected List activeInterpreters = new ArrayList<>();
-
+protected List activeInterpreters = new LinkedList<>();
+
+protected final int MAX_CACHED_TABLED_GOALS =
+
Integer.getInteger("jena.rulesys.lp.max_cached_tabled_goals", 512*1024);
+
--- End diff --
getting System properties is problematic in some environments. See
JenaRuntime.getSystemProperty for some mitigation of this.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-901 LPDRuleEngine cache Guava from jena-sh...
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/47#discussion_r29499213
--- Diff:
jena-core/src/main/java/com/hp/hpl/jena/reasoner/rulesys/impl/LPBRuleEngine.java
---
@@ -252,19 +272,31 @@ public synchronized void tablePredicate(Node
predicate) {
/**
* Return a generator for the given goal (assumes that the caller
knows that
* the goal should be tabled).
+ *
+ * Note: If an earlier Generator for the same goal exists
in the
+ * cache, it will be returned without considering the provided
clauses.
+ *
* @param goal the goal whose results are to be generated
* @param clauses the precomputed set of code blocks used to implement
the goal
*/
-public synchronized Generator generatorFor(TriplePattern goal,
List clauses) {
-Generator generator = tabledGoals.get(goal);
-if (generator == null) {
-LPInterpreter interpreter = new LPInterpreter(this, goal,
clauses, false);
-activeInterpreters.add(interpreter);
-generator = new Generator(interpreter, goal);
-schedule(generator);
-tabledGoals.put(goal, generator);
-}
-return generator;
+public synchronized Generator generatorFor(final TriplePattern goal,
final List clauses) {
+ return getCachedTabledGoal(goal, new Callable() {
+ @Override
+ public Generator call() {
+ /** FIXME: Unify with
#generatorFor(TriplePattern) - but investigate what about
+* the edge case that this method might have
been called with the of goal == null
+* or goal.size()==0 -- which gives different
behaviour in
+* LPInterpreter constructor than through the
route of
+* generatorFor(TriplePattern) which calls a
different LPInterpreter constructor
+* which would fill in from RuleStore.
+*/
--- End diff --
I don't know enough about the rules engine to comment. We need another
reviewer 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-901 LPDRuleEngine cache Guava from jena-sh...
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/47#discussion_r29499192
--- Diff:
jena-core/src/main/java/com/hp/hpl/jena/reasoner/rulesys/impl/LPBRuleEngine.java
---
@@ -252,19 +272,31 @@ public synchronized void tablePredicate(Node
predicate) {
/**
* Return a generator for the given goal (assumes that the caller
knows that
* the goal should be tabled).
+ *
+ * Note: If an earlier Generator for the same goal exists
in the
+ * cache, it will be returned without considering the provided
clauses.
+ *
* @param goal the goal whose results are to be generated
* @param clauses the precomputed set of code blocks used to implement
the goal
*/
-public synchronized Generator generatorFor(TriplePattern goal,
List clauses) {
-Generator generator = tabledGoals.get(goal);
-if (generator == null) {
-LPInterpreter interpreter = new LPInterpreter(this, goal,
clauses, false);
-activeInterpreters.add(interpreter);
-generator = new Generator(interpreter, goal);
-schedule(generator);
-tabledGoals.put(goal, generator);
-}
-return generator;
+public synchronized Generator generatorFor(final TriplePattern goal,
final List clauses) {
+ return getCachedTabledGoal(goal, new Callable() {
+ @Override
--- End diff --
This can be Java8'ed!
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: Deprecating Map1 in favor of Java8 Function
Github user afs commented on the pull request:
https://github.com/apache/jena/pull/54#issuecomment-98128625
Looks good but I'm having a problem with {{OntResourceImpl.AsMapper}}.
This is removed in {{OntResourceImpl}] but still in use in {{OntClassImpl}},
{{EnumeratedClassImpl}}, {{BooleanClassDescriptionImpl}}, {{DataRangeImpl}}.
Maybe some other changes didn't get committed/pushed?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: Deprecating Map1 in favor of Java8 Function
Github user afs commented on the pull request: https://github.com/apache/jena/pull/54#issuecomment-98129164 Created JENA-929 to help track this long-term. Just mention JENA-929 in comments and the JIRA wil be updated automatically. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Deprecating Map1 in favor of Java8 Function
Github user afs commented on the pull request:
https://github.com/apache/jena/pull/54#issuecomment-98129528
Restoring {{AsMapper}} seemed to fix things - at least , it compiled then,
just a bunch of warnings about use of {{Map1}} and unused
{{java.util.Function}} imports so it does look like a minor sync issue.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: Deprecating Map1 in favor of Java8 Function
Github user afs commented on the pull request: https://github.com/apache/jena/pull/54#issuecomment-98133080 Got it - it's clean now. I remove unused imports, replaced Statement.Util and some Triple statics by rewriting the few uses in place e.g. Statement::getSubject. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: First draft of migrating Filter to Java 8 Predi...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/55#issuecomment-98178582 Looks good. If it would be helpful to you, we can break this up into several steps in anyway so as to make it progressive improvement? e.g. even as specific as as rename `Filter.accept` as `Filter.test` as one step on the overall path. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: First draft of migrating Filter to Java 8 Predi...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/55#issuecomment-98217891 FYI: For iterators see `Iter` (in jena-base). Again, pre-java8 but has a use in applying stream-list functions to iterators, instead of converting iterator to stream to iterator. (It came about because not all iterators start as Jena-originated iterators; it applied to Iterable to but that's gone with Java8.) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: First draft of migrating Filter to Java 8 Predi...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/55#issuecomment-98218286 Looks like a good plan. The last point (manipulations) is possibly open-ended than the first two. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: First draft of migrating Filter to Java 8 Predi...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/55#issuecomment-98221746 History! It's used in jena-sdb and jena-tdb. It originated out of work on SDB (pre-iterables) and needed to work on general iterators, not (just) ExtendedIterators. It has not been accessible to core until the jena3 refactoring. I think keep as two PRs - from memory, `Iter` and `ExtendedIterator` don't intersect much. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: First draft of migrating Filter to Java 8 Predi...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/55#issuecomment-98231727 OK - to confirm this PR is not "draft" any more but proposed final. `ExtendedIterator` is different because it is in the `Graph` interface from `find`. That means it is used not just within the codebase of Jena but also by external projects/products that extend Jena. That does not mean it can't change; it needs wider discussion of the tradeoffs though. Jena3 is not compatible with unchanged Jena2 applications but at the same time, if the gap is too great, the changeover is likely longer. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Jena base to java8
Github user afs commented on the pull request: https://github.com/apache/jena/pull/56#issuecomment-99023698 This PR is quite big so comments will be incremental. In fact, it might be better to tease out different sets of changes. Could you give a quick overview of the changes? Overall, this looks like a great set of changes, including using classes available in Google Guava to replace ones in Jena (e.g. MultiMap, MultiSet). `oaj.atlas.lib.Closeable` Adding `extends java.io.Closeable` causes about 76 more warnings in the code base because `java.io.Closeable` is interpreted by the compiler specially. Some of the classes that use `oaj.atlas.lib.Closeable` aren't suitable for try-resource. Maybe some of these classes don't really need closeable but that needs checking. Can we pull this out as a separate strand and not include it in the general Java8 overhaul? `NodeTransform` This is an important abstraction in the SPARQL optimizer. Can we keep it, define it as `extends Function` and keep use of `NodeTransform`? (There are times in Java when it would be nice to say "interface/class A is the same as B" where B is a complex expression, but you can't). It makes finding `NodeTransform`s easier and makes interfaces clearer as to purpose. The optimizer code is not general java code. `@Deprecated` I noticed this on `Lib.equal` and it seems to be a style in the PR to say "Prefer" in the javadoc. It gets highlighted in javadoc if `@deprecated` 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Jena base to java8
Github user afs commented on the pull request: https://github.com/apache/jena/pull/56#issuecomment-99120861 The vast majority of the PR looks good including converting code "in passing". MultiMap?mulitSet are unlikely to have leaked (and can be restored if need be); I've already converted my own code elsewhere not to use them. Where NodeTransform is located is relatively minor - it's not just in "optimize" as the engine to replace nodes by other nodes is used elsewhere (e.g. update, few other places). Other systems may have used it where it is for their own optimizer operations so its a balance of impedance to adopting Jena3 compared to where might go NodeTransform, NodeTransformLib and NodeTransformOp. While such cases are not going to be numerous, they already have the most work to do to migrate. In other words, I'd class it as "minor" and concentrate on other things but that's just my personal take. The sooner we can get to a sufficiently stable version to start basing a release on the better - no "second system" effect, or "third system" effect in this case - and keeping the migration work down encourages users to migrate. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Jena base to java8
Github user afs commented on the pull request: https://github.com/apache/jena/pull/56#issuecomment-99403016 I'll look again but the NodeTransform and the Closeable issues seem to be the main things and after that the size is due to the number of places that Java8 improves things. Checking in my own code MultiMap/MulitSet don't seem likely to have leaked too far. So if we have discussed the different aspects of the PR, it can be applied in the current form as far as I'm concerned. The next step is any other review and discussion then integrate 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Relying more on Guava impl for caching
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/57#discussion_r29938058
--- Diff:
jena-base/src/main/java/org/apache/jena/atlas/lib/CacheFactory.java ---
@@ -33,7 +33,7 @@
/** Create a null cache */
public static Cache createNullCache() {
-return new Cache0<>() ;
+return new CacheGuava<>(0) ;
}
--- End diff --
Cache0 is really just a "complete the set" implementation. If any use as a
dummy, the weight of CacheGuava might not be worth it. Proposal: just remove
Cache0 (it does not fit with getOrFill anyway).
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: Relying more on Guava impl for caching
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/57#discussion_r29938061
--- Diff:
jena-base/src/main/java/org/apache/jena/atlas/lib/CacheFactory.java ---
@@ -43,7 +43,7 @@
/** One slot cache */
public static Cache createOneSlotCache() {
-return new Cache1<>() ;
+return new CacheGuava<>(1) ;
}
--- End diff --
CacheGuava is a bit heavy for a one-slot cache. While Cahce1 may not be
used currently, if it were, then keeping it as simple, small implementation is
better. Proposal: Keep Cache1 as is.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: Relying more on Guava impl for caching
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/57#discussion_r29938052
--- Diff:
jena-base/src/main/java/org/apache/jena/atlas/lib/cache/Cache1.java ---
@@ -1,127 +0,0 @@
-/*
- * 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.jena.atlas.lib.cache;
-
-import java.util.Iterator ;
-import java.util.Objects;
-import java.util.concurrent.Callable ;
-import java.util.function.BiConsumer;
-
-import org.apache.jena.atlas.iterator.SingletonIterator ;
-import org.apache.jena.atlas.lib.Cache ;
-
-/** A one-slot cache.*/
-public class Cache1 implements Cache
-{
--- End diff --
Proposal: keep this.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: Relying more on Guava impl for caching
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/57#discussion_r29938759
--- Diff:
jena-base/src/main/java/org/apache/jena/atlas/lib/CacheFactory.java ---
@@ -33,7 +33,7 @@
/** Create a null cache */
public static Cache createNullCache() {
-return new Cache0<>() ;
+return new CacheGuava<>(0) ;
}
--- End diff --
Yes. It would not be hard to bring it back!
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: Relying more on Guava impl for caching
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/57#discussion_r29938974
--- Diff:
jena-base/src/main/java/org/apache/jena/atlas/lib/CacheFactory.java ---
@@ -43,7 +43,7 @@
/** One slot cache */
public static Cache createOneSlotCache() {
-return new Cache1<>() ;
+return new CacheGuava<>(1) ;
}
--- End diff --
It's not used (if it is used elsewhere, then as it's not API, reasonable
change is OK). The cache interface is useful though and `Supplier` does not
cover the `getOrFill` pattern. It also allows a switch from `Cache1` to a
bigger, better cache as the interface is the same.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: Relying more on Guava impl for caching
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/57#discussion_r29938565
--- Diff:
jena-base/src/main/java/org/apache/jena/atlas/lib/cache/CacheSimple.java ---
@@ -37,120 +37,81 @@
public class CacheSimple implements Cache
{
-private final V[] values ;
-private final K[] keys ;
-private final int size ;
-private int currentSize = 0 ;
-private BiConsumer dropHandler = null ;
+ private Map internalCache;
+
--- End diff --
A Map grows as more, different, keys are added whereas the existing
CacheSimple is a fixed size slot replacement cache with the property that it
never resizes.
CacheSimple is a fast cache, traded against a poorer than LRU
replacement/eviction policy of same-hash slot replacement. Using a Map changes
its characteristics.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: Relying more on Guava impl for caching
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/57#discussion_r29939141
--- Diff:
jena-base/src/main/java/org/apache/jena/atlas/lib/cache/CacheSimple.java ---
@@ -37,120 +37,81 @@
public class CacheSimple implements Cache
{
-private final V[] values ;
-private final K[] keys ;
-private final int size ;
-private int currentSize = 0 ;
-private BiConsumer dropHandler = null ;
+ private Map internalCache;
+
--- End diff --
If keys K1 and K2 have the same short hash, then one adding K2 overwrites
k1 in CacheSimple but in a java Map, you end up with two entries K1 and K2,
with internal reorganisation as needed and the Map is bigger. CacheSimple is a
sort of fixed size map.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: Relying more on Guava impl for caching
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/57#discussion_r29957079
--- Diff:
jena-base/src/main/java/org/apache/jena/atlas/lib/cache/CacheSimple.java ---
@@ -37,120 +37,81 @@
public class CacheSimple implements Cache
{
-private final V[] values ;
-private final K[] keys ;
-private final int size ;
-private int currentSize = 0 ;
-private BiConsumer dropHandler = null ;
+ private Map internalCache;
+
--- End diff --
LinkedHashMap implement an LRU policy and is what the old CacheLRU used.
We swapped that to CacheGuava and removed CacheLRU. Using LinkedHashMap again
means we have two LRU caches.
CacheSimple has a specific policy of slot replacement should that be
wanted. We can leave it be and decide whether to delete it later.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: Relying more on Guava impl for caching
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/57#discussion_r29957286
--- Diff:
jena-tdb/src/main/java/org/apache/jena/tdb/base/block/BlockMgrCache.java ---
@@ -58,10 +58,7 @@ private BlockMgrCache(int readSlots, int writeSlots,
final BlockMgr blockMgr)
{
--- End diff --
I hadn't realized that Cache0 was used. My bad.
The use in BlockMgrCache for a "not a cache really" avoid null checking or
some such in BlockMgrCache.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: Relying more on Guava impl for caching
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/57#discussion_r29957731
--- Diff:
jena-tdb/src/main/java/org/apache/jena/tdb/base/block/BlockMgrCache.java ---
@@ -58,10 +58,7 @@ private BlockMgrCache(int readSlots, int writeSlots,
final BlockMgr blockMgr)
{
--- End diff --
`Cache::getIfPresent` returning Optional means that the calling code ends
up checking Optional rather than null. Looking in TDB, for example, it's no
gain, in fact it's a loss because of the additional object creation.
The reason is that these aren't in streams where Optional skipping is
natural.
The node cache is performance sensitive and uses getIfPresent so changes
here might have undesirable effects.
What would be good is looking at the unrelated cache code in jena-core and
seeing if it can now be consolidated (including using LRU caching which IIRC it
does not).
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: Relying more on Guava impl for caching
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/57#discussion_r29958189
--- Diff:
jena-tdb/src/main/java/org/apache/jena/tdb/base/block/BlockMgrCache.java ---
@@ -58,10 +58,7 @@ private BlockMgrCache(int readSlots, int writeSlots,
final BlockMgr blockMgr)
{
--- End diff --
Cache0 is the `Optional` of caches :-)
`BlockMgrCache` is mainly used for 32-bit TDB only - 64-bit uses memory
mapped file and does not layer a `BlockMgrCache` on top.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: Relying more on Guava impl for caching
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/57#discussion_r29995826
--- Diff:
jena-base/src/main/java/org/apache/jena/atlas/lib/cache/CacheSimple.java ---
@@ -37,120 +37,81 @@
public class CacheSimple implements Cache
{
-private final V[] values ;
-private final K[] keys ;
-private final int size ;
-private int currentSize = 0 ;
-private BiConsumer dropHandler = null ;
+ private Map internalCache;
+
--- End diff --
CacheSimple has a plain old, fashioned bug in maintaining the size!
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: Relying more on Guava impl for caching
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/57#discussion_r29995830
--- Diff:
jena-tdb/src/main/java/org/apache/jena/tdb/base/block/BlockMgrCache.java ---
@@ -58,10 +58,7 @@ private BlockMgrCache(int readSlots, int writeSlots,
final BlockMgr blockMgr)
{
--- End diff --
The current clearing up makes a good checkpoint.
This is ready to integrate now?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: Relying more on Guava impl for caching
Github user afs commented on the pull request: https://github.com/apache/jena/pull/57#issuecomment-100894301 I fixed the bug while I was looking at it :-) I'll review and try out the current state and merge it if there are no further comments. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Relying more on Guava impl for caching
Github user afs commented on the pull request: https://github.com/apache/jena/pull/57#issuecomment-101003455 Thank you. Merged into code base, and reflecting the conversion here: Keep Cache0 (avoid nulls!) and keep the general CacheSet over a Cache due to otherwise changing the eviction policy in TDB missing nodes cache. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Relying more on Guava impl for caching
Github user afs commented on the pull request: https://github.com/apache/jena/pull/57#issuecomment-101007860 Good suggestion. I'll add that as I do #59. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: JENA-938: Nonfunctional cleanup in various modu...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/60#issuecomment-101010157 The files in package "thrift/wire" starting `RDF_` are autogenerated by the Thrift code generator. They'll get uncleaned if that's remade. (Thrift code generation is not part of the build to reduce the dependency; the script and master grammar are in jena-arq/Grammar/). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: JENA-938: Nonfunctional cleanup in various modu...
Github user afs commented on the pull request:
https://github.com/apache/jena/pull/60#issuecomment-101012215
Thrift is another Apache project. In the Thrift case, all Jena adds the
`@SuppressWarnings("all")` at the top. As the warnings also depend on the
local compiler settings, its hard for a general code generator to generate
clean code. From my experience, that's typical for code generators.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-938: Nonfunctional cleanup in various modu...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/60#issuecomment-101013671 Please, if it's easy to do that; ditto the javacc overwrites. (sparql_10 are sparql_11 actually code directly from SPARQL spec so anything touching them gets special attention!) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: JENA-938: Nonfunctional cleanup in various modu...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/60#issuecomment-101015106 Yes. (Sorry about assuming that was clear - over-familiarity with the code layout) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: JENA-938: Nonfunctional cleanup in various modu...
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/63#discussion_r30086187
--- Diff: jena-core/src/main/java/jena/rdfcompare.java ---
@@ -108,11 +105,6 @@ protected static void usage() {
protected static void read(Model model, String in, String lang)
throws java.io.FileNotFoundException {
-try {
-URL url = new URL(in);
-model.read(in, lang);
-} catch (java.net.MalformedURLException e) {
-model.read(new FileInputStream(in), "", lang);
-}
+model.read(in, lang);
--- End diff --
An historical note:
That is a way to handle URLs and plain file names. It only works as plain
`model.read(in, lang);` if RIOT is around otherwise there is no HTTP processing
of `model.read(String, .`
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-938: Nonfunctional cleanup in various modu...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/60#issuecomment-101212657 Another code generated batch : org/apache/jena/iri/impl/Lexer* are JFlex generated (these files do all say "autogenerated" in them) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Kill dead things
Github user afs commented on the pull request: https://github.com/apache/jena/pull/58#issuecomment-101213856 JENA-938 (using this PR to cover #60, #61, #62, #63) Some of these are style points: Unthrown checked exceptions: many (not all but most in my sampling) of these are where the interface declaration has an exception but the code implementation does not throw it currently. Autogenerated code from an interface will include the exception declaration and it is a right to have a later change and then throw the exception. This will have an effect if the app code is using the implementing class, so when an interface is part, not all, of a classes contract. Unnecessary superinterface declarations: this is style - if inheriting from a base class. it can be clearer to keep the "implement interface". Needless typecasts: probably a left-over from converting to generics and should be cleared. Or int/long conversion which also can be cleared up. In all cases, a principle of "keep even if redundant to be clear" needs to apply. (For me, the int/long cast conversion can fall into that category - can't remember the rules at the time and probably won't next time I see the code and certainly do not want accidiently truncation!) Unused imports: my personal preference is no unused imports. #58 also mentions unused variables (almost always should go IMO), unused fields (almost always should go, occasional they are internal trackers like stats. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: JENA-938: Nonfunctional cleanup in various modu...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/61#issuecomment-101214478 Actually, it looks like there is quite an impetus to get Hadoop on Java8 (even if Java6/7 bytecodes). EOL is making it a focus of attention. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: JENA-938: Nonfunctional cleanup in various modu...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/61#issuecomment-101215479 One possibility is to keep Java8-isms out of Elephas (on a practical basis) for the moment so backporting to 2.13.0 is a low barrier. Obviously, bug fixes that go into 3.0.0 and not backported are a potential issue and hard to spot in all the cleaning. JIRA should help there. My sense is that most fixes have not been in core/arq which Elephas is reliant on. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: JENA-938: Nonfunctional cleanup in various modu...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/63#issuecomment-101221776 This PR looks easier to process (it does not mix a lot of modules; sometimes that mix means several people are going to be interested, but in different parts). How does this fit with the other PRs? Are all the JENA-938 PR independent and so can be processed in any order? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Kill dead things
Github user afs commented on the pull request: https://github.com/apache/jena/pull/58#issuecomment-101256776 Good point. I wasn't able to catch on this PR before the JENA-938 ones emerged over the weekend so you have got ahead of me and everyone else. My apologies. Finding time has to be balanced with $dayjob and life. The project has not traditionally had such policies decided upfront, they emerge. Now may be a good time to capture them now they have emerged. I am not keen on defining such things too early because such discussions focus on the controversial points yet they c an be a small, important part of the whole. Instead, having concrete code to talk about means focusing on what has the most benefit. Would you like to draft a proposal and email dev@? And then progress to adding some pages to the website. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: JENA-938: Nonfunctional cleanup in various modu...
Github user afs commented on the pull request:
https://github.com/apache/jena/pull/63#issuecomment-101322670
When I applied the changes, I get a lot of "hiding" warnings. Seems to me
the root cause is the hiding in the first place, not the use of
`@SuppressWarnings("hiding")` which, for me at least in Eclipse, is a warning
to be suppressed. Does the use of `@SuppressWarnings("hiding")` cause any
problems?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-938: Nonfunctional cleanup in various modu...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/63#issuecomment-101780114 Generally yes - fixes are better when possible. In this case though (on `.factory`) there is a coding pattern than was fine at the time and now isn't. The hiding effect is due to inheritance of `ResourceImpl` and `OntResourceImpl` . I'll integrate what I can and see if fix up does not muddy things up in this case. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: JENA-938: Nonfunctional cleanup in various modu...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/60#issuecomment-102169543 I've picked out as much material from the PR as I can and applied according to the discussions (so not removing interface declarations when the base type includes it, left exceptions on \@overrides in main code, removed unnecessary exceptions more completely in test code). I have not handled jena-permissions, a module I know less about. One of the reasons split by module is helpful is that different people tend to look after different modules so one person does not have oversight of all code. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: JENA-938: Nonfunctional cleanup in various modu...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/61#issuecomment-102834531 Would it be an easy task to possible to split this into jena-elephas related changes and changas related to other modules? I'm not so familiar with jena-elephas but I could progress the other bits and pieces. (If it's not an easy task, I can take a patch and edit it but the history is lost). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: JENA-938: Nonfunctional cleanup in various modu...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/62#issuecomment-102834572 Would it be an easy task to possible to split this into jena-jdbc related changes and changes related to other modules? (If it's not an easy task, I can take a patch and edit it but the history is lost). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: JENA-938: Nonfunctional cleanup in various modu...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/61#issuecomment-103207166 jena-elephas, querybuilder, and the rest (Fuseki's) would be great. (Actually, looking at the querybuilder file, I wonder if that is a style for a purpose ... @Claudenw?) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: JENA-938: Nonfunctional cleanup in various modu...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/62#issuecomment-103207748 In this PR, jena-jdbc and the rest would make it easier from my point of view. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: JENA-938: Nonfunctional cleanup in various modu...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/61#issuecomment-103566061 I can process the "everything else" PR because I think I understand those code modules well enough. If it's not much work, 3 PRs is a clear cut split but if it turns into a lot of work, plan B is that I can take a patch file and edit 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: JENA-938: Nonfunctional cleanup in various modu...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/62#issuecomment-103564682 Good to be precise! 2 PRs would be fantastic - (1) jena-jdbc and everything below it; they all go to make up jena-jdbc and (2) the rest. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Kill dead things
Github user afs commented on the pull request: https://github.com/apache/jena/pull/58#issuecomment-103970036 Changes based the ideas here have been applied to the codebase (See JENA-938). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: JENA-938: Nonfunctional cleanup in jena-jdbc
Github user afs commented on the pull request: https://github.com/apache/jena/pull/62#issuecomment-103970644 Thanks for the splitting. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: jena-text multilingual indexing (take 2)
Github user afs commented on the pull request: https://github.com/apache/jena/pull/52#issuecomment-104230097 The submitter can close it - I'll close it anyway. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Lucene index synchro on triple deletion (jena-t...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/53#issuecomment-104230830 What's now the relation of this #53 to #64? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: JENA-938: Nonfunctional cleanup in jena-jdbc
Github user afs commented on the pull request: https://github.com/apache/jena/pull/62#issuecomment-104236721 Re: private methods I turned on Eclipse's "warn on unused private member" which includes private methods, (it does not seem to separate the two). A quick scan of code I recognized suggests they represent functionally there for completeness, maybe also used at one time, just not currently active. That represents useful code IMO. I didn't find much that seemed removable so, from my limited experiment, it does not look like it would be so useful / much return-on-investment. "Mostly harmless" --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Lucene index synchro on triple deletion (jena-t...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/53#issuecomment-104307149 Thanks for the explanation. (I was just wondering about cleaning up after the earlier discussion on another out of date PR.) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Jena-text multilingual alternative implementati...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/64#issuecomment-104704987 Generally looks good. I tried applying it as a patch and `TextIndexLuceneMultilingualAssembler` uses non-existent `TextDatasetFactory.createLuceneIndexMultilingual`. Is this PR dependent on another? There is document at [query/text-query](http://jena.apache.org/documentation/query/text-query.html). In what way should that be updated? Classes `analyzer.Util` and `LuceneUtil` appear to be the same code. Would it be possible to put back `EntityDefinition` for maximum compatibility with the original code? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Jena-text multilingual alternative implementati...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/64#issuecomment-104755152 I'm working from the pull request (`.../64.patch`). Strange indeed : the `.diff` is good, the `.patch` is bad. The website is at https://svn.apache.org/repos/asf/jena/site/trunk/ and the source of query.html is from the markdown https://svn.apache.org/repos/asf/jena/site/trunk/content/documentation/query/text-query.mdtext. Either go to the HTML page and click "Improve this Page" or send the updated mdtext to dev@ and I can incorporate into the site. No svn directly involved :-) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Jena-text multilingual alternative implementati...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/64#issuecomment-104760829 The diff and patch are supposed to be the same end result, even if cumulative commits. The patch does not seem to have the later deletes. A bug(let) in github perhaps? I use them to look at a PR before pulling it - it shows what's changed better (for me, in Eclipse) without changing git. For the real thing, I pull requests if taking all of 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Jena-text multilingual alternative implementati...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/64#issuecomment-104784383 "git apply" didn't see those! No matter - the PR is otherwise fine. When the `EntityDefinition` the migration constructor (which can be @Deprecated -- just trying to minimise the bump to jena3) is there, we can merge it for further comment. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: pull master to begin
Github user afs commented on the pull request: https://github.com/apache/jena/pull/71#issuecomment-104994252 A good place to start is with the changes needed for master.jj. If you want to discuss how to use javacc, which you may not have used before, then let's talk on the dev@ mailing list. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Jena-text multilingual alternative implementati...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/64#issuecomment-105223531 PR merged and I've added back the constructors to align to the original `EntityDefinition`. This isn't to say that it is better - just maximised the compatibility with any existing code. Please check I've done this correctly. Last thing to do is the documentation. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Update TextQueryPF.java
Github user afs commented on the pull request: https://github.com/apache/jena/pull/73#issuecomment-105658544 Thanks - PR merged. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Add (?uri ?score) to jena-text
Github user afs commented on the pull request: https://github.com/apache/jena/pull/72#issuecomment-106861440 @osma - I'll find some time to review this but To your points: 1. I dont think that change is a problem and \@Deprecated isn't needed because (a) Jena3 allows us to be a bit more flexibility to do things right and (b) the major use is via SPARQL and that is not affected. I don't recall anyone asking about detailed direct use of the module. 1. IIRC solr works differently and puts in a "score" field in the results. 1. Yes - we already have enough old practices! I'll look at the PR for these. I'd like to proceed getting functionality in and worry about clearing up very soon after. It exposes the changes early for those interested. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Jena-text multilingual alternative implementati...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/64#issuecomment-106875094 Yes - adding the version info is more nice than essential. (Really, we ought to undo references to really old stuff.) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Add (?uri ?score) to jena-text
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/72#discussion_r31388513
--- Diff:
jena-text/src/main/java/org/apache/jena/query/text/TextHitConverter.java ---
@@ -0,0 +1,50 @@
+/**
+ * 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.jena.query.text ;
+
+import java.util.function.Function;
+
+import org.apache.jena.sparql.core.Var;
+import org.apache.jena.sparql.engine.binding.Binding;
+import org.apache.jena.sparql.engine.binding.BindingFactory;
+import org.apache.jena.sparql.engine.binding.BindingMap;
+import org.apache.jena.sparql.util.NodeFactoryExtra;
+
+/** Class that converts TextHits to Bindings that can be returned from
query */
+public class TextHitConverter implements Function
+{
+private Binding binding;
+private Var match;
+private Var score;
+
+public TextHitConverter(Binding binding, Var match, Var score) {
+this.binding = binding;
+this.match = match;
+this.score = score;
+}
+
+public Binding apply(TextHit hit) {
+BindingMap bmap = BindingFactory.create(binding);
+bmap.add(match, hit.getNode());
+if (score != null) {
+bmap.add(score, NodeFactoryExtra.floatToNode(hit.getScore()));
+}
+return bmap;
+}
--- End diff --
Minor improvement: `BindingMap` is a map so it's bigger than the one slot
binding. This is slightly better when not returning the score:
```
public Binding apply(TextHit hit) {
if (score == null)
return BindingFactory.binding(binding, match, hit.getNode()) ;
BindingMap bmap = BindingFactory.create(binding);
bmap.add(match, hit.getNode());
bmap.add(score, NodeFactoryExtra.floatToNode(hit.getScore()));
return bmap ;
}
```
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: Add (?uri ?score) to jena-text
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/72#discussion_r31388517
--- Diff:
jena-text/src/main/java/org/apache/jena/query/text/TextHitConverter.java ---
@@ -0,0 +1,50 @@
+/**
+ * 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.jena.query.text ;
+
+import java.util.function.Function;
+
+import org.apache.jena.sparql.core.Var;
+import org.apache.jena.sparql.engine.binding.Binding;
+import org.apache.jena.sparql.engine.binding.BindingFactory;
+import org.apache.jena.sparql.engine.binding.BindingMap;
+import org.apache.jena.sparql.util.NodeFactoryExtra;
+
+/** Class that converts TextHits to Bindings that can be returned from
query */
+public class TextHitConverter implements Function
+{
+private Binding binding;
+private Var match;
+private Var score;
+
+public TextHitConverter(Binding binding, Var match, Var score) {
+this.binding = binding;
+this.match = match;
+this.score = score;
+}
+
+public Binding apply(TextHit hit) {
--- End diff --
Need `@Override`
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: Add (?uri ?score) to jena-text
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/72#discussion_r31388520
--- Diff:
jena-text/src/main/java/org/apache/jena/query/text/TextHitConverter.java ---
@@ -0,0 +1,50 @@
+/**
+ * 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.jena.query.text ;
+
+import java.util.function.Function;
+
+import org.apache.jena.sparql.core.Var;
+import org.apache.jena.sparql.engine.binding.Binding;
+import org.apache.jena.sparql.engine.binding.BindingFactory;
+import org.apache.jena.sparql.engine.binding.BindingMap;
+import org.apache.jena.sparql.util.NodeFactoryExtra;
+
+/** Class that converts TextHits to Bindings that can be returned from
query */
+public class TextHitConverter implements Function
+{
+private Binding binding;
+private Var match;
+private Var score;
+
+public TextHitConverter(Binding binding, Var match, Var score) {
+this.binding = binding;
+this.match = match;
+this.score = score;
+}
+
--- End diff --
Need `@Override`
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: Add (?uri ?score) to jena-text
Github user afs commented on a diff in the pull request: https://github.com/apache/jena/pull/72#discussion_r31388531 --- Diff: jena-text/src/main/java/org/apache/jena/query/text/TextQueryPF.java --- @@ -27,11 +29,13 @@ import org.apache.jena.datatypes.xsd.XSDDatatype ; import org.apache.jena.graph.Node ; import org.apache.jena.query.QueryBuildException ; +import org.apache.jena.query.QueryExecException ; import org.apache.jena.sparql.core.* ; import org.apache.jena.sparql.engine.ExecutionContext ; import org.apache.jena.sparql.engine.QueryIterator ; import org.apache.jena.sparql.engine.binding.Binding ; import org.apache.jena.sparql.engine.iterator.QueryIterExtendByVar ; --- End diff -- `QueryIterExtendByVar` : Unused import --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Add (?uri ?score) to jena-text
Github user afs commented on a diff in the pull request: https://github.com/apache/jena/pull/72#discussion_r31388532 --- Diff: jena-text/src/main/java/org/apache/jena/query/text/TextQueryPF.java --- @@ -18,7 +18,9 @@ package org.apache.jena.query.text ; +import java.util.Iterator ; import java.util.List ; +import java.util.function.Function ; import org.apache.jena.atlas.iterator.Iter ; import org.apache.jena.atlas.lib.InternalErrorException ; --- End diff -- `InternalErrorException` : Unused import - should be removed. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Add (?uri ?score) to jena-text
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/72#discussion_r31388541
--- Diff:
jena-text/src/main/java/org/apache/jena/query/text/TextQueryPF.java ---
@@ -165,37 +176,48 @@ public QueryIterator exec(Binding binding,
PropFuncArg argSubject, Node predicat
//
QueryIterator qIter = (Var.isVar(s))
-? variableSubject(binding, s, match, execCxt)
-: concreteSubject(binding, s, match, execCxt) ;
+? variableSubject(binding, s, score, match, execCxt)
+: concreteSubject(binding, s, score, match, execCxt) ;
if (match.getLimit() >= 0)
qIter = new QueryIterSlice(qIter, 0, match.getLimit(),
execCxt) ;
return qIter ;
}
-private QueryIterator variableSubject(Binding binding, Node s,
StrMatch match, ExecutionContext execCxt) {
-Var v = Var.alloc(s) ;
-List r = query(match.getQueryString(), match.getLimit(),
execCxt) ;
-// Make distinct. Note interaction with limit is imperfect
-r = Iter.iter(r).distinct().toList() ;
-QueryIterator qIter = new QueryIterExtendByVar(binding, v,
r.iterator(), execCxt) ;
+private QueryIterator variableSubject(Binding binding, Node s, Node
score, StrMatch match, ExecutionContext execCxt) {
+Var sVar = Var.alloc(s) ;
+Var scoreVar = (score==null) ? null : Var.alloc(score) ;
+List r = query(match.getQueryString(), match.getLimit(),
execCxt) ;
+Function converter = new
TextHitConverter(binding, sVar, scoreVar);
+Iterator bIter = new Map1Iterator(converter, r.iterator());
--- End diff --
There is a library of iterator utilities:
```
Iterator bIter = Iter.map(r.iterator(), converter) ;
```
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: Add (?uri ?score) to jena-text
Github user afs commented on the pull request: https://github.com/apache/jena/pull/72#issuecomment-107149537 There are a few `// *** score` markers left - presumably these can all go now. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Add (?uri ?score) to jena-text
Github user afs commented on the pull request:
https://github.com/apache/jena/pull/72#issuecomment-107150120
About changing return result from `DatasetGraphText.search`:
I'm unsure whether it is better to add `searchWithScore` (or better name?),
which returns the `List` form, and retain `search` returning
`List` (just a `Iter.map` added each time):
Example:
```
/** Search the text index on the default text field */
public Iterator searchWithScore(String queryString) {
return search(queryString, null) ;
}
/** Search the text index on the default text field */
public Iterator search(String queryString) {
return Iter.map(searchWithScore(queryString,
textHit->textHit.getNode()) ;
}
```
If the change rote, then Jena3 is the time to do it.
What do you think?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: Add (?uri ?score) to jena-text
Github user afs commented on the pull request: https://github.com/apache/jena/pull/72#issuecomment-107150565 Other than the point-wise comments above, all looks good and the tests pass for me. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Add (?uri ?score) to jena-text
Github user afs commented on the pull request: https://github.com/apache/jena/pull/72#issuecomment-107516262 Re: Map1Iterator -- yes is it similar (and older). `Iter` works on any iterators, not just `ExtendedIterator`. Re: naming of search - your choice. It is alwayss difficult to choose between compatibility and creating legacy. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Jena-text multilingual alternative implementati...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/64#issuecomment-107519936 Thank for pointing that out. The direct reflection of `site/trunk` is http://jena.staging.apache.org/ and the button should work there as well. The website gets published explicitly and we have probably not published it as Jena3 mentions get into the content. Sorry for any confusion. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Jena-text multilingual alternative implementati...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/64#issuecomment-107605937 "Hmm" indeed. I'll trying making a small change to see if it flushes the correct mdtext file. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Add (?uri ?score) to jena-text
Github user afs commented on the pull request: https://github.com/apache/jena/pull/72#issuecomment-107944800 Merge done! Thank you. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Add (?uri ?score) to jena-text
Github user afs commented on the pull request: https://github.com/apache/jena/pull/72#issuecomment-107947521 An update for http://jena.staging.apache.org/documentation/query/text-query.html would be good. Either via through the CMS UI or a diff to https://svn.apache.org/repos/asf/jena/site/trunk/content/documentation/query/text-query.mdtext. Or if those are problematic, text changes emailed to dev@ as I assume we are not talking about a vast change. (caution - as @amiara514 experienced, "[update]" then "update this resource" may be necessary) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Jena-text multilingual alternative implementati...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/64#issuecomment-108096782 @amiara514 -- I'm not seeing a message from you on dev@. Did it get sent? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Jena-text multilingual alternative implementati...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/64#issuecomment-108237260 Sorry your having problems - that should have worked when you press "commit". Just send the modified markdown to me directly and I'll merge the files. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Jena-text multilingual alternative implementati...
Github user afs commented on the pull request: https://github.com/apache/jena/pull/64#issuecomment-108387604 That works great - thanks. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Enhance Javadocs for QueryEngineFactory
Github user afs commented on the pull request: https://github.com/apache/jena/pull/75#issuecomment-109324133 Thanks - I've used this with clarifications. Normally, only query execution process would call the factory operations. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jena pull request: Eliminate /jena.cmdline, deprecate r...
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/76#discussion_r32211834
--- Diff: jena-core/src/main/java/jena/RuleMap.java ---
@@ -50,7 +53,7 @@
*
*/
public class RuleMap {
-static { setLog4jConfiguration() ; }
+static { setCmdLogging("jena-log4j.properties") ; }
--- End diff --
Can this be plain `setCmdLogging()` which (1) is sensitive to a local
log4j.properties if the user that and (2) has a built-in config in case no
properties file is found.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
