[
https://issues.apache.org/jira/browse/TINKERPOP-3193?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18023157#comment-18023157
]
ASF GitHub Bot commented on TINKERPOP-3193:
-------------------------------------------
Cole-Greer commented on code in PR #3215:
URL: https://github.com/apache/tinkerpop/pull/3215#discussion_r2382927510
##########
docs/src/upgrade/release-3.8.x.asciidoc:
##########
@@ -888,6 +874,41 @@ pinned variable is one which must remain bound to the
current value. By paramete
variable, as it can be substituted in the optimized traversal without any loss
of meaning, however the variable "count"
will be pinned as the optimized traversal is not valid for other values of
"count".
+===== Decouple internal step state from `Configuring`/`Parameterizing`
interfaces
+
+Previously `AddVertexStep`, `AddVertexStartStep`, `AddEdgeStep`,
`AddEdgeStartStep`, and `AddPropertyStep` stored their
+internal state (id, label, properties, from/to vertices...) in a `Parameters`
object which was exposed via the
+`Configuring` and `Parameterizing` interfaces. These interfaces are primarily
intended to support with()-modulation, and
+thus these steps had weird unintended behaviors when combined with
with()-modulation:
+
+[source,text]
+----
+gremlin> g.addV().with("name", "cole").valueMap()
+==>[name:[cole]]
+----
+
+This internal step state has now been decoupled from the `Configuring` and
`Parameterizing` interfaces, which are now
+exclusively used for the purposes of with()-modulation. All accesses of these
steps internal data must now go through
Review Comment:
I will update with an explicit list of steps. It's primarily a concern for
strategy authors and providers as this is changing how data such as properties
is accessed from steps such as addV. The only users who will be impacted are
those doing silly things like using `with()` to set properties, which has never
been a documented or supported behaviour.
> Decouple non with()-related configs from Configuring/Parameterizing interfaces
> ------------------------------------------------------------------------------
>
> Key: TINKERPOP-3193
> URL: https://issues.apache.org/jira/browse/TINKERPOP-3193
> Project: TinkerPop
> Issue Type: Improvement
> Components: process
> Affects Versions: 3.7.4
> Reporter: Cole Greer
> Priority: Major
> Labels: breaking
>
> Currently, the Configuring and Parameterizing interfaces serve two distinct
> purposes. The most notable purpose is that these interfaces enable
> with()-modulation for a step. Additionally, there are a handful of steps such
> as AddVertexStep, AddEdgeStep, and AddPropertyStep which utilize Parameters
> to store their internal state. The dual purpose of these interfaces leads to
> strange behaviours such as the ability to set properties via with modulators:
> {code:java}
> gremlin> g.addV().with("name", "cole").valueMap()
> ==>[name:[cole]]
> {code}
> As part of the ongoing work leading to 3.8.0, we’ve extracted StepContracts
> (interfaces) from many steps including the ones listed above.
> These contracts expose new methods to interact with a step’s contents in a
> controlled manner instead of by directly accessing the
> Parameters (getElementId(), getLabel(), addProperty()…). The presence of
> these new interfaces creates an opportunity to escape the
> internal-state management role of Parameterizing, and change
> Configuring/Parameterizing to be purely focused on with()-modulation.
> We should isolate any state not related to with()-modulation from
> Configuring/Parameterizing interfaces. TinkerPop should never call the
> configure() method on a step except in response to a with() modulator, and
> that the Parameters returned by getParameters() should only contain
> configurations originating from with() modulators. Classes such as
> AddVertexStep may still leverage a second private Parameters objects to store
> its internal state, however this should not be publicly exposed at all. Any
> accesses and mutations of these steps should be forced to use the new
> contract interfaces instead.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)