Last week, I attempted to use InflateColumn::File to discover that the latest version on CPAN is broken.
I have created a branch to resurrect it, since it appeared to require more than a simple bug fix. http://dev.catalyst.perl.org/repos/bast/DBIx-Class/0.08/branches/file_column I have committed changes that I believe restore the functionality to match the original spec. There are, however, some design considerations and I would like some feedback before merging to trunk. Below, I outline some problems I believe exist in the original design, propose some design changes to address them, and discuss why changing the API *may* be acceptable and not break existing user code. There may be flaws in my analysis and implementation, so I welcome review and feedback. I especially welcome any feedback for the original author to ensure his design goals are met. The original design inflates to a hashref: { handle => $fh, filename => $filename } And deflates to $filename. I find the file file handle troublesome for a couple of reasons: - An open file handle for each row of a result set seems like a potential drain on resources that should be avoided. - On insert, the hashref presumably contains a file handle, ready to read. On return from insert, the file handle will be positioned to EOF, but still open, making it fairly useless for the remaining life of the object. In the original design, on deflation, the file is saved to the file system as (pseudo code, here): $file_column_path/$pk/$filename This has the advantage of keeping $filename visible in the file system. It has the disadvantage of potentially leaving orphaned files behind in the file system, and worse, stomping on files that belong to other columns. For example, if $pk is 1 and we have a "file column" named fc1, storing a file named 'foo', we have the following in the file system: $file_column_path/1/foo Consider the update: $row->fc1({ handle => $fh, filename => 'bar' }); # later... $row->update; Without some extra code to keep track the prior filename, 'foo', so that we can delete it on update, we now have the following in the file system: $file_column_path/1/bar $file_column_path/1/foo 'foo' is orphaned. In a more troubling case, consider a table with multiple file columns, say 'fc1' and 'fc2'. If both columns are assigned files with the same name, one will overwrite the other. So, I propose the following: - By default, store to: $file_column_path/$pk/$column (from the prior examples, $file_column_path/1/fc1 instead of .../1/foo). - Inflate to a Path::Class object. This encapsulates the file name and allows: $fh = $row->fc1->open('r') as many times as necessary during the life of the object. - Provide a method encapsulating the file storage naming algorithm making it easy to override. The original design uses a separate path for each "file column", but it may be advantageous to use a different file structure for some applications. We might consider some other enhancements, as well. For instance, rather than inflating to a Path::Class, perhaps a class that also includes an optional MIME type. The class would serialize the filename and MIME type on deflation. I welcome some input, here. InflateColumn::File apparently worked at some point, but unless I'm mistaken, the working version was lost somewhere between the working development copy and release to CPAN. The problems that prevented it from working for me with the current CPAN release seem to exist in the version 0.07006 where it first appears. I didn't find any early version in the repository. If someone has it, it would be a useful reference. If no working version was ever released to CPAN, then presumably there is no existing user code that relies on it and we can make some design changes to address the problems I've pointed out without breaking existing code. If that isn't the case, the after some feedback and testing, we should merge this version (with any necessary corrections) to the current trunk to restore the documented functionality. -Marc _______________________________________________ List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class IRC: irc.perl.org#dbix-class SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/ Searchable Archive: http://www.grokbase.com/group/[EMAIL PROTECTED]
