You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jena.apache.org by ajs6f <gi...@git.apache.org> on 2018/01/01 20:03:10 UTC
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
GitHub user ajs6f opened a pull request:
https://github.com/apache/jena/pull/337
JENA-1391: Dataset dyadic views, Model and Dataset Stream collectors
This is a bit preliminary-- probably needs some more tests. But I wanted to get it out there for early comments so I can take them into account and respond to criticism.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ajs6f/jena JENA-1391
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/jena/pull/337.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #337
----
commit 9fd1f41870b730fcf5ea528b9fd27cd7a4ceb438
Author: ajs6f <aj...@...>
Date: 2017-10-01T14:02:28Z
First draft of Collector utilities
commit 197abd4e0addd131714a541b5a900cea719b2c37
Author: ajs6f <aj...@...>
Date: 2017-10-12T14:07:12Z
Refactoring to assume fresh Context for any Dataset view
commit 4d6d36969c502ad2d5df83c6324b3813993691ca
Author: ajs6f <aj...@...>
Date: 2017-11-03T18:06:22Z
Better use of preexisting Pair type, refactoring, introducing DifferenceDatasetGraph
commit 8416e028a8a77e44f351609f4056f0a032cfedd3
Author: ajs6f <aj...@...>
Date: 2017-11-03T19:25:04Z
Adding method impls to DatasetLib
commit ccf011aca6d5aef9186a510a53f3b32bb28d2982
Author: ajs6f <aj...@...>
Date: 2017-11-04T17:59:16Z
Adding IntersectionDatasetGraph
commit 53861bb36212da86f7e88815bf7a29ffbd47bc7e
Author: ajs6f <aj...@...>
Date: 2017-11-04T18:02:07Z
Cleaner threading of Context through constructors and statics
commit e5cc28ba852153057b37b6881c4ec1f48013de2d
Author: ajs6f <aj...@...>
Date: 2017-11-04T18:11:46Z
Cleanup between DatasetLib and DatasetCollector
commit de74807555a2efce4081eee2028cdc4e7f92504e
Author: ajs6f <aj...@...>
Date: 2017-11-05T14:13:32Z
Correcting whitespace
commit 272b3c10687b9e643f869d62055edec68d202e53
Author: ajs6f <aj...@...>
Date: 2017-11-05T14:15:04Z
Correcting type param names
commit d4a22357ec714ac2dde218d242a36bf7f1df7d2d
Author: ajs6f <aj...@...>
Date: 2017-11-17T17:05:18Z
Further simplifying new Collector API
commit 5aa0e26ca4d3dd46f77a9c993006b5cba1a6b178
Author: ajs6f <aj...@...>
Date: 2017-11-19T21:37:54Z
Minor refactoring
commit 9fd57ea0593d755ceaa9ae2a78283a4940cd8d48
Author: ajs6f <aj...@...>
Date: 2017-12-15T20:55:05Z
More tests
commit bbf0fc6c6ac78111ffd8827ac03a80d6ff687a7e
Author: ajs6f <aj...@...>
Date: 2017-12-18T14:43:10Z
Removing extra file
commit b90c14364e3355f66ab063c797e8c476515b0ef3
Author: ajs6f <aj...@...>
Date: 2017-12-29T14:03:02Z
Further test coverage
commit d1e6b90d9cedfe063ae088d3ec595f1b629827e8
Author: ajs6f <aj...@...>
Date: 2017-12-29T14:29:40Z
Further test coverage
commit ca28e9f77bc8bd4f1e6de34bd6cb7cd8d793f324
Author: ajs6f <aj...@...>
Date: 2017-12-29T15:04:36Z
Fixing license headers, more tests
commit dcd5cd6337f063e832bb9f569eb27da4d167814a
Author: ajs6f <aj...@...>
Date: 2017-12-29T15:08:26Z
Adding license header
commit 973c6dec5111bcc8a617e0ba8a27bbafba0c759c
Author: ajs6f <aj...@...>
Date: 2017-12-29T15:13:04Z
Adding license headers
commit 5e62aff38ea56a76af047451b546153c3b3fc76a
Author: ajs6f <aj...@...>
Date: 2017-12-29T15:37:28Z
Adding more public API
commit 6162bc0928d453bc193a1b28e8d5626610927376
Author: ajs6f <aj...@...>
Date: 2017-12-29T15:53:27Z
Cleanup in TIM
commit 8e9881f52e1d8c9b0ce62883d6485e367689fd7e
Author: ajs6f <aj...@...>
Date: 2017-12-29T16:33:55Z
Linking tests into build suite
commit 01ed060f71e577d41edf2c8501e155eafbd1bb56
Author: ajs6f <aj...@...>
Date: 2017-12-29T17:54:56Z
Beginning tests of dataset collectors
commit 5fa9101f383b7f86103024a3aa218754dc00ab8f
Author: ajs6f <aj...@...>
Date: 2017-12-31T18:45:09Z
Tests for UnionDatasetCollector
commit 9562e71fb05d5c9910e43457a51f21eeeb188721
Author: ajs6f <aj...@...>
Date: 2017-12-31T18:45:47Z
Add license headers
commit 446a6093885920b0a516187cc53aa7e4158ad70c
Author: ajs6f <aj...@...>
Date: 2017-12-31T19:19:06Z
Test for TestIntersectionDatasetCollector now passes
commit 8f725b6064820930a7299cd16f06a0bb40029ed8
Author: ajs6f <aj...@...>
Date: 2017-12-31T19:26:34Z
Stronger test for TestIntersectionDatasetCollector
commit 51df4d8c9b386e82edf8a6de98f13cfc917e1f9e
Author: ajs6f <aj...@...>
Date: 2017-12-31T19:29:11Z
License header
commit 086f9aa4a8486587699faf99f144da54718f2cfa
Author: ajs6f <aj...@...>
Date: 2017-12-31T19:32:17Z
Locking down Collector API
commit 1f794c7f6e1d05ed3cddcd3760cee3f601677bb9
Author: ajs6f <aj...@...>
Date: 2018-01-01T01:28:26Z
Correct behavior on Transational::abort
commit e1c09188f720a7308f3b7b0b0c8c313eefcd9657
Author: ajs6f <aj...@...>
Date: 2018-01-01T01:34:29Z
Better importing of throwNoMutationAllowed()
----
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159252167
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/util/ViewDatasetGraph.java ---
@@ -0,0 +1,225 @@
+/*
+ * 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.sparql.util;
+
+import static java.util.Objects.requireNonNull;
+import static org.apache.jena.atlas.iterator.Iter.count;
+import static org.apache.jena.atlas.iterator.Iter.map;
+import static org.apache.jena.ext.com.google.common.collect.Iterators.concat;
+import static org.apache.jena.graph.Node.ANY;
+import static org.apache.jena.query.ReadWrite.READ;
+import static org.apache.jena.sparql.core.Quad.defaultGraphIRI;
+import static org.apache.jena.sparql.util.graph.GraphUtils.triples2quads;
+
+import java.util.Iterator;
+
+import org.apache.jena.atlas.lib.Pair;
+import org.apache.jena.graph.Graph;
+import org.apache.jena.graph.Node;
+import org.apache.jena.graph.compose.MultiUnion;
+import org.apache.jena.query.ReadWrite;
+import org.apache.jena.shared.Lock;
+import org.apache.jena.sparql.core.DatasetGraph;
+import org.apache.jena.sparql.core.Quad;
+
+public abstract class ViewDatasetGraph extends Pair.OfSameType<DatasetGraph> implements DatasetGraph {
+
--- End diff --
Oh, cool. Yes, the `View`ishness is really different than the `Pair`ishness. It's easy to imagine a view that isn't a pair or a pair that is more than a view. I'll go with `DyadicDatasetGraph`, since we already have, as you say, `Dyadic`.
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159252897
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/util/ViewDatasetGraph.java ---
@@ -0,0 +1,225 @@
+/*
+ * 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.sparql.util;
+
+import static java.util.Objects.requireNonNull;
+import static org.apache.jena.atlas.iterator.Iter.count;
+import static org.apache.jena.atlas.iterator.Iter.map;
+import static org.apache.jena.ext.com.google.common.collect.Iterators.concat;
+import static org.apache.jena.graph.Node.ANY;
+import static org.apache.jena.query.ReadWrite.READ;
+import static org.apache.jena.sparql.core.Quad.defaultGraphIRI;
+import static org.apache.jena.sparql.util.graph.GraphUtils.triples2quads;
+
+import java.util.Iterator;
+
+import org.apache.jena.atlas.lib.Pair;
+import org.apache.jena.graph.Graph;
+import org.apache.jena.graph.Node;
+import org.apache.jena.graph.compose.MultiUnion;
+import org.apache.jena.query.ReadWrite;
+import org.apache.jena.shared.Lock;
+import org.apache.jena.sparql.core.DatasetGraph;
+import org.apache.jena.sparql.core.Quad;
+
+public abstract class ViewDatasetGraph extends Pair.OfSameType<DatasetGraph> implements DatasetGraph {
+
+ private Context context;
+
+ private final Lock lock;
+
+ public ViewDatasetGraph(DatasetGraph left, DatasetGraph right, Context c) {
+ super(requireNonNull(left), requireNonNull(right));
+ this.context = requireNonNull(c);
+ this.lock = new PairLock(left.getLock(), right.getLock());
+ }
+
+ protected static void throwNoMutationAllowed() {
+ throw new UnsupportedOperationException("This view does not allow mutation!");
+ }
+
+ @Override
+ public void commit() {
+ throwNoMutationAllowed();
+ }
+
+ @Override
+ public void begin(ReadWrite readWrite) {
+ switch (readWrite) {
+ case WRITE:
+ throwNoMutationAllowed();
+ case READ:
+ forEach(dsg -> dsg.begin(READ));
--- End diff --
That's true. Okay, let's go with `synchronized` and if problems result, I'll deal with them, but they probably won't develop anyway. Most fears never give birth!
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159528932
--- Diff: jena-base/src/main/java/org/apache/jena/atlas/lib/Pair.java ---
@@ -37,6 +40,30 @@
public A car() { return a ; }
public B cdr() { return b ; }
+ public static class OfSameType<T> extends Pair<T, T> {
+
--- End diff --
Point 3 was the observation
```
public <S, X> S apply(BiFunction<X, X, S> f, Function<T, X> op) {
return f.apply(op.apply(a), op.apply(b));
}
```
could be a function that can acts on `Pair<X,X>`.
```
static public <S, X> S apply(Pair<X,X> pair, BiFunction<X, X, S> f, Function<T, X> op) {
return f.apply(op.apply(pair.car()), op.apply(pair.cdr()));
}
```
now whether `<S, X>` can meaningfully/usefully be `<S, X extends FOO>` and interface FOO defines the "to boolean" method,
I don't usually push generics very hard - I leave that to Scala.
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159250196
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/util/ViewDatasetGraph.java ---
@@ -0,0 +1,225 @@
+/*
+ * 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.sparql.util;
+
+import static java.util.Objects.requireNonNull;
+import static org.apache.jena.atlas.iterator.Iter.count;
+import static org.apache.jena.atlas.iterator.Iter.map;
+import static org.apache.jena.ext.com.google.common.collect.Iterators.concat;
+import static org.apache.jena.graph.Node.ANY;
+import static org.apache.jena.query.ReadWrite.READ;
+import static org.apache.jena.sparql.core.Quad.defaultGraphIRI;
+import static org.apache.jena.sparql.util.graph.GraphUtils.triples2quads;
+
+import java.util.Iterator;
+
+import org.apache.jena.atlas.lib.Pair;
+import org.apache.jena.graph.Graph;
+import org.apache.jena.graph.Node;
+import org.apache.jena.graph.compose.MultiUnion;
+import org.apache.jena.query.ReadWrite;
+import org.apache.jena.shared.Lock;
+import org.apache.jena.sparql.core.DatasetGraph;
+import org.apache.jena.sparql.core.Quad;
+
+public abstract class ViewDatasetGraph extends Pair.OfSameType<DatasetGraph> implements DatasetGraph {
+
--- End diff --
In core, the parent class is calls "Dyadic"
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159250915
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/util/Context.java ---
@@ -199,13 +193,12 @@ public long getLong(Symbol symbol, long defaultValue) {
}
}
- public void putAll(Context other) {
+ public Context putAll(Context other) {
if ( readonly )
--- End diff --
I think that was leftover from when I was still trying to find a way to take context from both underlying datasets. Would you rather "fluent-ify" all such methods?
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159251025
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/util/ViewDatasetGraph.java ---
@@ -0,0 +1,225 @@
+/*
+ * 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.sparql.util;
+
+import static java.util.Objects.requireNonNull;
+import static org.apache.jena.atlas.iterator.Iter.count;
+import static org.apache.jena.atlas.iterator.Iter.map;
+import static org.apache.jena.ext.com.google.common.collect.Iterators.concat;
+import static org.apache.jena.graph.Node.ANY;
+import static org.apache.jena.query.ReadWrite.READ;
+import static org.apache.jena.sparql.core.Quad.defaultGraphIRI;
+import static org.apache.jena.sparql.util.graph.GraphUtils.triples2quads;
+
+import java.util.Iterator;
+
+import org.apache.jena.atlas.lib.Pair;
+import org.apache.jena.graph.Graph;
+import org.apache.jena.graph.Node;
+import org.apache.jena.graph.compose.MultiUnion;
+import org.apache.jena.query.ReadWrite;
+import org.apache.jena.shared.Lock;
+import org.apache.jena.sparql.core.DatasetGraph;
+import org.apache.jena.sparql.core.Quad;
+
+public abstract class ViewDatasetGraph extends Pair.OfSameType<DatasetGraph> implements DatasetGraph {
+
--- End diff --
So `Pair.Dyadic`?
---
[GitHub] jena issue #337: JENA-1391: Dataset dyadic views, Model and Dataset Stream c...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the issue:
https://github.com/apache/jena/pull/337
I'll let this hang out until EOD today in case anyone else wants to opine. Happy new year everyone!
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159261113
--- Diff: jena-base/src/main/java/org/apache/jena/atlas/lib/Pair.java ---
@@ -37,6 +40,30 @@
public A car() { return a ; }
public B cdr() { return b ; }
+ public static class OfSameType<T> extends Pair<T, T> {
+
--- End diff --
To be clear, it has to do with `Pair` insofar as it subclasses `Pair`. The idea is to
1. restrict `Pair` to `Pair`s with both elements of the same type and
2. offer some convenience methods that only make sense when both halves of a pair are the same type.
I've broken it out as a separate class, but I'm still up for turning it into a couple of functions. I just don't want to do that until I understand your third point better.
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159885344
--- Diff: jena-base/src/main/java/org/apache/jena/atlas/lib/Pair.java ---
@@ -37,6 +40,30 @@
public A car() { return a ; }
public B cdr() { return b ; }
+ public static class OfSameType<T> extends Pair<T, T> {
+
--- End diff --
It's just a suggestion from looking at the code. With `OfSameType` a separate class, I'm not worried about the design. It was coupling what looks to me like specific behaviour to a class that exists primarily because java only returns one thing from a method call.
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159238850
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/util/Context.java ---
@@ -37,7 +35,7 @@
public static final Context emptyContext = new Context(true) ;
protected Map<Symbol, Object> context = new ConcurrentHashMap<>() ;
- protected List<Consumer<Symbol>> callbacks = new ArrayList<>() ;
+
--- End diff --
+1 to getting rid of callbacks! (wasn't a good idea at the time...)
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159253627
--- Diff: jena-arq/src/main/java/org/apache/jena/query/util/DatasetCollector.java ---
@@ -0,0 +1,125 @@
+/*
+ * 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.util;
--- End diff --
`o.a.j.sparql.util.compose` (so much of "sparql" has leaked anyway) because "compose" is used for the graph ones. Or we can have a big util.
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159255082
--- Diff: jena-arq/src/main/java/org/apache/jena/query/util/DatasetCollector.java ---
@@ -0,0 +1,125 @@
+/*
+ * 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.util;
--- End diff --
I'll use `compose` for now and if more stuff accumulates, let's do a big `util`.
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159528705
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/util/Context.java ---
@@ -199,13 +193,12 @@ public long getLong(Symbol symbol, long defaultValue) {
}
}
- public void putAll(Context other) {
+ public Context putAll(Context other) {
if ( readonly )
--- End diff --
Pulled out.
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159529387
--- Diff: jena-base/src/main/java/org/apache/jena/atlas/lib/Pair.java ---
@@ -37,6 +40,30 @@
public A car() { return a ; }
public B cdr() { return b ; }
+ public static class OfSameType<T> extends Pair<T, T> {
+
--- End diff --
Okay, I think I see where you are going-- let me try to work this out. Should be able to commit something before the weekend, if we don't get swamped by a blizzard. sigh!
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159251422
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/util/ViewDatasetGraph.java ---
@@ -0,0 +1,225 @@
+/*
+ * 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.sparql.util;
+
+import static java.util.Objects.requireNonNull;
+import static org.apache.jena.atlas.iterator.Iter.count;
+import static org.apache.jena.atlas.iterator.Iter.map;
+import static org.apache.jena.ext.com.google.common.collect.Iterators.concat;
+import static org.apache.jena.graph.Node.ANY;
+import static org.apache.jena.query.ReadWrite.READ;
+import static org.apache.jena.sparql.core.Quad.defaultGraphIRI;
+import static org.apache.jena.sparql.util.graph.GraphUtils.triples2quads;
+
+import java.util.Iterator;
+
+import org.apache.jena.atlas.lib.Pair;
+import org.apache.jena.graph.Graph;
+import org.apache.jena.graph.Node;
+import org.apache.jena.graph.compose.MultiUnion;
+import org.apache.jena.query.ReadWrite;
+import org.apache.jena.shared.Lock;
+import org.apache.jena.sparql.core.DatasetGraph;
+import org.apache.jena.sparql.core.Quad;
+
+public abstract class ViewDatasetGraph extends Pair.OfSameType<DatasetGraph> implements DatasetGraph {
+
+ private Context context;
+
+ private final Lock lock;
+
+ public ViewDatasetGraph(DatasetGraph left, DatasetGraph right, Context c) {
+ super(requireNonNull(left), requireNonNull(right));
+ this.context = requireNonNull(c);
+ this.lock = new PairLock(left.getLock(), right.getLock());
+ }
+
+ protected static void throwNoMutationAllowed() {
+ throw new UnsupportedOperationException("This view does not allow mutation!");
+ }
+
+ @Override
+ public void commit() {
+ throwNoMutationAllowed();
+ }
+
+ @Override
+ public void begin(ReadWrite readWrite) {
+ switch (readWrite) {
+ case WRITE:
+ throwNoMutationAllowed();
+ case READ:
+ forEach(dsg -> dsg.begin(READ));
--- End diff --
Yeah, I was wondering about that. I was trying to avoid `synchronized`. Do you think a new lock in `ViewDatasetGraph` would be more clear?
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159251106
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/util/ViewDatasetGraph.java ---
@@ -0,0 +1,225 @@
+/*
+ * 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.sparql.util;
+
+import static java.util.Objects.requireNonNull;
+import static org.apache.jena.atlas.iterator.Iter.count;
+import static org.apache.jena.atlas.iterator.Iter.map;
+import static org.apache.jena.ext.com.google.common.collect.Iterators.concat;
+import static org.apache.jena.graph.Node.ANY;
+import static org.apache.jena.query.ReadWrite.READ;
+import static org.apache.jena.sparql.core.Quad.defaultGraphIRI;
+import static org.apache.jena.sparql.util.graph.GraphUtils.triples2quads;
+
+import java.util.Iterator;
+
+import org.apache.jena.atlas.lib.Pair;
+import org.apache.jena.graph.Graph;
+import org.apache.jena.graph.Node;
+import org.apache.jena.graph.compose.MultiUnion;
+import org.apache.jena.query.ReadWrite;
+import org.apache.jena.shared.Lock;
+import org.apache.jena.sparql.core.DatasetGraph;
+import org.apache.jena.sparql.core.Quad;
+
+public abstract class ViewDatasetGraph extends Pair.OfSameType<DatasetGraph> implements DatasetGraph {
+
+ private Context context;
+
+ private final Lock lock;
+
+ public ViewDatasetGraph(DatasetGraph left, DatasetGraph right, Context c) {
+ super(requireNonNull(left), requireNonNull(right));
--- End diff --
Can do.
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159251828
--- Diff: jena-base/src/main/java/org/apache/jena/atlas/lib/Pair.java ---
@@ -37,6 +40,30 @@
public A car() { return a ; }
public B cdr() { return b ; }
+ public static class OfSameType<T> extends Pair<T, T> {
+
--- End diff --
I'm up for another design, as long as `ViewDatasetGraph` and `PairLock` stay reasonably clear and concise. A set of `static` methods could work fine here. I'm not sure what you mean by "even "X extends interface" for the "as boolean" nature?", can you explain that?
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159251782
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/util/ViewDatasetGraph.java ---
@@ -0,0 +1,225 @@
+/*
+ * 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.sparql.util;
+
+import static java.util.Objects.requireNonNull;
+import static org.apache.jena.atlas.iterator.Iter.count;
+import static org.apache.jena.atlas.iterator.Iter.map;
+import static org.apache.jena.ext.com.google.common.collect.Iterators.concat;
+import static org.apache.jena.graph.Node.ANY;
+import static org.apache.jena.query.ReadWrite.READ;
+import static org.apache.jena.sparql.core.Quad.defaultGraphIRI;
+import static org.apache.jena.sparql.util.graph.GraphUtils.triples2quads;
+
+import java.util.Iterator;
+
+import org.apache.jena.atlas.lib.Pair;
+import org.apache.jena.graph.Graph;
+import org.apache.jena.graph.Node;
+import org.apache.jena.graph.compose.MultiUnion;
+import org.apache.jena.query.ReadWrite;
+import org.apache.jena.shared.Lock;
+import org.apache.jena.sparql.core.DatasetGraph;
+import org.apache.jena.sparql.core.Quad;
+
+public abstract class ViewDatasetGraph extends Pair.OfSameType<DatasetGraph> implements DatasetGraph {
+
--- End diff --
It's a comment about "view" in the class name..
ViewDatasetGraph -> DyadicDatasetGraph, BiDatasetGraph, ...
?
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159241709
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/util/ViewDatasetGraph.java ---
@@ -0,0 +1,225 @@
+/*
+ * 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.sparql.util;
+
+import static java.util.Objects.requireNonNull;
+import static org.apache.jena.atlas.iterator.Iter.count;
+import static org.apache.jena.atlas.iterator.Iter.map;
+import static org.apache.jena.ext.com.google.common.collect.Iterators.concat;
+import static org.apache.jena.graph.Node.ANY;
+import static org.apache.jena.query.ReadWrite.READ;
+import static org.apache.jena.sparql.core.Quad.defaultGraphIRI;
+import static org.apache.jena.sparql.util.graph.GraphUtils.triples2quads;
+
+import java.util.Iterator;
+
+import org.apache.jena.atlas.lib.Pair;
+import org.apache.jena.graph.Graph;
+import org.apache.jena.graph.Node;
+import org.apache.jena.graph.compose.MultiUnion;
+import org.apache.jena.query.ReadWrite;
+import org.apache.jena.shared.Lock;
+import org.apache.jena.sparql.core.DatasetGraph;
+import org.apache.jena.sparql.core.Quad;
+
+public abstract class ViewDatasetGraph extends Pair.OfSameType<DatasetGraph> implements DatasetGraph {
+
+ private Context context;
+
+ private final Lock lock;
+
+ public ViewDatasetGraph(DatasetGraph left, DatasetGraph right, Context c) {
+ super(requireNonNull(left), requireNonNull(right));
+ this.context = requireNonNull(c);
+ this.lock = new PairLock(left.getLock(), right.getLock());
+ }
+
+ protected static void throwNoMutationAllowed() {
+ throw new UnsupportedOperationException("This view does not allow mutation!");
+ }
+
+ @Override
+ public void commit() {
+ throwNoMutationAllowed();
+ }
+
+ @Override
+ public void begin(ReadWrite readWrite) {
+ switch (readWrite) {
+ case WRITE:
+ throwNoMutationAllowed();
+ case READ:
+ forEach(dsg -> dsg.begin(READ));
--- End diff --
`synchronized public void begin` so that the `->dsg.begin` calls are atomic.
I don't other calls need sync'ing because once begun, outer locking should handle it.
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159250681
--- Diff: jena-arq/src/main/java/org/apache/jena/query/util/DatasetCollector.java ---
@@ -0,0 +1,125 @@
+/*
+ * 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.util;
--- End diff --
I'm 100% fine with not making a new one-- where might be a better place? Right in `org.apache.jena.query`?
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159527518
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/util/Context.java ---
@@ -199,13 +193,12 @@ public long getLong(Symbol symbol, long defaultValue) {
}
}
- public void putAll(Context other) {
+ public Context putAll(Context other) {
if ( readonly )
--- End diff --
Doesn't seem much point IMO. Changing the context, let alone changing several setting, is not a common thing to do.
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159238188
--- Diff: jena-base/src/main/java/org/apache/jena/atlas/lib/Pair.java ---
@@ -37,6 +40,30 @@
public A car() { return a ; }
public B cdr() { return b ; }
+ public static class OfSameType<T> extends Pair<T, T> {
+
--- End diff --
A few of thoughts about this class:
* it seems to exist for the specialized to the boolean behaviors - maybe the name should reflect that?
* maybe it should be a top level class because it is public and static (and not does not relate specially to Pair<>)?
* should it be functions taking a `Pair<X,X>` as argument? As a class, it's stateless other than the `Pair<X,X>`-ness even "X extends interface" for the "as boolean" nature?
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159527929
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/util/ViewDatasetGraph.java ---
@@ -0,0 +1,225 @@
+/*
+ * 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.sparql.util;
+
+import static java.util.Objects.requireNonNull;
+import static org.apache.jena.atlas.iterator.Iter.count;
+import static org.apache.jena.atlas.iterator.Iter.map;
+import static org.apache.jena.ext.com.google.common.collect.Iterators.concat;
+import static org.apache.jena.graph.Node.ANY;
+import static org.apache.jena.query.ReadWrite.READ;
+import static org.apache.jena.sparql.core.Quad.defaultGraphIRI;
+import static org.apache.jena.sparql.util.graph.GraphUtils.triples2quads;
+
+import java.util.Iterator;
+
+import org.apache.jena.atlas.lib.Pair;
+import org.apache.jena.graph.Graph;
+import org.apache.jena.graph.Node;
+import org.apache.jena.graph.compose.MultiUnion;
+import org.apache.jena.query.ReadWrite;
+import org.apache.jena.shared.Lock;
+import org.apache.jena.sparql.core.DatasetGraph;
+import org.apache.jena.sparql.core.Quad;
+
+public abstract class ViewDatasetGraph extends Pair.OfSameType<DatasetGraph> implements DatasetGraph {
+
+ private Context context;
+
+ private final Lock lock;
+
+ public ViewDatasetGraph(DatasetGraph left, DatasetGraph right, Context c) {
+ super(requireNonNull(left), requireNonNull(right));
+ this.context = requireNonNull(c);
+ this.lock = new PairLock(left.getLock(), right.getLock());
+ }
+
+ protected static void throwNoMutationAllowed() {
+ throw new UnsupportedOperationException("This view does not allow mutation!");
+ }
+
+ @Override
+ public void commit() {
+ throwNoMutationAllowed();
+ }
+
+ @Override
+ public void begin(ReadWrite readWrite) {
+ switch (readWrite) {
+ case WRITE:
+ throwNoMutationAllowed();
+ case READ:
+ forEach(dsg -> dsg.begin(READ));
--- End diff --
Done.
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159240514
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/util/ViewDatasetGraph.java ---
@@ -0,0 +1,225 @@
+/*
+ * 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.sparql.util;
+
+import static java.util.Objects.requireNonNull;
+import static org.apache.jena.atlas.iterator.Iter.count;
+import static org.apache.jena.atlas.iterator.Iter.map;
+import static org.apache.jena.ext.com.google.common.collect.Iterators.concat;
+import static org.apache.jena.graph.Node.ANY;
+import static org.apache.jena.query.ReadWrite.READ;
+import static org.apache.jena.sparql.core.Quad.defaultGraphIRI;
+import static org.apache.jena.sparql.util.graph.GraphUtils.triples2quads;
+
+import java.util.Iterator;
+
+import org.apache.jena.atlas.lib.Pair;
+import org.apache.jena.graph.Graph;
+import org.apache.jena.graph.Node;
+import org.apache.jena.graph.compose.MultiUnion;
+import org.apache.jena.query.ReadWrite;
+import org.apache.jena.shared.Lock;
+import org.apache.jena.sparql.core.DatasetGraph;
+import org.apache.jena.sparql.core.Quad;
+
+public abstract class ViewDatasetGraph extends Pair.OfSameType<DatasetGraph> implements DatasetGraph {
+
+ private Context context;
+
+ private final Lock lock;
+
+ public ViewDatasetGraph(DatasetGraph left, DatasetGraph right, Context c) {
+ super(requireNonNull(left), requireNonNull(right));
--- End diff --
public -> protected constructor
(implicitly it is because it's abstract)
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159239355
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/util/Context.java ---
@@ -199,13 +193,12 @@ public long getLong(Symbol symbol, long defaultValue) {
}
}
- public void putAll(Context other) {
+ public Context putAll(Context other) {
if ( readonly )
--- End diff --
Doing just one operation for fluent seems inconsistent. `Context` manipulation isn't (shouldn't) be that common an operation.
See also the quite recent `Context.mergeCopy`.
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159527781
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/util/ViewDatasetGraph.java ---
@@ -0,0 +1,225 @@
+/*
+ * 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.sparql.util;
+
+import static java.util.Objects.requireNonNull;
+import static org.apache.jena.atlas.iterator.Iter.count;
+import static org.apache.jena.atlas.iterator.Iter.map;
+import static org.apache.jena.ext.com.google.common.collect.Iterators.concat;
+import static org.apache.jena.graph.Node.ANY;
+import static org.apache.jena.query.ReadWrite.READ;
+import static org.apache.jena.sparql.core.Quad.defaultGraphIRI;
+import static org.apache.jena.sparql.util.graph.GraphUtils.triples2quads;
+
+import java.util.Iterator;
+
+import org.apache.jena.atlas.lib.Pair;
+import org.apache.jena.graph.Graph;
+import org.apache.jena.graph.Node;
+import org.apache.jena.graph.compose.MultiUnion;
+import org.apache.jena.query.ReadWrite;
+import org.apache.jena.shared.Lock;
+import org.apache.jena.sparql.core.DatasetGraph;
+import org.apache.jena.sparql.core.Quad;
+
+public abstract class ViewDatasetGraph extends Pair.OfSameType<DatasetGraph> implements DatasetGraph {
+
+ private Context context;
+
+ private final Lock lock;
+
+ public ViewDatasetGraph(DatasetGraph left, DatasetGraph right, Context c) {
+ super(requireNonNull(left), requireNonNull(right));
+ this.context = requireNonNull(c);
+ this.lock = new PairLock(left.getLock(), right.getLock());
+ }
+
+ protected static void throwNoMutationAllowed() {
+ throw new UnsupportedOperationException("This view does not allow mutation!");
+ }
+
+ @Override
+ public void commit() {
+ throwNoMutationAllowed();
+ }
+
+ @Override
+ public void begin(ReadWrite readWrite) {
+ switch (readWrite) {
+ case WRITE:
+ throwNoMutationAllowed();
+ case READ:
+ forEach(dsg -> dsg.begin(READ));
--- End diff --
Quite! And safety before performance .. otherwise it goes wrong IRL while the tests are fine.
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159238495
--- Diff: jena-arq/src/main/java/org/apache/jena/query/util/DatasetCollector.java ---
@@ -0,0 +1,125 @@
+/*
+ * 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.util;
--- End diff --
Are there are enough classes to justify a subpackage of "util"?
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159240339
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/util/ViewDatasetGraph.java ---
@@ -0,0 +1,225 @@
+/*
+ * 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.sparql.util;
+
+import static java.util.Objects.requireNonNull;
+import static org.apache.jena.atlas.iterator.Iter.count;
+import static org.apache.jena.atlas.iterator.Iter.map;
+import static org.apache.jena.ext.com.google.common.collect.Iterators.concat;
+import static org.apache.jena.graph.Node.ANY;
+import static org.apache.jena.query.ReadWrite.READ;
+import static org.apache.jena.sparql.core.Quad.defaultGraphIRI;
+import static org.apache.jena.sparql.util.graph.GraphUtils.triples2quads;
+
+import java.util.Iterator;
+
+import org.apache.jena.atlas.lib.Pair;
+import org.apache.jena.graph.Graph;
+import org.apache.jena.graph.Node;
+import org.apache.jena.graph.compose.MultiUnion;
+import org.apache.jena.query.ReadWrite;
+import org.apache.jena.shared.Lock;
+import org.apache.jena.sparql.core.DatasetGraph;
+import org.apache.jena.sparql.core.Quad;
+
+public abstract class ViewDatasetGraph extends Pair.OfSameType<DatasetGraph> implements DatasetGraph {
+
--- End diff --
Suggestion: a name that expresses the binary operation nature of this class.
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/jena/pull/337
---
[GitHub] jena pull request #337: JENA-1391: Dataset dyadic views, Model and Dataset S...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/337#discussion_r159252508
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/util/ViewDatasetGraph.java ---
@@ -0,0 +1,225 @@
+/*
+ * 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.sparql.util;
+
+import static java.util.Objects.requireNonNull;
+import static org.apache.jena.atlas.iterator.Iter.count;
+import static org.apache.jena.atlas.iterator.Iter.map;
+import static org.apache.jena.ext.com.google.common.collect.Iterators.concat;
+import static org.apache.jena.graph.Node.ANY;
+import static org.apache.jena.query.ReadWrite.READ;
+import static org.apache.jena.sparql.core.Quad.defaultGraphIRI;
+import static org.apache.jena.sparql.util.graph.GraphUtils.triples2quads;
+
+import java.util.Iterator;
+
+import org.apache.jena.atlas.lib.Pair;
+import org.apache.jena.graph.Graph;
+import org.apache.jena.graph.Node;
+import org.apache.jena.graph.compose.MultiUnion;
+import org.apache.jena.query.ReadWrite;
+import org.apache.jena.shared.Lock;
+import org.apache.jena.sparql.core.DatasetGraph;
+import org.apache.jena.sparql.core.Quad;
+
+public abstract class ViewDatasetGraph extends Pair.OfSameType<DatasetGraph> implements DatasetGraph {
+
+ private Context context;
+
+ private final Lock lock;
+
+ public ViewDatasetGraph(DatasetGraph left, DatasetGraph right, Context c) {
+ super(requireNonNull(left), requireNonNull(right));
+ this.context = requireNonNull(c);
+ this.lock = new PairLock(left.getLock(), right.getLock());
+ }
+
+ protected static void throwNoMutationAllowed() {
+ throw new UnsupportedOperationException("This view does not allow mutation!");
+ }
+
+ @Override
+ public void commit() {
+ throwNoMutationAllowed();
+ }
+
+ @Override
+ public void begin(ReadWrite readWrite) {
+ switch (readWrite) {
+ case WRITE:
+ throwNoMutationAllowed();
+ case READ:
+ forEach(dsg -> dsg.begin(READ));
--- End diff --
Ideally, one should not expose the method sync:
```
// field:
private Object lockObj = new Object() ;
// in begin(ReadWrite):
synchronized(lockObj) {
...
}
```
but otherwise "synchronized" is better than a `Lock` IMO because Java understands it.
---