Skip to content
Closed
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
65 changes: 65 additions & 0 deletions google/cloud/firestore_v1/base_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,71 @@ def find_nearest(
stages.FindNearest(field, vector, distance_measure, options)
)

def literals(self, *documents: str | Selectable) -> "_BasePipeline":

Choose a reason for hiding this comment

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

medium

The type hint for *documents is a bit too restrictive. It doesn't account for passing dict objects directly to represent documents, which is a common and valid use case. Please consider broadening it to include dict.

Suggested change
def literals(self, *documents: str | Selectable) -> "_BasePipeline":
def literals(self, *documents: dict | str | Selectable) -> "_BasePipeline":

"""
Returns documents from a fixed set of predefined document objects.

This stage is commonly used for testing other stages in isolation, though it can
also be used as inputs to join conditions.

Example:
>>> from google.cloud.firestore_v1.pipeline_expressions import Constant
>>> documents = [
... {"name": "joe", "age": 10},
... {"name": "bob", "age": 30},
... {"name": "alice", "age": 40}
... ]
>>> pipeline = client.pipeline()
... .literals(Constant.of(documents))
... .where(field("age").lessThan(35))

Output documents:
```json
[
{"name": "joe", "age": 10},
{"name": "bob", "age": 30}
]
```

Behavior:
The `literals(...)` stage can only be used as the first stage in a pipeline (or
sub-pipeline). The order of documents returned from the `literals` matches the
order in which they are defined.

While literal values are the most common, it is also possible to pass in
expressions, which will be evaluated and returned, making it possible to test
out different query / expression behavior without first needing to create some
test data.

For example, the following shows how to quickly test out the `length(...)`
function on some constant test sets:

Example:
>>> from google.cloud.firestore_v1.pipeline_expressions import Constant
>>> documents = [
... {"x": Constant.of("foo-bar-baz").char_length()},
... {"x": Constant.of("bar").char_length()}
... ]
>>> pipeline = client.pipeline().literals(Constant.of(documents))

Output documents:
```json
[
{"x": 11},
{"x": 3}
]
```

Args:
documents: A `str` or `Selectable` expression. If a `str`, it's
treated as a field path to an array of documents.
If a `Selectable`, it's usually a `Constant`
containing an array of documents (as dictionaries).
Comment on lines +331 to +335

Choose a reason for hiding this comment

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

medium

The docstring for the Args section is a bit misleading and incomplete.

  1. It refers to a single documents parameter, but the signature uses *documents (varargs).
  2. It claims that a str is "treated as a field path", which is incorrect. A raw string is treated as a string literal. To use a field path, one must wrap it in Field.of().
  3. It doesn't mention that dict objects can be passed to represent documents.

I suggest updating the docstring for clarity and correctness.

Suggested change
Args:
documents: A `str` or `Selectable` expression. If a `str`, it's
treated as a field path to an array of documents.
If a `Selectable`, it's usually a `Constant`
containing an array of documents (as dictionaries).
Args:
*documents: A sequence of documents or expressions. Each argument can be:
- A `dict` representing a document object.
- A `str` representing a string literal. To use a field path,
wrap the string with `Field.of()`.
- A `Selectable` expression. For example, a `Constant`
containing an array of documents (as dictionaries).

Returns:
A new Pipeline object with this stage appended to the stage list.
"""
return self._append(stages.Literals(*documents))

def replace_with(
self,
field: Selectable,
Expand Down
17 changes: 17 additions & 0 deletions google/cloud/firestore_v1/pipeline_stages.py
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,23 @@ def _pb_args(self):
return [Value(integer_value=self.limit)]


class Literals(Stage):
"""Returns documents from a fixed set of predefined document objects."""

def __init__(self, *documents: str | Selectable):

Choose a reason for hiding this comment

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

medium

The type hint for *documents is too restrictive. The implementation correctly handles dict objects (via encode_value), but the type hint doesn't reflect this. This is inconsistent with the usage shown in system tests. Please consider broadening the type hint to include dict.

Suggested change
def __init__(self, *documents: str | Selectable):
def __init__(self, *documents: dict | str | Selectable):

super().__init__("literals")
self.documents = documents

def _pb_args(self):
args = []
for doc in self.documents:
if hasattr(doc, "_to_pb"):
args.append(doc._to_pb())
else:
args.append(encode_value(doc))
return args


class Offset(Stage):
"""Skips a specified number of documents."""

Expand Down
22 changes: 21 additions & 1 deletion tests/system/pipeline_e2e/general.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -684,4 +684,24 @@ tests:
- args:
- fieldReferenceValue: awards
- stringValue: full_replace
name: replace_with
name: replace_with
- description: literals
pipeline:
- Literals:
- title: "The Hitchhiker's Guide to the Galaxy"
author: "Douglas Adams"
assert_results:
- title: "The Hitchhiker's Guide to the Galaxy"
author: "Douglas Adams"
assert_proto:
pipeline:
stages:
- args:
- mapValue:
fields:
author:
stringValue: "Douglas Adams"
title:
stringValue: "The Hitchhiker's Guide to the Galaxy"
name: literals

1 change: 1 addition & 0 deletions tests/unit/v1/test_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ def test_pipeline_execute_stream_equivalence():
),
("replace_with", ("name",), stages.ReplaceWith),
("replace_with", (Field.of("n"),), stages.ReplaceWith),
("literals", (Field.of("a"),), stages.Literals),
("sort", (Field.of("n").descending(),), stages.Sort),
("sort", (Field.of("n").descending(), Field.of("m").ascending()), stages.Sort),
("sample", (10,), stages.Sample),
Expand Down
29 changes: 29 additions & 0 deletions tests/unit/v1/test_pipeline_stages.py
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,35 @@ def test_to_pb(self):
assert len(result.options) == 0


class TestLiterals:
def _make_one(self, *args, **kwargs):
return stages.Literals(*args, **kwargs)

def test_ctor(self):
val1 = Constant.of({"a": 1})
val2 = Constant.of({"b": 2})
instance = self._make_one(val1, val2)
assert instance.documents == (val1, val2)
assert instance.name == "literals"

def test_repr(self):
val1 = Constant.of({"a": 1})
instance = self._make_one(val1)
repr_str = repr(instance)
assert repr_str == "Literals(documents=(Constant.of({'a': 1}),))"

def test_to_pb(self):
val1 = Constant.of({"a": 1})
val2 = Constant.of({"b": 2})
instance = self._make_one(val1, val2)
result = instance._to_pb()
assert result.name == "literals"
assert len(result.args) == 2
assert result.args[0].map_value.fields["a"].integer_value == 1
assert result.args[1].map_value.fields["b"].integer_value == 2
assert len(result.options) == 0


class TestOffset:
def _make_one(self, *args, **kwargs):
return stages.Offset(*args, **kwargs)
Expand Down
Loading