Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new method IQueryResult.stream() #312

Merged
merged 2 commits into from
Sep 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,12 @@
</message_arguments>
</filter>
</resource>
<resource path="src/org/eclipse/equinox/p2/query/IQueryResult.java" type="org.eclipse.equinox.p2.query.IQueryResult">
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IQueryResult is not marked with @noimplement/@noextend but I wonder if clients are really supposed to create own implementations?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tycho has some implementations of IQueryResult, I don't see why one should not allow this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

People do what they're not supposed to do, so if they're allowed, one can assume that someone will to it. 😱

What about a default method that does toSet().stream() so that by default there is always an implementation?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK with custom queryable this makes sense.
Tycho even already has a stream() method in its own implementation, but it only needs to be made public when depending on the next Eclipse Release, so I think this is fine.
https://github.com/eclipse-tycho/tycho/blob/master/p2-maven-plugin/src/main/java/org/eclipse/tycho/p2maven/ListQueryable.java#L80-L83

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IQueryable already implements Iterable so using the iterator for default implementations seems a better choice to prevent creation of an intermediate set.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about a default method that does toSet().stream() so that by default there is always an implementation?

That's possible. But as Christoph suggested it could use the Spliterator of the IQueryResult, like it is currently done in QueryResult.

Iterable itself unfortunately does not provide a stream() method (if it would this PR would be pointless).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about a default method that does toSet().stream() so that by default there is always an implementation?

That's possible. But as Christoph suggested it could use the Spliterator of the IQueryResult, like it is currently done in QueryResult.

Done.

<filter id="404000815">
<message_arguments>
<message_argument value="org.eclipse.equinox.p2.query.IQueryResult"/>
<message_argument value="stream()"/>
</message_arguments>
</filter>
</resource>
</component>
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Bundle-ManifestVersion: 2
Bundle-Name: %pluginName
Bundle-SymbolicName: org.eclipse.equinox.p2.metadata;singleton:=true
Bundle-Version: 2.7.100.qualifier
Bundle-Version: 2.8.0.qualifier

Check warning on line 5 in bundles/org.eclipse.equinox.p2.metadata/META-INF/MANIFEST.MF

View workflow job for this annotation

GitHub Actions / build / Verify Linux

The minor version should be the same for version 2.8.0, since no new APIs have been added since version 2.7.100

Check warning on line 5 in bundles/org.eclipse.equinox.p2.metadata/META-INF/MANIFEST.MF

View workflow job for this annotation

GitHub Actions / build / Verify Windows

The minor version should be the same for version 2.8.0, since no new APIs have been added since version 2.7.100

Check warning on line 5 in bundles/org.eclipse.equinox.p2.metadata/META-INF/MANIFEST.MF

View workflow job for this annotation

GitHub Actions / build / Verify MacOS

The minor version should be the same for version 2.8.0, since no new APIs have been added since version 2.7.100
Bundle-Vendor: %providerName
Bundle-Localization: plugin
Export-Package: org.eclipse.equinox.internal.p2.metadata;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,31 +14,40 @@
package org.eclipse.equinox.internal.p2.metadata.expression;

import java.lang.reflect.Array;
import java.util.*;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.equinox.p2.metadata.index.IIndexProvider;
import org.eclipse.equinox.p2.query.IQuery;
import org.eclipse.equinox.p2.query.IQueryResult;

