Avoid setting dummy values inappropriately for original files.

[mtbc]

The previous code had dummy required values being set for hashes and lengths of OriginalFile instances for things like files that hadn't yet been uploaded, and for directories. Discussion in http://trac.openmicroscopy.org.uk/ome/ticket/2581 considers the idea of simply allowing such properties to be set to null unless there is actually an applicable value for them, so as to declutter the code and the database.

For OriginalFile instances, this PR makes the size and checksum optional, to tidy things slightly in anticipation of further changes once the checksum algorithm is made selectable. The model and dummy values around Pixels instances remain unchanged.

In testing, one might want to check the originalfile table in the DB to make sure that some size or sha1 that was previously being correctly set still is set, i.e. that this PR causes no regressions. Obviously, also test things like importing and annotating images with files to make sure that there are no regressions there either.

In the longer term one may want to consider designing a richer type hierarchy around OriginalFile.

---

--no-rebase FS only

[joshmoore]

Adding exclude label:

<http://hudson.openmicroscopy.org.uk/job/OMERO-merge-develop/ws/src/components/blitz/src/ome/services/blitz/repo/PublicRepositoryI.java>:146: cannot find symbol
symbol  : class ServerFilePathTransformer
location: class ome.services.blitz.repo.PublicRepositoryI
       this.serverPaths = new ServerFilePathTransformer();

[mtbc]

I have no idea what's going on here.

HEAD is now at 240b2c5... Avoid setting dummy values inappropriately for original files.
mark@ls27175:~/src/openmicroscopy$ grep 'import.*ServerFilePathTransformer' components/blitz/src/ome/services/blitz/repo/PublicRepositoryI.java 
import ome.services.blitz.repo.path.ServerFilePathTransformer;

It's imported, and builds fine locally. I assume it's some weird kind of merge conflict? Maybe I should wait a day, rebase and repush, and see what happens then.

[joshmoore]

It's likely a merge interaction on snoopy between your 2 PRs.

[mtbc]

Yeah, looks like a duplicate import, maybe two different PRs each removed a different duplicate. (-:

[mtbc]

It should be okay now. The conflict was with #881.

[joshmoore]

Since a number of DB changes are coming, this is likely not an issue, but as a heads up, modifying the SQL files in place can cause weird errors if someone is working with an older version of the DB. It's usually safest to bump the patch number forcing someone to re-create.

[joshmoore]

Note: this is also still excluded. Should it be?

[mtbc]

Can probably try including in build again; I don't have any reasons noted why not to.

Patch number: good idea, thank you. I'll push a commit today for that to get into the habit.

[mtbc]

Gah, actually maybe I oughtn't add a commit, I guess I add a few hundred kilobytes to the repository each time due to the directory name under sql/psql/ changing with each new patch version. Any rules of thumb here?

(Is it important to keep the old SQL in the current release -- can we just keep the version number out of the directory name and trust git and our archive of release artifacts to make older versions easily obtained when necessary? Not for this PR...)

[joshmoore]

I think we definitely want to work toward removing all SQL scripts from the main repo.

[joshmoore]

(But yes, in another PR)

[joshmoore]

For everyone else's benefit, using git mv sql/psql/...1.sql sql/psql/...2.sql && ./build.py build-schema prevents the large SQL diff until the SQL has been moved out completely. You can see the results here: 24e10ac410b6597

Otherwise, interested to get this in. There may be some documentation we need to do in terms of "expect nulls for sha1 and size". @knabar might be able to say how much of a surprise this could be.

Merging.

[mtbc]

Thanks very much, I'll remember that the git mv trick works well enough.

[joshmoore]

Note: @chris-allan was able to deploy this merged PR but without the dbpatch change the NOT NULLs on sha1 and size broke all imports. Summary: I shouldn't have clicked on this without having the bump of dbpatch in this PR (regardless of patch size).

[mtbc]

The git mv trick worked nicely for me on the version info PR so it shouldn't happen again, at least.