Introduces a new writer API #680

zslayton · 2023-11-14T21:19:17Z

Introduces a friendlier writer API that will supplant the existing IonWriter. For now, it's called the LazyWriter for symmetry with the LazyReader. However, I plan to revisit that naming scheme once these implementations are ready to become the primary (read: only) implementations in the library.

The new API uses traits instead of methods to handle writing different data types, which results less noisy code. For example, this code:

let mut writer = LazyRawTextWriter_1_0::new(&mut buffer);
writer
    .write(1)?
    .write(false)?
    .write(3f32)?
    .write("foo")?
    .write("bar".as_symbol_ref())?
    .write(Timestamp::with_ymd(2023, 11, 9).build()?)?
    .write([0xE0u8, 0x01, 0x00, 0xEA])?
    .write([1, 2, 3])?
    .flush()?;

produces this output:

1
false
3e0
"foo"
bar
2023-11-09T
{{4AEA6g==}}
[1, 2, 3]

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

zslayton

I suggest reading the files in this order:

write_as_ion.rs
annotate.rs
encoder/mod.rs
text/raw_text_writer.rs

and the rest in any order.

🗺️ PR tour

zslayton · 2023-11-14T21:38:02Z

src/lazy/encoder/annotate.rs

+    fn annotate<'a, A: AsRawSymbolTokenRef>(
+        &'a self,
+        annotations: &'a [A],
+    ) -> Annotated<'a, Self, A>;


🗺️ This extension method allows values being passed into writer.write(...) to add annotations to themselves:

writer.write(30_000.annotate("lbs_bananas"))?;

zslayton · 2023-11-14T21:41:34Z

src/lazy/encoder/annotate.rs

+    fn write_as_ion<'a, W: Write + 'a, E: LazyEncoder<W>, V: AnnotatedValueWriter<'a, W, E>>(
+        &self,
+        annotations_writer: V,
+    ) -> IonResult<()> {
+        let value_writer = annotations_writer.write_annotations(self.annotations.iter())?;
+        self.value.write_as_ion_value(value_writer)
+    }


🗺️ AnnotatedValueWriter is a staged writer. You call a method to either write annotations or no_annotations() and upon success it returns a ValueWriter that can be used to write the value itself.

zslayton · 2023-11-14T21:44:19Z

src/lazy/encoder/mod.rs

+        SymbolType: AsRawSymbolTokenRef,
+        IterType: Iterator<Item = SymbolType> + Clone,


🗺️ An earlier version of this used much more generics; I started spelling out what each generic was (e.g. SymbolType instead of A). I think I kind of like it?

Yes, please continue this. It gets hard to keep track sometimes even when there's only a few.

zslayton · 2023-11-14T21:46:51Z

src/lazy/encoder/mod.rs

+    }
+}
+
+impl<'a, W: Write> ValueWriter<'a, W, TextEncoding_1_0> for TextValueWriter_1_0<'a, W> {


🗺️ The bodies of these methods are either copy/pasted from the existing raw text writer or invoke one of its methods.

zslayton · 2023-11-14T21:49:50Z

src/lazy/encoder/mod.rs

+impl<'a, W: Write> Drop for TextListWriter_1_0<'a, W> {
+    fn drop(&mut self) {
+        // If the user didn't call `end`, the closing delimiter was not written to output.
+        // It's too late to call it here because we can't return a Result.
+        if !self.has_been_closed {
+            panic!("List writer was dropped without calling `end()`.");
+        }
+    }
+}


🗺️ This is my compromise in the absence of linear types. We can't close() in Drop because there's no way for the application to handle a failure there. However, that also means we cannot force the writer to close automatically. This at least makes contract violations noisy -- it's not possible to forget to to close the container and keep going.

zslayton · 2023-11-14T21:52:52Z

src/lazy/encoder/mod.rs

+    fn write_clob<A: AsRef<[u8]>>(self, value: A) -> IonResult<()>;
+    fn write_blob<A: AsRef<[u8]>>(self, value: A) -> IonResult<()>;
+    fn list_writer(self) -> IonResult<E::ListWriter<'a>>;
+    fn sexp_writer(self) -> IonResult<E::StructWriter<'a>>;


TODO for me: change the return type to SExpWriter.

zslayton · 2023-11-14T21:53:13Z

src/lazy/encoder/mod.rs

+    type SExpWriter<'a> = ();
+    type StructWriter<'a> = ();
+    type EExpressionWriter<'a> = ();