/**
* A result optimized for dealing with iterators returned from
* expression evaluation.
* A result optimized for dealing with iterators returned from expression
* evaluation.
*/
public class QueryResult<T> implements IQueryResult<T> {

private final IRepeatableIterator<T> iterator;
private boolean firstUse = true;

/**
* Create an QueryResult based on the given iterator. The <code>oneShot</code> parameter
* can be set to <code>true</code> if the returned instance is expected to be perused
* only once. This will allow some optimizations since the result of the iteration doesn't
* need to be copied in preparation for a second iteration.
* Create an QueryResult based on the given iterator. The <code>oneShot</code>
* parameter can be set to <code>true</code> if the returned instance is
* expected to be perused only once. This will allow some optimizations since
* the result of the iteration doesn't need to be copied in preparation for a
* second iteration.
*
* @param iterator The iterator to use as the result iterator.
*/
public QueryResult(Iterator<T> iterator) {
this.iterator = (iterator instanceof IRepeatableIterator<?>) ? (IRepeatableIterator<T>) iterator : RepeatableIterator.create(iterator);
this.iterator = (iterator instanceof IRepeatableIterator<T> repeatable) //
? repeatable
: RepeatableIterator.create(iterator);
}

public QueryResult(Collection<T> collection) {
Expand All @@ -63,31 +72,26 @@ public Iterator<T> iterator() {
@SuppressWarnings("unchecked")
public T[] toArray(Class<T> clazz) {
Object provider = iterator.getIteratorProvider();
if (provider.getClass().isArray())
if (provider.getClass().isArray()) {
return (T[]) provider;

Collection<T> c = toUnmodifiableSet();
return c.toArray((T[]) Array.newInstance(clazz, c.size()));
}
return toArray(toUnmodifiableSet(), clazz);
}

@Override
@SuppressWarnings("unchecked")
public Set<T> toSet() {
Object provider = iterator.getIteratorProvider();
if (provider instanceof Collection<?>)
return new HashSet<>((Collection<T>) provider);
if (provider instanceof IIndexProvider<?>)
return iteratorToSet(((IIndexProvider<T>) provider).everything());
if (provider.getClass().isArray()) {
if (provider instanceof Collection collection) {
return new HashSet<>(collection);
} else if (provider instanceof IIndexProvider indexProvider) {
return iteratorToSet(indexProvider.everything());
} else if (provider.getClass().isArray()) {
T[] elems = (T[]) provider;
int idx = elems.length;
HashSet<T> copy = new HashSet<>(idx);
while (--idx >= 0)
copy.add(elems[idx]);
return copy;
return new HashSet<>(Arrays.asList(elems));
} else if (provider instanceof Map<?, ?> map) {
return new HashSet<>((Set<T>) map.entrySet());
}
if (provider instanceof Map<?, ?>)
return new HashSet<>((Set<T>) ((Map<?, ?>) provider).entrySet());
return iteratorToSet(iterator());
}

Expand All @@ -100,17 +104,25 @@ public IQueryResult<T> query(IQuery<T> query, IProgressMonitor monitor) {
@SuppressWarnings("unchecked")
public Set<T> toUnmodifiableSet() {
Object provider = iterator.getIteratorProvider();
if (provider instanceof Set<?>)
return Collections.unmodifiableSet((Set<T>) provider);
if (provider instanceof Map<?, ?>)
return Collections.unmodifiableSet((Set<T>) ((Map<?, ?>) provider).entrySet());
if (provider instanceof Set set) {
return Collections.unmodifiableSet(set);
} else if (provider instanceof Map map) {
return Collections.unmodifiableSet((Set<T>) map.entrySet());
}
return toSet();
}

private Set<T> iteratorToSet(Iterator<T> iter) {
HashSet<T> set = new HashSet<>();
while (iter.hasNext())
Set<T> set = new HashSet<>();
while (iter.hasNext()) {
set.add(iter.next());
}
return set;
}

public static <T> T[] toArray(Collection<T> collection, Class<T> clazz) {
@SuppressWarnings("unchecked")
T[] arr = (T[]) Array.newInstance(clazz, collection == null ? 0 : collection.size());
return collection != null ? collection.toArray(arr) : arr;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,19 @@
*******************************************************************************/
package org.eclipse.equinox.p2.query;

import java.lang.reflect.Array;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Set;
import java.util.stream.Stream;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.equinox.internal.p2.metadata.expression.QueryResult;

/**
* This class allows to adapt java collections to a p2 a query result and as such something queryable
* This class allows to adapt java collections to a p2 a query result and as
* such something queryable
*
* @since 2.0
*/
public class CollectionResult<T> implements IQueryResult<T> {
Expand All @@ -49,12 +52,7 @@ public Iterator<T> iterator() {

@Override
public T[] toArray(Class<T> clazz) {
int size = collection.size();
@SuppressWarnings("unchecked")
T[] result = (T[]) Array.newInstance(clazz, size);
if (size != 0)
collection.toArray(result);
return result;
return QueryResult.toArray(collection, clazz);
}

@Override
Expand All @@ -71,4 +69,9 @@ public Set<T> toUnmodifiableSet() {
public String toString() {
return collection.toString();
}

@Override
public Stream<T> stream() {
return collection.stream();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,16 @@
*******************************************************************************/
package org.eclipse.equinox.p2.query;

import java.lang.reflect.Array;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Set;
import java.util.stream.Stream;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.equinox.internal.p2.metadata.Messages;
import org.eclipse.equinox.internal.p2.metadata.expression.QueryResult;

/**
* A collector is a generic visitor that collects objects passed to it, and can
Expand Down Expand Up @@ -51,13 +52,6 @@ public static final <T> Collector<T> emptyCollector() {
return (Collector<T>) EMPTY_COLLECTOR;
}

/**
* Creates a new collector.
*/
public Collector() {
super();
}

/**
* Accepts an object.
* <p>
Expand Down Expand Up @@ -96,8 +90,9 @@ public void addAll(IQueryResult<T> queryResult) {
* @return the collection being used to collect results.
*/
protected Collection<T> getCollection() {
if (collected == null)
if (collected == null) {
collected = new HashSet<>();
}
return collected;
}

Expand All @@ -119,7 +114,7 @@ public boolean isEmpty() {
*/
@Override
public Iterator<T> iterator() {
return collected == null ? Collections.<T>emptyList().iterator() : collected.iterator();
return collected == null ? Collections.emptyIterator() : collected.iterator();
}

/**
Expand All @@ -140,18 +135,13 @@ public int size() {
*/
@Override
public T[] toArray(Class<T> clazz) {
int size = collected == null ? 0 : collected.size();
@SuppressWarnings("unchecked")
T[] result = (T[]) Array.newInstance(clazz, size);
if (size != 0)
collected.toArray(result);
return result;
return QueryResult.toArray(collected, clazz);
}

/**
* Returns a copy of the collected objects.
*
* @return An unmodifiable collection of the collected objects
* @return An modifiable collection of the collected objects
*/
@Override
public Set<T> toSet() {
Expand All @@ -164,8 +154,9 @@ public Set<T> toSet() {
@Override
public IQueryResult<T> query(IQuery<T> query, IProgressMonitor monitor) {
IQueryResult<T> result;
if (monitor == null)
if (monitor == null) {
monitor = new NullProgressMonitor();
}
try {
monitor.beginTask(Messages.performing_subquery, 1);
result = query.perform(iterator());
Expand All @@ -188,4 +179,9 @@ public Set<T> toUnmodifiableSet() {
}
return Collections.unmodifiableSet(collected);
}

@Override
public Stream<T> stream() {
return collected == null ? Stream.empty() : collected.stream();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,29 +14,33 @@
******************************************************************************/
package org.eclipse.equinox.p2.query;

import java.util.Collection;
import java.util.Iterator;
import java.util.Set;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;

/**
* An IQueryResult represents the results of a query.
* An IQueryResult represents the results of a query.
*
* @since 2.0
*/
public interface IQueryResult<T> extends IQueryable<T>, Iterable<T> {
/**
* Returns whether this QueryResult is empty.
* @return <code>true</code> if this QueryResult has accepted any results,
* and <code>false</code> otherwise.
* Returns whether this QueryResult is empty.
*
* @return <code>true</code> if this QueryResult has accepted any results, and
* <code>false</code> otherwise.
*/
public boolean isEmpty();
boolean isEmpty();

/**
* Returns an iterator on the collected objects.
*
* @return an iterator of the collected objects.
*/
@Override
public Iterator<T> iterator();
Iterator<T> iterator();

/**
* Returns the collected objects as an array
Expand All @@ -46,18 +50,34 @@ public interface IQueryResult<T> extends IQueryable<T>, Iterable<T> {
* @throws ArrayStoreException the runtime type of the specified array is
* not a super-type of the runtime type of every collected object
*/
public T[] toArray(Class<T> clazz);
T[] toArray(Class<T> clazz);

/**
* Creates a new Set copy with the contents of this query result. The
* copy can be altered without any side effects on its origin.
* Creates a new Set copy with the contents of this query result. The copy can
* be altered without any side effects on its origin.
*
* @return A detached copy of the result.
*/
public Set<T> toSet();
Set<T> toSet();

/**
* Returns a Set backed by this query result. The set is immutable.
*
* @return A Set backed by this query result.
*/
public Set<T> toUnmodifiableSet();
Set<T> toUnmodifiableSet();

/**
* Returns a sequential {@code Stream} of the collected objects.
*
* @implSpec The default implementation creates a sequential {@code Stream} from
* this query-results {@code Spliterator}. Implementations backed by a
* {@code Collection} should override this method and call
* {@link Collection#stream()}.
* @since 2.8
*/
default Stream<T> stream() {
return StreamSupport.stream(spliterator(), false);
}

}
Loading