Re: please read: storage interface

[Michael Meeks <michael helixcode com>]

Hi Dietmar,

On Wed, 11 Oct 2000, Dietmar Maurer wrote:
> maybe some of you already noticed that I have sent many mails in the
> last month concerning the storage interface. The storage interface is
> simply not ready and I already pointed out its weakness.
 
        I agree.

> Although I sent many mails to the list I have received very few
> constructive answers. My point of view is:
       
        So lets have some criticism :-) firstly, overall I like the
changes, they seem to be very worthwhile.

--- bonobo-storage.idl	Mon Oct  9 19:00:01 2000
+++ bonobo-storage-fat.idl	Fri Oct 13 03:21:21 2000

 module Bonobo {
 
+        const long MASK_CONTENT_TYPE = 1;
+        const long MASK_SIZE         = 2;
+        const long MASK_TYPE         = 4;
+
+	typedef string ContentType;
+
+	struct StorageInfo {
+		string		name;
+		unsigne long    type;
	typo:	unsigned long

	I would call this InfoFields to match the VFS' naming scheme
also name the 'MASK' stuff more lucidly eg. FIELD_CONTENT_TYPE.

 		/**
+		 * get_info:
+		 *
+		 * Returns a StorageInfo structure which contains
+		 * the name, content_type and size info.
+		 */
+		StorageInfo get_info (in long mask)
+			raises (IOError);
+		
+		/**	
+		 * set_info:
+		 *
+		 */
+		void set_info (in StorageInfo info, in long mask)
+			raises (IOError);
+		

	These look good; it may be possible to have things that we
are not allowed to set the permission of, perhaps it is good to add
NoPermission exceptions here.

 		 *
 		 * writes the buffer to this stream.
 		 */
-		long write (in iobuf buffer)
+		void write (in iobuf buffer)
 			raises (NoPermission, IOError);

	I'm less convinced about this, but I've not seen much 
code that deals with short writes correctly and it confuses
people, better to block as you suggest.

-		/**
-		 * commit:
-		 *
-		 * Commits any pending changes to the Storage
-		 */
-		void commit ();

	How did we loose this ? an accident I assume.

-		/**
-		 * close:
-		 *
-		 * Close the Stream
-		 */
-		void close ();

	This is made redundant by the Stream object's lifecycle
management; good.

-		/**
-		 * eos:
-		 *
-		 * End of Stream?
-		 */
-
-		boolean eos ();

	The implementations of this function look extremely ugly /
non-existant. I assume you propose testing eos by a short read.
 
-		/**
-		 * length:
-		 *
-		 * Returns the length of the stream
-		 */
-		long length ()
-			raises (NotSupported);

	Again we can find out this from your 'stat' like command
if we need it I suppose.

-		const OpenMode DENY_READ = 4;
-		const OpenMode DENY_WRITE = 8;
+		const OpenMode CREATE      = 4;
+		const OpenMode FAILIFEXIST = 8;
+		const OpenMode COMPRESSED  = 16; /* try to compress */

	You prolly want an EXCLUSIVE flag of some sort here.

+		exception NotStream {};
+		exception NotStorage {};
+		StorageInfo get_info (in string path, in long mask)
+			raises (IOError);
+		void set_info (in string path, in StorageInfo info, 
+			       in long mask)
+			raises (IOError);

	You prolly want a few more exceptions here ?
 
+		/**
+		 * list_contents:
+		 * @path: path that we want to examine.
+		 *
+		 * Returns a list of all the Storage and Streams available
+		 * at @path.
+		 */
+		DirectoryList list_contents (in string path, in long mask)
+			raises (IOError, NotStorage, NotFound);

	This minor rename directory_list -> DirectoryList we can do with
the impending StdlyCapsification.
 
-		/**
-		 * commit:
-		 * 
-		 * Commits any pending changes to the Storage since it was
-		 * opened.  This operation is syncronous.
-		 */
-
-		void commit ();

	I'm sure we need this.

	Otherwise it looks nice indeed. It seems to me that very few of
these changes will have a compatibility impact which is great.

	Regards,

		Michael.

-- 
 mmeeks gnu org  <><, Pseudo Engineer, itinerant idiot