Another thought: an option is to run the iterator immediately and return a disconnected iterator.

Maybe this is good enough for 4.0.0?

It means all the permissions concerns are dealt with , and exception generated, at the point of calling the permissions graph. PermTripleFilter throws an exception on processing.

    Andy

---- Sketch
It does "contains" separately as the one obvious case of cost when running to completion early.


static class SecuredGraphEx extends GraphBase {
    private final Graph dataGraph;
    SecuredGraphEx(Graph graph) {
        this.dataGraph = graph;
    }

    @Override
    protected boolean graphBaseContains(Triple triplePattern) {
        ExtendedIterator<Triple> eIter = findFilter(triplePattern);
        return safeFindAction(eIter, ExtendedIterator::hasNext);
    }

    @Override
    protected ExtendedIterator<Triple>
               graphBaseFind(Triple triplePattern) {
        ExtendedIterator<Triple> eIter = findFilter(triplePattern);
        return materialize(eIter);
    }

    private ExtendedIterator<Triple> findFilter(Triple triplePatter) {
        ExtendedIterator<Triple> baseIter = dataGraph.find();
        // PermTripleFilter throws AuthenticationRequiredException
        Predicate<Triple> thePermTripleFilter = (t)->true;
        return baseIter.filterKeep(thePermTripleFilter);
    }

    // Execute an action on an iterator, ensuring close is called.
private static <T,X> X safeFindAction(ExtendedIterator<T> eIter, Function<ExtendedIterator<T>, X> action) {
        try {
        return action.apply(eIter);
        } finally {
        eIter.close();
        }
    }

    // Materialize an iterator, ensuring close is called.
private static <T> ExtendedIterator<T> materialize(ExtendedIterator<T> eIter) {
        return safeFindAction(eIter, it->{
        List<T> x = it.toList();
        return WrappedIterator.create(x.iterator());
        });
    }


On 11/03/2021 22:51, Claude Warren wrote:
My thought in all this was that the try with resources is now standard in
Java so we should try to support it.

Perhaps it makes sense to do the change in V4.
We can clean up our code so that there are no warnings.
Since the issues are warnings only, it should be manageable for our users.

Claude

On Thu, Mar 11, 2021 at 10:48 AM Andy Seaborne <a...@apache.org> wrote:

I'm not clear which problem we are discussing:

1/ Ensuring inside jena-permissions that iterators are properly closed.

2/ Ensuring use code using jena-permissions (i.e
SecuredGraph/SecuredModel by name) closes iterators properly.

3/ Ensuring app code that uses the Graph or Model interface (and so may
be Secured* but it does not know) closes iterators properly.

If (3) isn't any change going to impact existing use code?

What's more, it does not seem to work

On 10/03/2021 21:56, Claude Warren wrote:
I think that if the solution was any simpler it would have bit me.

Change ClosableIterator<T> to extend AutoCloseable as well as Iterator.
mark ClosableIterator.close() with an @Override annotation and done.

While Autoclosable throws an exception the ClosableIterator does not so
the
try with resources works.

I tried this and then changed
FinderUtils.contains() to be coded as

@Override
          public boolean contains(TriplePattern pattern) {
              try (ClosableIterator<Triple> it = find(pattern))
              {
              return it.hasNext();
              }
          }

and the IDE (Eclipse) does not complain.  So I think it will just work.

because writing

public boolean contains(TriplePattern pattern) {
        ExtendedIterator<Triple> it = find(pattern);
        try {
           return it.hasNext();
        } finally { it.close(); }
}

is accident-prone?


I changed ClosableIterator as you describe and got hundreds of warnings
from the codebase, and it will happen to user code.

1/
A warning happens on code that is currently correctly written old style as

      ExtendedIterator<> iter =
      try {
        ...
      } finally { iter.close(); }

Eclipse "quick-fix" does not do a good job on this.

The other "old style" code is where the iterator runs the the end
(exceptions aside).

2/
Sometimes there isn't a warning!

Warning: as expected:
      public static void main(String...a) throws FileNotFoundException {
          InputStream in = new FileInputStream("");
      }

No warning:  !!!!
      static ExtendedIterator<Triple> foo() { return null; }
      public static void main(String...a) {
          ExtendedIterator<Triple> iter = foo();
      }

Warning:
      static ExtendedIterator<Triple> foo() { return null; }
      public static void main(String...a) {
          ExtendedIterator<Triple> iter = foo();
          iter.close();
      }

Weird.
That somewhat negates the utility.

      Andy



Reply via email to