Re: frt - r23998 - abiword/branches/gsoc2008ooxml/plugins/openxml/exp/xp

On Fri, May 23, 2008 at 7:03 PM, <cvs@abisource.com> wrote:
>
> Author: frt
> Date: 2008-05-24 01:03:39 +0200 (Sat, 24 May 2008)
> New Revision: 23998
>
> Modified:
> abiword/branches/gsoc2008ooxml/plugins/openxml/exp/xp/ie_exp_OpenXML.cpp
> abiword/branches/gsoc2008ooxml/plugins/openxml/exp/xp/ie_exp_OpenXML.h
> abiword/branches/gsoc2008ooxml/plugins/openxml/exp/xp/ie_exp_OpenXML_Sniffer.cpp
> Log:
> OpenXML Exporter is extended to create an empty valid .docx file

Hi Firat,

Nice start :). I've made some comments on parts of your code that
could be problematic, especially in circumstances such as running out
of hard drive space (something like:
http://bugzilla.abisource.com/show_bug.cgi?id=11541).

>
> Modified: abiword/branches/gsoc2008ooxml/plugins/openxml/exp/xp/ie_exp_OpenXML.cpp
> ===================================================================
> --- abiword/branches/gsoc2008ooxml/plugins/openxml/exp/xp/ie_exp_OpenXML.cpp 2008-05-23 15:39:14 UTC (rev 23997)
> +++ abiword/branches/gsoc2008ooxml/plugins/openxml/exp/xp/ie_exp_OpenXML.cpp 2008-05-23 23:03:39 UTC (rev 23998)
> @@ -24,15 +24,6 @@
> // Class definition include
> #include <ie_exp_OpenXML.h>
>
> -// Internal includes
> -
> -// AbiWord includes
> -#include <ut_types.h>
> -#include <ut_assert.h>
> -
> -// External includes
> -
> -
> /**
> * Constructor
> */
> @@ -56,7 +47,37 @@
> * Export the OOXML document here
> */
> UT_Error IE_Exp_OpenXML::_writeDocument (){
> - UT_DEBUGMSG(("FRT: Writing the OOXML file\n"));
> +
> + UT_DEBUGMSG(("FRT: Writing the OOXML file started\n"));

Use xxx_UT_DEBUGMSG() for less important debug messages to prevent the
important ones from being drowned out (this applies to most of the
debug messages in this commit).

> +
> + //get the file pointer (target file)
> + GsfOutput* sink = getFp();
> +
> + if(sink != NULL){

If you check for the error case first (sink == NULL), you can keep
nested code to a minimum to make it easier to read and maintain (i.e.
since you remove the need for an else block altogether).

> + UT_DEBUGMSG(("FRT: Zip root file created successfully\n"));
> +
> + //create a zip file root based on the target
> + GsfOutfile* root = gsf_outfile_zip_new(sink, NULL);

Check the return value of gsf_outfile_zip_new() before using root.

> +
> + //unreference sink since we don't need it directly anymore
> + //instead we will be using "root"
> + g_object_unref (G_OBJECT (sink));
> +
> +
> + writeContentTypes(root);
> + writeRelations(root);
> + writeMainPart(root);

Actively use return values in these functions (i.e. don't just return
UT_OK) and check them to make sure the files were successfully
written.

> +
> + //finally close the file
> + gsf_output_close((GsfOutput*)root);

Be pedantic and check the return value of gsf_output_close() too.

> + }
> + else{
> + UT_DEBUGMSG(("FRT: Error, Zip root file couldn't be created\n"));
> + return UT_IE_COULDNOTWRITE;
> + }
> +
> + UT_DEBUGMSG(("FRT: Writing the OOXML file completed\n"));
> +
> return UT_OK;
> }
>
> @@ -68,3 +89,112 @@
> UT_DEBUGMSG(("FRT: Cleaning up the OOXML exporter\n"));
> }
>
> +/**
> + * Writes the [Content_Types].xml file which describes the contents of the package
> + */
> +UT_Error IE_Exp_OpenXML::writeContentTypes(GsfOutfile* root){
> +
> + UT_DEBUGMSG(("FRT: Writing the [Content_Types].xml file started\n"));
> +
> + //[Content_Types].xml file
> + GsfOutput* contentTypesFile = gsf_outfile_new_child(root, "[Content_Types].xml", FALSE);

Check return value (CRV from here on out).

> +
> + //write the basic file contents
> + //we only have .rels and .xml file types in the simple basis file
> + //TODO: extend this for other file types as needed
> + writeXmlHeader(contentTypesFile);

CRV.

> +
> + gsf_output_puts(contentTypesFile, "<Types xmlns=\"http://schemas.openxmlformats.org/package/2006/content-types\">");
> + gsf_output_puts(contentTypesFile, "<Default Extension=\"rels\" ContentType=\"application/vnd.openxmlformats-package.relationships+xml\"/>");
> + gsf_output_puts(contentTypesFile, "<Default Extension=\"xml\" ContentType=\"application/xml\"/>");
> + gsf_output_puts(contentTypesFile, "<Override PartName=\"/word/document.xml\" ContentType=\"application/vnd.openxmlformats-officedocument.wordprocessingml.document.main+xml\"/>");
> + gsf_output_puts(contentTypesFile, "</Types>");
> +

CRVs (it will be easier to check if you combine these calls into one).

> +
> + //close the file
> + gsf_output_close(contentTypesFile);

CRV.

> +
> + UT_DEBUGMSG(("FRT: Writing the [Content_Types].xml file completed\n"));
> +
> + return UT_OK;
> +}
> +
> +
> +/**
> + * Writes the relationships for the files within the package
> + * Outputs the _rels folder and _rels/.rels file which defines the package relations.
> + */
> +UT_Error IE_Exp_OpenXML::writeRelations(GsfOutfile* root){
> +
> + UT_DEBUGMSG(("FRT: Writing the _rels/.rels file started\n"));
> + //_rels directory
> + GsfOutfile* relsDir = GSF_OUTFILE(gsf_outfile_new_child(root, "_rels", TRUE));

CRV.

> + //_rels/.rel file
> + GsfOutput* relFile = gsf_outfile_new_child(relsDir, ".rels", FALSE);

CRV.

> +
> + //write the basic file content
> + //TODO: extend this for other relations as needed
> + writeXmlHeader(relFile);

CRV.

> +
> + gsf_output_puts(relFile, "<Relationships xmlns=\"http://schemas.openxmlformats.org/package/2006/relationships\">");
> + gsf_output_puts(relFile, "<Relationship Id=\"rId1\" ");
> + gsf_output_puts(relFile, "Type=\"http://schemas.openxmlformats.org/officeDocument/2006/relationships/officeDocument\" ");
> + gsf_output_puts(relFile, "Target=\"word/document.xml\"/>");
> + gsf_output_puts(relFile, "</Relationships>");
> +

CRVs (it will be easier to check if you combine these calls into one).

> +
> + //close the .rels file and rels directory
> + gsf_output_close(relFile);
> + gsf_output_close((GsfOutput*)relsDir);

CRVs.

> +
> + UT_DEBUGMSG(("FRT: Writing the _rels/.rels file completed\n"));
> +
> + return UT_OK;
> +}
> +
> +/**
> + * Writes the main part of the document to word/document.xml file.
> + */
> +UT_Error IE_Exp_OpenXML::writeMainPart(GsfOutfile* root){
> +
> + UT_DEBUGMSG(("FRT: Writing the word/document.xml file started\n"));
> +
> + //word directory
> + GsfOutfile* wordDir = GSF_OUTFILE(gsf_outfile_new_child(root, "word", TRUE));

CRV.

> + //word/document.xml file
> + GsfOutput* documentFile = gsf_outfile_new_child(wordDir, "document.xml", FALSE);

CRV.

> +
> + //write the basic content to document.xml
> + writeXmlHeader(documentFile);

CRV.

> +
> + gsf_output_puts(documentFile, "<w:wordDocument xmlns:r=\"http://schemas.openxmlformats.org/officeDocument/2006/relationships\" ");
> + gsf_output_puts(documentFile, "xmlns:v=\"urn:schemas-microsoft-com:vml\" ");
> + gsf_output_puts(documentFile, "xmlns:w=\"http://schemas.openxmlformats.org/wordprocessingml/2006/main\">");
> + gsf_output_puts(documentFile, "<w:body>");

CRVs (it will be easier to check if you combine these calls into one).

> +
> + //TODO: Write the document body here
> +
> + gsf_output_puts(documentFile, "</w:body>");
> + gsf_output_puts(documentFile, "</w:wordDocument>");

CRVs (it will be easier to check if you combine these calls into one).

> +
> + //close the document.xml file and word directory
> + gsf_output_close(documentFile);
> + gsf_output_close((GsfOutput*)wordDir);

CRVs.

> +
> + UT_DEBUGMSG(("FRT: Writing the word/document.xml file completed\n"));
> +
> + return UT_OK;
> +}
> +
> +/**
> + * Write the simple xml header to the file
> + * This function should be called before anything written to file
> + */
> +UT_Error IE_Exp_OpenXML::writeXmlHeader(GsfOutput* file){
> +
> + gsf_output_puts(file, "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>");

CRV.

> +
> + UT_DEBUGMSG(("FRT: Writing xml header completed\n"));
> +
> + return UT_OK;
> +}
Received on Sat May 24 03:15:51 2008