[Libreoffice-commits] core.git: 2 commits - desktop/source ucb/source

Stephan Bergmann sbergman at redhat.com
Mon May 27 09:15:55 PDT 2013


 desktop/source/deployment/registry/package/dp_package.cxx |    9 +-
 ucb/source/ucp/package/pkgcontent.cxx                     |   30 +------
 ucb/source/ucp/package/pkgprovider.cxx                    |   54 ++++----------
 ucb/source/ucp/package/pkgprovider.hxx                    |    2 
 4 files changed, 29 insertions(+), 66 deletions(-)

New commits:
commit 6b562710cb8051e921726899246b421b197d49f0
Author: Stephan Bergmann <sbergman at redhat.com>
Date:   Mon May 27 17:51:43 2013 +0200

    Let package_ucp::ContentProvider::createPackage throw exceptions on failure
    
    ...instead of returning a null XHierarchicalNameAccess.  Otherwise, UCB's
    globalTransfer from a vnd.sun.star.package URL that denotes a file that is not a
    zip file to a file URL folder (i.e., to extract the zip content) silently
    succeeds but just creates an empty file in the target folder.  That, in turn,
    causes "unopkg add foo.oxt", where foo.oxt is a file that is not a zip file, to
    silently succeed and add an "empty" extension.
    
    This change is somewhat bold in that it changes createPackage from "can return
    empty reference" to "never returns empty reference, but can throw
    RuntimeException."  Especially, it considers "empty name" as a (silent)
    violation of its contract with its caller now.  I hope this does not affect any
    legitimate scenarios---at least, it does not break a "make check" here.  (In
    turn, the two package_ucp::Content::getPackage overloads change to never return
    a null reference, either.  And I changed the parameters of createPackage, seeing
    that all call-sites pass in some PackageUri's getPackage()/getParam() pair.)
    
    Change-Id: I0eab5f8059dfedefc7030da38da528ba21ea8d79

diff --git a/ucb/source/ucp/package/pkgcontent.cxx b/ucb/source/ucp/package/pkgcontent.cxx
index d19cccb..be6cdbe 100644
--- a/ucb/source/ucp/package/pkgcontent.cxx
+++ b/ucb/source/ucp/package/pkgcontent.cxx
@@ -217,7 +217,7 @@ Content* Content::create(
 
     uno::Reference< container::XHierarchicalNameAccess > xPackage;
 
-    xPackage = pProvider->createPackage( aURI.getPackage(), aURI.getParam() );
+    xPackage = pProvider->createPackage( aURI );
 
     uno::Reference< ucb::XContentIdentifier > xId
         = new ::ucbhelper::ContentIdentifier( aURI.getUri() );
@@ -2217,12 +2217,12 @@ uno::Reference< container::XHierarchicalNameAccess > Content::getPackage(
     if ( rURI.getPackage() == m_aUri.getPackage() )
     {
         if ( !m_xPackage.is() )
-            m_xPackage = m_pProvider->createPackage( m_aUri.getPackage(), m_aUri.getParam() );
+            m_xPackage = m_pProvider->createPackage( m_aUri );
 
         return m_xPackage;
     }
 
-    return m_pProvider->createPackage( rURI.getPackage(), rURI.getParam() );
+    return m_pProvider->createPackage( rURI );
 }
 
 //=========================================================================
@@ -2238,10 +2238,7 @@ sal_Bool Content::hasData(
             const PackageUri& rURI,
             uno::Reference< container::XHierarchicalNameAccess > & rxPackage )
 {
-    rxPackage = pProvider->createPackage( rURI.getPackage(), rURI.getParam() );
-    if ( !rxPackage.is() )
-        return sal_False;
-
+    rxPackage = pProvider->createPackage( rURI );
     return rxPackage->hasByHierarchicalName( rURI.getPath() );
 }
 
@@ -2254,9 +2251,6 @@ sal_Bool Content::hasData( const PackageUri& rURI )
     if ( rURI.getPackage() == m_aUri.getPackage() )
     {
         xPackage = getPackage();
-        if ( !xPackage.is() )
-            return sal_False;
-
         return xPackage->hasByHierarchicalName( rURI.getPath() );
     }
 
@@ -2271,9 +2265,7 @@ sal_Bool Content::loadData(
             ContentProperties& rProps,
             uno::Reference< container::XHierarchicalNameAccess > & rxPackage )
 {
-    rxPackage = pProvider->createPackage( rURI.getPackage(), rURI.getParam() );
-    if ( !rxPackage.is() )
-        return sal_False;
+    rxPackage = pProvider->createPackage( rURI );
 
     if ( rURI.isRootFolder() )
     {
@@ -2469,8 +2461,6 @@ sal_Bool Content::renameData(
     PackageUri aURI( xOldId->getContentIdentifier() );
     uno::Reference< container::XHierarchicalNameAccess > xNA = getPackage(
                                                                         aURI );
-    if ( !xNA.is() )
-        return sal_False;
 
     if ( !xNA->hasByHierarchicalName( aURI.getPath() ) )
         return sal_False;
@@ -2508,8 +2498,6 @@ sal_Bool Content::storeData( const uno::Reference< io::XInputStream >& xStream )
     osl::Guard< osl::Mutex > aGuard( m_aMutex );
 
     uno::Reference< container::XHierarchicalNameAccess > xNA = getPackage();
-    if ( !xNA.is() )
-        return sal_False;
 
     uno::Reference< beans::XPropertySet > xPackagePropSet(
                                                     xNA, uno::UNO_QUERY );
@@ -2740,8 +2728,6 @@ sal_Bool Content::removeData()
     osl::Guard< osl::Mutex > aGuard( m_aMutex );
 
     uno::Reference< container::XHierarchicalNameAccess > xNA = getPackage();
-    if ( !xNA.is() )
-        return sal_False;
 
     PackageUri aParentUri( getParentURL() );
     if ( !xNA->hasByHierarchicalName( aParentUri.getPath() ) )
@@ -2785,8 +2771,6 @@ sal_Bool Content::flushData()
     //       by the single entries. Maybe this has to change...
 
     uno::Reference< container::XHierarchicalNameAccess > xNA = getPackage();
-    if ( !xNA.is() )
-        return sal_False;
 
     uno::Reference< util::XChangesBatch > xBatch( xNA, uno::UNO_QUERY );
     if ( !xBatch.is() )
@@ -2815,8 +2799,6 @@ uno::Reference< io::XInputStream > Content::getInputStream()
 
     uno::Reference< io::XInputStream > xStream;
     uno::Reference< container::XHierarchicalNameAccess > xNA = getPackage();
-    if ( !xNA.is() )
-        return xStream;
 
     if ( !xNA->hasByHierarchicalName( m_aUri.getPath() ) )
         return xStream;
@@ -2854,8 +2836,6 @@ uno::Reference< container::XEnumeration > Content::getIterator()
 
     uno::Reference< container::XEnumeration > xIter;
     uno::Reference< container::XHierarchicalNameAccess > xNA = getPackage();
-    if ( !xNA.is() )
-        return xIter;
 
     if ( !xNA->hasByHierarchicalName( m_aUri.getPath() ) )
         return xIter;
diff --git a/ucb/source/ucp/package/pkgprovider.cxx b/ucb/source/ucp/package/pkgprovider.cxx
index c27e3f2..8fdb2aa 100644
--- a/ucb/source/ucp/package/pkgprovider.cxx
+++ b/ucb/source/ucp/package/pkgprovider.cxx
@@ -30,6 +30,7 @@
 #include <cppuhelper/weak.hxx>
 #include <ucbhelper/contentidentifier.hxx>
 #include <com/sun/star/container/XHierarchicalNameAccess.hpp>
+#include <com/sun/star/lang/WrappedTargetRuntimeException.hpp>
 #include "pkgprovider.hxx"
 #include "pkgcontent.hxx"
 #include "pkguri.hxx"
@@ -232,17 +233,11 @@ uno::Reference< ucb::XContent > SAL_CALL ContentProvider::queryContent(
 //=========================================================================
 
 uno::Reference< container::XHierarchicalNameAccess >
-ContentProvider::createPackage( const OUString & rName, const OUString & rParam )
+ContentProvider::createPackage( const PackageUri & rURI )
 {
     osl::MutexGuard aGuard( m_aMutex );
 
-    if ( rName.isEmpty() )
-    {
-        OSL_FAIL( "ContentProvider::createPackage - Invalid URL!" );
-        return uno::Reference< container::XHierarchicalNameAccess >();
-    }
-
-    OUString rURL = rName + rParam;
+    OUString rURL = rURI.getPackage() + rURI.getParam();
 
     if ( m_pPackages )
     {
@@ -257,43 +252,30 @@ ContentProvider::createPackage( const OUString & rName, const OUString & rParam
         m_pPackages = new Packages;
 
     // Create new package...
+    uno::Sequence< uno::Any > aArguments( 1 );
+    aArguments[ 0 ] <<= rURL;
+    uno::Reference< container::XHierarchicalNameAccess > xNameAccess;
     try
     {
-        uno::Sequence< uno::Any > aArguments( 1 );
-        aArguments[ 0 ] <<= rURL;
-
-        uno::Reference< uno::XInterface > xIfc
-            = m_xContext->getServiceManager()->createInstanceWithArgumentsAndContext("com.sun.star.packages.comp.ZipPackage",
-                aArguments, m_xContext );
-
-        if ( xIfc.is() )
-        {
-            uno::Reference<
-                container::XHierarchicalNameAccess > xNameAccess(
-                                                        xIfc, uno::UNO_QUERY );
-
-            OSL_ENSURE( xNameAccess.is(),
-                        "ContentProvider::createPackage - "
-                        "Got no hierarchical name access!" );
-
-            rtl::Reference< Package> xPackage
-                = new Package( rURL, xNameAccess, this );
-
-            (*m_pPackages)[ rURL ] = xPackage.get();
-
-            return xPackage.get();
-        }
+        xNameAccess = uno::Reference< container::XHierarchicalNameAccess >(
+            m_xContext->getServiceManager()->createInstanceWithArgumentsAndContext(
+                "com.sun.star.packages.comp.ZipPackage",
+                aArguments, m_xContext ),
+            css::uno::UNO_QUERY_THROW );
     }
     catch ( uno::RuntimeException const & )
     {
-        // createInstanceWithArguemts
+        throw;
     }
-    catch ( uno::Exception const & )
+    catch ( uno::Exception const & e )
     {
-        // createInstanceWithArguemts
+        throw css::lang::WrappedTargetRuntimeException(
+            e.Message, e.Context, css::uno::makeAny(e));
     }
 
-    return uno::Reference< container::XHierarchicalNameAccess >();
+    rtl::Reference< Package> xPackage = new Package( rURL, xNameAccess, this );
+    (*m_pPackages)[ rURL ] = xPackage.get();
+    return xPackage.get();
 }
 
 //=========================================================================
diff --git a/ucb/source/ucp/package/pkgprovider.hxx b/ucb/source/ucp/package/pkgprovider.hxx
index a2d671f..4913442 100644
--- a/ucb/source/ucp/package/pkgprovider.hxx
+++ b/ucb/source/ucp/package/pkgprovider.hxx
@@ -87,7 +87,7 @@ public:
 
     ::com::sun::star::uno::Reference<
         ::com::sun::star::container::XHierarchicalNameAccess >
-    createPackage( const OUString & rName, const OUString & rParam );
+    createPackage( const PackageUri & rParam );
     sal_Bool
     removePackage( const OUString & rName );
 };
commit 740c7b1bb2132852784c01dfe21112ced3314eb1
Author: Stephan Bergmann <sbergman at redhat.com>
Date:   Mon May 27 17:51:08 2013 +0200

    Better debug logging
    
    Change-Id: Ida97a74a8629e0b170d882b38b0eeaa8f53d9bd3

diff --git a/desktop/source/deployment/registry/package/dp_package.cxx b/desktop/source/deployment/registry/package/dp_package.cxx
index 1aac891..51689a6 100644
--- a/desktop/source/deployment/registry/package/dp_package.cxx
+++ b/desktop/source/deployment/registry/package/dp_package.cxx
@@ -1407,13 +1407,14 @@ void BackendImpl::PackageImpl::scanBundle(
 {
     OSL_ASSERT( !m_legacyBundle );
 
+    OUString mfUrl( makeURL( m_url_expanded, "META-INF/manifest.xml" ) );
     ::ucbhelper::Content manifestContent;
     if (! create_ucb_content(
-            &manifestContent,
-            makeURL( m_url_expanded, "META-INF/manifest.xml" ),
-            xCmdEnv, false /* no throw */ ))
+            &manifestContent, mfUrl, xCmdEnv, false /* no throw */ ))
     {
-        OSL_FAIL( "### missing META-INF/manifest.xml file!" );
+        SAL_WARN(
+            "desktop.deployment",
+            "cannot create UCB Content for <" << mfUrl << ">" );
         return;
     }
 


More information about the Libreoffice-commits mailing list