Skip to content

Commit

Permalink
TINKERPOP-3072 Let Next() throw if no result present
Browse files Browse the repository at this point in the history
This makes the behavior of Gremlin.Net consistent with that of other
GLVs in cases where `Next()` is called on a traversal where no result
is present to be returned. It also ensures consistent behavior for .NET
across .NET runtimes as users already got this exception if they were on
.NET 8.

`InvalidOperationException` seems appropriate here as that is also the
exception thrown by .NET internally in these cases or for example by
LINQ's `First()` method.
  • Loading branch information
FlorianHockmann committed Apr 24, 2024
1 parent 16441ee commit 226fa73
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
[[release-3-6-8]]
=== TinkerPop 3.6.8 (NOT OFFICIALLY RELEASED YET)
* Gremlin.Net now throws an exception when `Next()` is called but no element is available instead of returning `null`.
[[release-3-6-7]]
=== TinkerPop 3.6.7 (April 8, 2024)
Expand Down
37 changes: 37 additions & 0 deletions docs/src/upgrade/release-3.6.x.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,43 @@ complete list of all the modifications that are part of this release.
=== Upgrading for Users
==== Gremlin.Net: Changed behavior for `Next()` when no element available
Gremlin.Net used to return `null` when no element is available and `Next()` is called.
This default behavior changed when Gremlin.Net was used on .NET 8 which came with a change in enumerators to
throw an exception instead of returning `null`.
Throwing an exception in these cases is however consistent with the other GLVs like Gremlin-Java and Gremlin-Python
which already throw an exception.
That is why we have decided to let Gremlin.Net also throw an exception for these cases, irrespective of .NET runtime
version.
An example to better illustrate this change:
[source,csharp]
----
var result = g.V().Has("name", "doesnotexist").Next();
// result before / on .NET 7 and earlier: result = null
// result now (and before if .NET 8 was used): InvalidOperationException thrown
----
Users who relied on this specific behavior of Gremlin.Net are advised to check first via `HasNext()` whether an element
is available before calling `Next()` if they don't know whether the element should be present or not:
[source,csharp]
----
var traversal = g.V().Has("name", "mayexistornot");
if (traversal.HasNext())
{
var result = traversal.Next();
}
----
Note that calling `Next()` will not send the traversal to the server again for evaluation.
The result was already fetched when `HasNext()` was executed and can therefore be directly returned.
See: link:https://issues.apache.org/jira/browse/TINKERPOP-3072[TINKERPOP-3072]
=== Upgrading for Providers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public abstract class DefaultTraversal<S, E> : ITraversal<S, E>
/// Gets the <see cref="Traversal.Bytecode" /> representation of this traversal.
/// </summary>
public Bytecode Bytecode { get; protected set; }

/// <summary>
/// Determines if this traversal was spawned anonymously or not.
/// </summary>
Expand Down Expand Up @@ -76,11 +76,11 @@ public void Dispose()
/// <inheritdoc />
public bool MoveNext()
{
var more = MoveNextInternal();
var more = MoveNextInternal();
_fetchedNext = false;
return more;
}

private bool MoveNextInternal()
{
if (_fetchedNext) return _nextAvailable;
Expand All @@ -90,7 +90,7 @@ private bool MoveNextInternal()
_nextAvailable = TraverserEnumerator.MoveNext();
}
if (!_nextAvailable) return false;

var currentTraverser = TraverserEnumerator.Current;
if (currentTraverser?.Bulk >= 1)
{
Expand Down Expand Up @@ -128,12 +128,16 @@ private TReturn GetCurrent<TReturn>()

private object GetCurrent()
{
if (!_nextAvailable)
{
throw new InvalidOperationException("No result available");
}
// Use dynamic to object to prevent runtime dynamic conversion evaluation
return TraverserEnumerator.Current?.Object;
}

/// <summary>
/// Gets the value, converting to the expected type when necessary and supported.
/// Gets the value, converting to the expected type when necessary and supported.
/// </summary>
private static object GetValue(Type type, object value)
{
Expand Down Expand Up @@ -188,7 +192,7 @@ private async Task ApplyStrategiesAsync(CancellationToken cancellationToken)
/// <inheritdoc />
public bool HasNext()
{
var more = MoveNextInternal();
var more = MoveNextInternal();
_fetchedNext = true;
return more;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

#endregion

using System;
using System.Linq;
using System.Collections.Generic;
using System.Threading;
Expand Down Expand Up @@ -87,6 +88,15 @@ public void g_VX1X_Next()
Assert.Equal(1, vertex.Id);
}

[Fact]
public void g_V_HasXnotpresentX_Next_NotPresent()
{
var connection = _connectionFactory.CreateRemoteConnection();
var g = AnonymousTraversalSource.Traversal().WithRemote(connection);

Assert.Throws<InvalidOperationException>(() => g.V().Has("notpresent").Next());
}

[Fact]
public void g_VX1X_NextTraverser()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public void ShouldReturnAvailableTraverserObjWhenNextIsCalled(object traverserOb

Assert.Equal(traverserObj, actualObj);

Assert.Null(traversal.Next());
Assert.False(traversal.HasNext());
}
[Theory]
[InlineData(1)]
Expand Down Expand Up @@ -96,20 +96,38 @@ public void ShouldDrainAllTraversersWhenIterateIsCalled()

var drainedTraversal = traversal.Iterate();

Assert.Null(drainedTraversal.Next());
Assert.False(drainedTraversal.HasNext());
}

[Fact]
public void ShouldReturnNullWhenNextIsCalledAndNoTraverserIsAvailable()
public void ShouldThrowWhenNextIsCalledAndNoTraverserIsAvailable()
{
var expectedFirstObj = 1;
var traversal = new TestTraversal(new List<object> {expectedFirstObj});
var traversal = new TestTraversal([expectedFirstObj]);

var actualFirstObj = traversal.Next();
var actualSecondObj = traversal.Next();

Assert.Equal(expectedFirstObj, actualFirstObj);
Assert.Null(actualSecondObj);
Assert.Throws<InvalidOperationException>(() => traversal.Next());
}

[Fact]
public void ShouldThrowWhenNextIsCalledAndNotEnoughTraversersAvailableAvailable()
{
var firstObj = 1;
var traversal = new TestTraversal([firstObj]);

Assert.Throws<InvalidOperationException>(() => traversal.Next(2).ToList());
}

[Fact]
public void ShouldReturnEmptyListWhenToListIsCalledAndNoTraverserIsAvailable()
{
var traversal = new TestTraversal(new List<object>());

var list = traversal.ToList();

Assert.Empty(list);
}

[Fact]
Expand Down

0 comments on commit 226fa73

Please sign in to comment.