Skip to content

Hide File instances from classes outside CheckedPath #881

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
joshmoore merged 4 commits into from
Mar 21, 2013
Merged

Hide File instances from classes outside CheckedPath #881

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
843f539
Polish CheckedPath; make specials dirs string constants.
mtbc Mar 14, 2013
a33f4c0
Fix #10372: hide CheckedPath's File instance
mtbc Mar 14, 2013
19b820a
Increase clarity of FsFile.toString() Javadoc.
mtbc Mar 18, 2013
0abb36a
Add CheckedPath helper method to avoid FsFile.toString().
mtbc Mar 18, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 127 additions & 15 deletions components/blitz/src/ome/services/blitz/repo/CheckedPath.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,23 @@
package ome.services.blitz.repo;

import java.io.File;
import java.io.IOException;
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

import javax.activation.MimetypesFileTypeMap;

import loci.formats.FormatException;
import loci.formats.ReaderWrapper;

import org.apache.commons.io.FileUtils;

import ome.util.Utils;
import ome.io.nio.FileBuffer;
import ome.services.blitz.repo.path.ServerFilePathTransformer;
import ome.services.blitz.repo.path.FsFile;

Expand All @@ -48,10 +56,13 @@
* @author m.t.b.carroll@dundee.ac.uk
*/
public class CheckedPath {

private static final String SAME_DIR = ".";
private static final String PARENT_DIR = "..";
private static final Set<String> SPECIAL_DIRS;

public final FsFile fsFile;
public /*final*/ boolean isRoot;
protected final File file;
private final File file;
private /*final*/ String parentDir;
private /*final*/ String baseName;
private final String original; // for error reporting
Expand All @@ -62,6 +73,13 @@ public class CheckedPath {
protected String sha1;
protected String mime;

static {
final Set<String> specialDirs = new HashSet<String>();
specialDirs.add(SAME_DIR);
specialDirs.add(PARENT_DIR);
SPECIAL_DIRS = Collections.unmodifiableSet(specialDirs);
}

/**
* Adjust an FsFile to remove "." components and to remove ".." components with the previous component.
* TODO: May not actually be necessary.
Expand All @@ -73,13 +91,13 @@ private FsFile processSpecialDirectories(FsFile fsFile) throws ValidationExcepti
final List<String> oldComponents = fsFile.getComponents();
final List<String> newComponents = new ArrayList<String>(oldComponents.size());
for (final String oldComponent : oldComponents)
if ("..".equals(oldComponent))
if (PARENT_DIR.equals(oldComponent))
if (newComponents.isEmpty())
throw new ValidationException(null, null, "Path may not make references above root");
else
// with Java 1.6 use a Deque
newComponents.remove(newComponents.size() - 1);
else if (!".".equals(oldComponent))
else if (!SAME_DIR.equals(oldComponent))
newComponents.add(oldComponent);
return new FsFile(newComponents);
}
Expand Down Expand Up @@ -184,18 +202,35 @@ public CheckedPath parent() throws ValidationException {
* @return
*/
public CheckedPath child(String name) throws ValidationException {
if (name == null) {
throw new ValidationException(null, null, "null name");
} else if (".".equals(name) || "..".equals(name)) {
throw new ValidationException(null, null,
"Only proper child name is allowed. Not '.' or '..'");
if (name == null || "".equals(name)) {
throw new ValidationException(null, null, "null or empty name");
} else if (SPECIAL_DIRS.contains(name)) {
final StringBuffer message = new StringBuffer();
message.append("Only proper child name is allowed, not ");
for (final String dir : SPECIAL_DIRS) {
message.append('\'');
message.append(dir);
message.append('\'');
message.append(", ");
}
message.setLength(message.length() - 2); // remove trailing ", "
message.append('.');
throw new ValidationException(null, null, message.toString());
} else if (name.indexOf(FsFile.separatorChar)>=0) {
throw new ValidationException(null, null,
"No subpaths allowed. Path contains '/'");
"No subpaths allowed. Path contains '" + FsFile.separatorChar + "'");
}
List<String> copy = new ArrayList<String>(this.fsFile.getComponents());
copy.add(name);
return new CheckedPath(new File(original, name), new FsFile(copy));
final FsFile fullChild = FsFile.concatenate(this.fsFile, new FsFile(name));
return new CheckedPath(new File(original, name), fullChild);
}

/**
* Check if this file actually exists on the underlying filesystem.
* Analogous to {@link java.io.File.exists()}.
* @return <code>true</code> if the file exists, <code>false</code> otherwise
*/
public boolean exists() {
return this.file.exists();
}

/**
Expand All @@ -206,7 +241,7 @@ public CheckedPath child(String name) throws ValidationException {
* @throws ValidationException
*/
public CheckedPath mustExist() throws ValidationException {
if (!file.exists()) {
if (!exists()) {
throw new ValidationException(null, null, original
+ " does not exist");
}
Expand All @@ -229,6 +264,15 @@ public CheckedPath mustEdit() throws omero.SecurityViolation {
return this;
}

/**
* Check if this file is actually readable on the underlying filesystem.
* Analogous to {@link java.io.File.canRead()}.
* @return <code>true</code> if the file is readable, <code>false</code> otherwise
*/
public boolean canRead() {
return this.file.canRead();
}

public boolean canEdit() {
return true;
}
Expand All @@ -245,14 +289,82 @@ protected String getDirname() {
return this.fsFile.toString() + FsFile.separatorChar;
}

/**
* Get the last component of this path, the entity to which the path corresponds.
* If this entity {@link #isRoot} then this is the empty string.
* @return the last path component
*/
protected String getName() {
return this.baseName;
}

/**
* Get the parent path of the entity to which this path corresponds.
* If this entity is not in some sub-directory below root,
* then this relative path is just the {@link FsFile#separatorChar}.
* @return the path components above the last,
* with separators including a trailing {@link FsFile#separatorChar}.
*/
protected String getRelativePath() {
return this.parentDir + FsFile.separatorChar;
}
}

/**
* The full path of the entity to which this path corresponds.
* Path components are separated by {@link FsFile#separatorChar}.
* @return the full path
*/
protected String getFullFsPath() {
return this.fsFile.toString();
}

/**
* Get a {@link FileBuffer} corresponding to this instance.
* It is the caller's responsibility to {@link FileBuffer#close()} it.
* @param mode as for {@link java.io.RandomAccessFile(File, String)},
* <code>"r"</code> and <code>"rw"</code> being common choices
* @return a new {@link FileBuffer}
*/
public FileBuffer getFileBuffer(String mode) {
return new FileBuffer(this.file.getPath(), mode);
}

/**
* Create this directory on the underlying filesystem.
* Analogous to {@link java.io.File.mkdir()}.
* @return <code>true</code> if the directory was created, <code>false</code> otherwise
*/
public boolean mkdir() {
return this.file.mkdir();
}

/**
* Create this directory, and parents if necessary, on the underlying filesystem.
* Analogous to {@link java.io.File.mkdirs()}.
* @return <code>true</code> if the directory was created, <code>false</code> otherwise
*/
public boolean mkdirs() {
return this.file.mkdirs();
}

/**
* Mark this existing file as having been modified at the present moment.
* @return <code>true</code> if the file's modification time was updated, <code>false</code> otherwise
*/
public boolean markModified() {
return this.file.setLastModified(System.currentTimeMillis());
}

/**
* Perform BioFormats {@link ReaderWrapper#setId(String)} for this file.
* @param reader the BioFormats reader upon which to operate
* @throws FormatException passed up from {@link ReaderWrapper#setId(String)}
* @throws IOException passed up from {@link ReaderWrapper#setId(String)}
*/
public void bfSetId(ReaderWrapper reader) throws FormatException, IOException {
Copy link
Member

@joshmoore joshmoore Mar 15, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though I understand the reasoning, this is the only delegator that feels a bit invasive.

Copy link
Member Author

@mtbc mtbc Mar 15, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree; I figured it was a lesser evil than attempting some major change to BioFormats.

reader.setId(file.getPath());
}

public String toString() {
return getClass().getSimpleName() + '(' + this.fsFile + ')';
}
Expand Down
7 changes: 3 additions & 4 deletions components/blitz/src/ome/services/blitz/repo/ManagedImportLocationI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

package ome.services.blitz.repo;

import java.io.File;
import java.util.List;

import omero.grid.ImportLocation;
Expand All @@ -33,10 +32,10 @@ public class ManagedImportLocationI extends ImportLocation {
public List<CheckedPath> checkedPaths;

/**
* Return the server-side {@link File} instance which can be passed to
* Return the server-side {@link CheckedPath} instance which can be passed to
* a Bio-Formats reader.
*/
public File getTarget() {
return checkedPaths.get(0).file;
public CheckedPath getTarget() {
return checkedPaths.get(0);
}
}
16 changes: 7 additions & 9 deletions components/blitz/src/ome/services/blitz/repo/ManagedImportRequestI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

package ome.services.blitz.repo;

import java.io.File;
import java.io.IOException;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
Expand Down Expand Up @@ -109,7 +108,7 @@ public class ManagedImportRequestI extends ImportRequest implements IRequest {

private OMEROWrapper reader = null;

private File file = null;
private CheckedPath file = null;

private IObject userSpecifiedTarget = null; // TODO: remove?

Expand Down Expand Up @@ -187,13 +186,12 @@ public void init(Helper helper) {
doThumbnails = settings.doThumbnails == null ? true :
settings.doThumbnails.getValue();

fileName = file.getAbsolutePath();
fileName = file.getFullFsPath();
shortName = file.getName();
format = null;
usedFiles = new String[1];
usedFiles[0] = file.getAbsolutePath();
usedFiles = new String[] {fileName};

open(reader, store, file.getAbsolutePath());
open(reader, store, file);
format = reader.getFormat();
if (reader.getUsedFiles() != null)
{
Expand Down Expand Up @@ -408,7 +406,7 @@ public Object pixelData(PixelDataJob pdj) throws Throwable {

// As we're in metadata only mode on we need to
// tell the server which Pixels set matches up to which series.
String targetName = file.getAbsolutePath();
final String targetName = file.getFullFsPath();
int series = 0;
for (Long pixelsId : pixelIds())
{
Expand Down Expand Up @@ -474,13 +472,13 @@ public Object script(ScriptJob sj) {

/** opens the file using the {@link FormatReader} instance */
private void open(OMEROWrapper reader, OMEROMetadataStoreClient store,
String fileName) throws IOException, FormatException
CheckedPath targetFile) throws FormatException, IOException
{
reader.close();
reader.setMetadataStore(store);
reader.setMinMaxStore(store);
store.setReader(reader.getImageReader());
reader.setId(fileName);
targetFile.bfSetId(reader);
//reset series count
if (log.isDebugEnabled())
{
Expand Down
16 changes: 7 additions & 9 deletions components/blitz/src/ome/services/blitz/repo/PublicRepositoryI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import java.util.ArrayList;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;

import org.apache.commons.io.filefilter.FileFilterUtils;
import org.apache.commons.io.filefilter.IOFileFilter;
Expand All @@ -47,9 +46,8 @@
import ome.formats.importer.ImportConfig;
import ome.formats.importer.OMEROWrapper;
import ome.services.blitz.impl.AbstractAmdServant;
import ome.services.blitz.repo.path.FsFile;
import ome.services.blitz.repo.path.ServerFilePathTransformer;
import ome.services.blitz.impl.ServiceFactoryI;
import ome.services.blitz.repo.path.FsFile;
import ome.services.blitz.repo.path.MakePathComponentSafe;
import ome.services.blitz.repo.path.ServerFilePathTransformer;
import ome.services.blitz.util.BlitzExecutor;
Expand Down Expand Up @@ -386,7 +384,7 @@ protected OriginalFile findOrCreateInDb(CheckedPath checked, String mode,
return ofile;
}

if (checked.file.exists()) {
if (checked.exists()) {
omero.grid.UnregisteredFileException ufe
= new omero.grid.UnregisteredFileException();
ofile = (OriginalFile) new IceMapper().map(checked.asOriginalFile(null));
Expand Down Expand Up @@ -575,11 +573,11 @@ protected void makeCheckedDirs(final LinkedList<CheckedPath> paths,
while (paths.size() > 1) { // Only possible if `parents`
checked = paths.removeFirst();

if (checked.file.exists()) {
if (!checked.file.isDirectory()) {
if (checked.exists()) {
if (!checked.isDirectory()) {
throw new omero.ResourceError(null, null,
"Path is not a directory.");
} else if (!checked.file.canRead()) {
} else if (!checked.canRead()) {
throw new omero.ResourceError(null, null,
"Directory is not readable");
}
Expand All @@ -595,12 +593,12 @@ protected void makeCheckedDirs(final LinkedList<CheckedPath> paths,

// Now we are ready to work on the actual intended path.
checked = paths.removeFirst(); // Size is now empty
if (checked.file.exists()) {
if (checked.exists()) {
if (parents) {
assertFindDir(checked, __current);
} else {
throw new omero.ResourceError(null, null,
"Path exists on disk:" + checked.file);
"Path exists on disk: " + checked.fsFile);
}
}
repositoryDao.register(repoUuid, checked,
Expand Down
Loading