TODO for me: mark these as TODO.

zslayton · 2023-11-14T21:55:26Z

src/lazy/encoder/mod.rs

+                .write("bar".as_symbol_ref())?
+                .write(Timestamp::with_ymd(2023, 11, 9).build()?)?
+                .write([0xE0u8, 0x01, 0x00, 0xEA])?
+                .write([1, 2, 3])?;


🗺️ There are two ways to write a list: if you have a type that represents the list and your type implements WriteAsIon[Value], you can pass it to .write(...) as is done here. For a streaming list writer that can emit heterogeneous values, you can use ListWriter.

If WriteAsIon[Value] is implemented, it would be implemented using ListWriter.

zslayton · 2023-11-14T21:56:43Z

src/lazy/encoder/write_as_ion.rs

+    }
+}
+
+// ===== WriteAsIonValue implementations for common types =====


🗺️ There are common types not covered here, like Vec<T: WriteAsIon> and several more integer types. I'd like to punt on those.

zslayton · 2023-11-14T21:58:53Z

src/types/null.rs

+// This type allows us to define custom behavior for `null` via trait implementations.
+// For an example, see the `WriteAsIonValue` trait.
+#[derive(Debug, PartialEq, Copy, Clone)]
+pub struct Null(pub IonType);


🗺️ We have similar wrapper types in the Element API for Blob(Bytes), Clob(Bytes), List(Sequence) and SExp(Sequence).

zslayton · 2023-11-14T22:06:37Z

src/text/raw_text_writer.rs

🗺️ Changes in this file make its inner workings crate-visible so the LazyRawTextWriter_1_0 can re-use them. Down the road, this type will be removed and I'll consolidate the logic.

popematt

Looks pretty good. I do wonder whether these traits will work for the binary writer, but that doesn't necessarily need to be solved right now.

src/lazy/encoder/mod.rs

popematt · 2023-11-14T22:01:00Z

src/lazy/encoder/mod.rs

+        SymbolType: AsRawSymbolTokenRef,
+        IterType: Iterator<Item = SymbolType> + Clone,


Yes, please continue this. It gets hard to keep track sometimes even when there's only a few.

src/lazy/encoder/annotate.rs

src/lazy/encoder/mod.rs

src/lazy/encoder/write_as_ion.rs

zslayton added 4 commits November 10, 2023 09:28

writer wip

860a8a2

Merge branch 'main' into lazy-writer

29168a0

Working scalars, lists, annotations

6921e99

Fixed import

42c60af

zslayton commented Nov 14, 2023

View reviewed changes

fixed a couple of TODOs

4c4d262

zslayton commented Nov 14, 2023

View reviewed changes

zslayton requested review from popematt and desaikd November 14, 2023 22:07

popematt reviewed Nov 15, 2023

View reviewed changes

zslayton mentioned this pull request Nov 16, 2023

Revisit annotate extension method name #681

Open

zslayton requested a review from popematt November 17, 2023 14:13

popematt approved these changes Nov 17, 2023

View reviewed changes

Merge branch 'main' into lazy-writer

3c6a65f

zslayton merged commit 6113bc7 into main Nov 17, 2023
20 checks passed

zslayton deleted the lazy-writer branch November 17, 2023 17:56

popematt mentioned this pull request Nov 29, 2023

adds inital support for code generation amazon-ion/ion-cli#61

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduces a new writer API #680

Introduces a new writer API #680

zslayton commented Nov 14, 2023

zslayton left a comment

zslayton Nov 14, 2023

zslayton Nov 14, 2023

zslayton Nov 14, 2023

popematt Nov 14, 2023

zslayton Nov 14, 2023

zslayton Nov 14, 2023

zslayton Nov 14, 2023

zslayton Nov 14, 2023

zslayton Nov 14, 2023

zslayton Nov 14, 2023

zslayton Nov 14, 2023

zslayton Nov 14, 2023 •

edited

Loading

popematt left a comment

popematt Nov 14, 2023

		SymbolType: AsRawSymbolTokenRef,
		IterType: Iterator<Item = SymbolType> + Clone,

Introduces a new writer API #680

Introduces a new writer API #680

Conversation

zslayton commented Nov 14, 2023

zslayton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zslayton Nov 14, 2023 • edited Loading

Choose a reason for hiding this comment

popematt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zslayton Nov 14, 2023 •

edited

Loading