From 2535cab7aa7b6795b8039ac99e28dd4213d0c64b Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Wed, 10 Dec 2025 11:38:09 -0800 Subject: [PATCH] make spv writer return an error indication --- rust/pspp/src/output/drivers/spv.rs | 4 +- rust/pspp/src/spv/write.rs | 138 ++++++++++++++++------------ 2 files changed, 80 insertions(+), 62 deletions(-) diff --git a/rust/pspp/src/output/drivers/spv.rs b/rust/pspp/src/output/drivers/spv.rs index d4be110591..a0bdd1d720 100644 --- a/rust/pspp/src/output/drivers/spv.rs +++ b/rust/pspp/src/output/drivers/spv.rs @@ -73,7 +73,7 @@ where impl SpvDriver { pub fn new(config: &SpvConfig) -> std::io::Result { - let mut writer = Writer::for_writer(File::create(&config.file)?); + let mut writer = Writer::for_writer(File::create(&config.file)?).unwrap(); if let Some(page_setup) = &config.page_setup { writer = writer.with_page_setup(page_setup.clone()); } @@ -87,7 +87,7 @@ where { pub fn for_writer(writer: W) -> Self { Self { - writer: Writer::for_writer(writer), + writer: Writer::for_writer(writer).unwrap(), } } } diff --git a/rust/pspp/src/spv/write.rs b/rust/pspp/src/spv/write.rs index 93ae30b3d0..f83b4b38ac 100644 --- a/rust/pspp/src/spv/write.rs +++ b/rust/pspp/src/spv/write.rs @@ -22,12 +22,13 @@ use std::{ use binrw::{BinWrite, Endian}; use chrono::Utc; +use displaydoc::Display; use enum_map::EnumMap; use quick_xml::{ ElementWriter, Writer as XmlWriter, events::{BytesText, attributes::Attribute}, }; -use zip::{ZipWriter, result::ZipResult, write::SimpleFileOptions}; +use zip::{ZipWriter, result::ZipError, write::SimpleFileOptions}; use crate::{ format::{Format, Type}, @@ -46,6 +47,19 @@ use crate::{ util::ToSmallString, }; +/// An error writing an SPV file. +#[derive(Debug, Display, thiserror::Error)] +pub enum Error { + /// {0} + ZipError(#[from] ZipError), + + /// {0} + IoError(#[from] std::io::Error), + + /// {0} + BinrwError(#[from] binrw::Error), +} + /// SPSS viewer (SPV) file writer. pub struct Writer where @@ -64,19 +78,17 @@ where { /// Creates a new `Writer` to write an SPV file to underlying stream /// `writer`. - pub fn for_writer(writer: W) -> Self { + pub fn for_writer(writer: W) -> Result { let mut writer = ZipWriter::new(writer); - writer - .start_file("META-INF/MANIFEST.MF", SimpleFileOptions::default()) - .unwrap(); - writer.write_all("allowPivoting=true".as_bytes()).unwrap(); - Self { + writer.start_file("META-INF/MANIFEST.MF", SimpleFileOptions::default())?; + writer.write_all("allowPivoting=true".as_bytes())?; + Ok(Self { writer, needs_page_break: false, next_table_id: 1, next_heading_id: 1, page_setup: None, - } + }) } /// Returns this `Writer` with `page_setup` set up to be written with the @@ -100,11 +112,11 @@ where /// Closes the underlying file and returns the inner writer and the final /// I/O result. - pub fn close(mut self) -> ZipResult { + pub fn close(mut self) -> Result { self.writer .start_file("META-INF/MANIFEST.MF", SimpleFileOptions::default())?; write!(&mut self.writer, "allowPivoting=true")?; - self.writer.finish() + Ok(self.writer.finish()?) } fn page_break_before(&mut self) -> bool { @@ -118,7 +130,8 @@ where item: &Item, pivot_table: &PivotTable, structure: &mut XmlWriter, - ) where + ) -> Result<(), Error> + where X: Write, { let table_id = self.next_table_id; @@ -126,13 +139,12 @@ where let mut content = Vec::new(); let mut cursor = Cursor::new(&mut content); - pivot_table.write_le(&mut cursor).unwrap(); + pivot_table.write_le(&mut cursor)?; let table_name = format!("{table_id:011}_lightTableData.bin"); self.writer - .start_file(&table_name, SimpleFileOptions::default()) - .unwrap(); // XXX - self.writer.write_all(&content).unwrap(); // XXX + .start_file(&table_name, SimpleFileOptions::default())?; // XXX + self.writer.write_all(&content)?; // XXX self.container(structure, item, "vtb:table", |element| { element @@ -158,23 +170,29 @@ where Ok(()) })?; Ok(()) - }) - .unwrap(); - }); + })?; + Ok(()) + })?; + Ok(()) } - fn write_text(&mut self, item: &Item, text: &Text, structure: &mut XmlWriter) + fn write_text( + &mut self, + item: &Item, + text: &Text, + structure: &mut XmlWriter, + ) -> Result<(), Error> where X: Write, { self.container(structure, item, "vtx:text", |w| { w.with_attribute(("type", text.type_.as_xml_str())) - .write_text_content(BytesText::new(&text.content.display(()).to_string())) - .unwrap(); - }); + .write_text_content(BytesText::new(&text.content.display(()).to_string()))?; + Ok(()) + }) } - fn write_item(&mut self, item: &Item, structure: &mut XmlWriter) + fn write_item(&mut self, item: &Item, structure: &mut XmlWriter) -> Result<(), Error> where X: Write, { @@ -195,17 +213,18 @@ where w.create_element("label") .write_text_content(BytesText::new(&item.label()))?; for child in &children.0 { - self.write_item(&child, w); + self.write_item(&child, w).map_err(std::io::Error::other)?; } Ok(()) - }) - .unwrap(); + })?; + Ok(()) } Details::Message(diagnostic) => { self.write_text(item, &Text::from(diagnostic.as_ref()), structure) } Details::PageBreak => { self.needs_page_break = true; + Ok(()) } Details::Table(pivot_table) => self.write_table(item, pivot_table, structure), Details::Text(text) => self.write_text(item, text, structure), @@ -218,9 +237,10 @@ where item: &Item, inner_elem: &str, closure: F, - ) where + ) -> Result<(), Error> + where X: Write, - F: FnOnce(ElementWriter), + F: FnOnce(ElementWriter) -> Result<(), Error>, { writer .create_element("container") @@ -232,16 +252,15 @@ where .write_inner_content(|w| { let mut element = w .create_element("label") - .write_text_content(BytesText::new(&item.label())) - .unwrap() + .write_text_content(BytesText::new(&item.label()))? .create_element(inner_elem); if let Some(command_name) = &item.command_name { element = element.with_attribute(("commandName", command_name.as_str())); }; closure(element); Ok(()) - }) - .unwrap(); + })?; + Ok(()) } } @@ -540,10 +559,10 @@ impl Writer where W: Write + Seek, { - pub fn write(&mut self, item: &Item) { + pub fn write(&mut self, item: &Item) -> Result<(), Error> { if item.details.is_page_break() { self.needs_page_break = true; - return; + return Ok(()); } let mut headings = XmlWriter::new(Cursor::new(Vec::new())); @@ -569,35 +588,32 @@ where )) .with_attribute(("xmlns:vtx", "http://xml.spss.com/spss/viewer/viewer-text")) .with_attribute(("xmlns:vtb", "http://xml.spss.com/spss/viewer/viewer-table")); - element - .write_inner_content(|w| { - w.create_element("label") - .write_text_content(BytesText::new("Output"))?; - if let Some(page_setup) = self.page_setup.take() { - write_page_setup(&page_setup, w)?; - } - self.write_item(item, w); - Ok(()) - }) - .unwrap(); + element.write_inner_content(|w| { + w.create_element("label") + .write_text_content(BytesText::new("Output"))?; + if let Some(page_setup) = self.page_setup.take() { + write_page_setup(&page_setup, w)?; + } + self.write_item(item, w); + Ok(()) + })?; let headings = headings.into_inner().into_inner(); let heading_id = self.next_heading_id; self.next_heading_id += 1; - self.writer - .start_file( - format!( - "outputViewer{heading_id:010}{}.xml", - if item.details.is_heading() { - "_heading" - } else { - "" - } - ), - SimpleFileOptions::default(), - ) - .unwrap(); // XXX - self.writer.write_all(&headings).unwrap(); // XXX + self.writer.start_file( + format!( + "outputViewer{heading_id:010}{}.xml", + if item.details.is_heading() { + "_heading" + } else { + "" + } + ), + SimpleFileOptions::default(), + )?; // XXX + self.writer.write_all(&headings)?; // XXX + Ok(()) } } @@ -725,7 +741,9 @@ impl Leaf { 0u8, 0u8, 2u32, - data_indexes.next().unwrap() as u32, + data_indexes + .next() + .expect("should have as many data indexes as leaves") as u32, 0u32, ) .write_le(writer) -- 2.30.2