Skip to content

Irsa 729 add ws commands #477

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.

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
merged 4 commits into from
Oct 20, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 3 additions & 4 deletions config/app-test.prop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,10 @@ irsa.gator.service.periodogram.keys=x y alg peaks

workspace.host.url=https://irsa.ipac.caltech.edu
workspace.root.dir=/work
workspace.protocol.irsa=webdav
workspace.protocol.irsa.webdav=edu.caltech.ipac.firefly.server.WebDAVWorkspaceManager

workspace.protocol=webdav
workspace.protocol.webdav=edu.caltech.ipac.firefly.server.WebDAVWorkspaceManager
# Another Impl?-> See edu.caltech.ipac.firefly.server.ws.WebDAVWorkspaceHandler#withCredentials
# workspace.protocol.irsa.webdav=edu.caltech.ipac.firefly.server.ws.WebdavImpl
# workspace.protocol.webdav=edu.caltech.ipac.firefly.server.ws.WebdavImpl

atlas.ibe.host=irsadev.ipac.caltech.edu

12 changes: 9 additions & 3 deletions src/firefly/java/edu/caltech/ipac/firefly/data/ServerParams.java
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,14 @@ public class ServerParams {
public static final String LOGOUT = "CmdLogout";

//Workspaces
public static final String WS_LIST = "wsList";
public static final String WS_GET_FILE = "wsGet";
public static final String WS_PUT_FILE = "wsPut";
public static final String WS_LIST = "wsList"; // Gets the list of content/files
public static final String WS_GET_FILE = "wsGet"; // TODO
public static final String WS_UPLOAD_FILE = "wsUpload";// to get the path of the file uploaded into firefly server from WS
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is needed. We already have an 'upload' command that will handle url and multipart. We can add from workspace in there as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

We need to decide what what workspace upload even means. I would prefer for out stuff treat workspace file the same way we treat URLs. Not so much with URLDownload but as a file that is retrieved when it is used instread requiring the user to wait while a file is transferred from one server to another. it could come in as workspace://a/b/c.tbl then we have the workspace layer know how to deal with that.

Copy link
Contributor

Choose a reason for hiding this comment

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

if the request is sent to CmdSrv by using command ServerParams.UPLOAD, current request parameter has either file: or URL: , we may add workspace: for workspace upload.

public static final String WS_PUT_IMAGE_FILE = "wsPut"; //FITS, PNG, comes from edu.caltech.ipac.firefly.server.servlets.AnyFileDownload
public static final String WS_PUT_TABLE_FILE = "wsPutTable";
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a need to distinguish image from table? Why not have WS_PUT_FILE and allow for additional params. I am sure there are other types beside image and table.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, a file should be a file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i feel that will be in the context where we use the WS manager that piece can be moved later to the right place, keeping it like this for now.

public static final String WS_DELETE_FILE = "wsDel";
public static final String WS_MOVE_FILE = "wsMove";
public static final String WS_GET_METADATA = "wsGetMeta";
public static final String WS_CREATE_FOLDER = "wsParent";
}

Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public class WspaceMeta implements Serializable {

public static final String DESC = "desc";
public static final String TYPE = "type";
private boolean isFile = false; //folder

public void setUrl(String url) {
this.url = url;
Expand All @@ -34,6 +35,10 @@ public String getUrl() {
return url;
}

public boolean isFile() {
return this.isFile;
}

public enum Includes {
NONE(false, 0), NONE_PROPS(true, 0), CHILDREN(false, 1), CHILDREN_PROPS(true, 1), ALL(false), ALL_PROPS(true);
public boolean inclProps = false;
Expand Down Expand Up @@ -176,6 +181,10 @@ public String getProperty(String keyname) {
return val;
}

public void setIsFile(boolean isFile) {
this.isFile = isFile;
}

public void setSize(long size) {
this.size = size;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,11 @@ public WsResponse renameFile(String originalFileRelPath, String fileName, boolea
throw new IllegalArgumentException("not implemented");
}

@Override
public WsResponse moveFile(String originalFileRelPath, String newfilepath, boolean shouldOverwrite) throws WsException {
throw new IllegalArgumentException("not implemented");
}

@Override
public WspaceMeta getMeta(String uri, WspaceMeta.Includes includes) {
throw new IllegalArgumentException("not implemented");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,15 @@ public Date getStartTime() {
}

public Map<String, String> getCookieMap() {
Map<String, Cookie> cookies = requestAgent.getCookies();
Map<String, String> cmap = new HashMap<>(cookies.size());
for(Cookie c : cookies.values()) {
String v = c == null ? null : c.getValue();
if (v != null) {
cmap.put(c.getName(), c.getValue());
Map<String, String> cmap = null;
if(requestAgent!=null) {
Map<String, Cookie> cookies = requestAgent.getCookies();
cmap = new HashMap<>(cookies.size());
for (Cookie c : cookies.values()) {
String v = c == null ? null : c.getValue();
if (v != null) {
cmap.put(c.getName(), c.getValue());
}
}
}
return cmap;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,13 @@ private static void initCommand() {
//Workspaces
_cmdMap.put(ServerParams.WS_LIST, new WsServerCommands.WsList());
_cmdMap.put(ServerParams.WS_GET_FILE, new WsServerCommands.WsGetFile());
_cmdMap.put(ServerParams.WS_PUT_FILE, new WsServerCommands.WsPutFile());
_cmdMap.put(ServerParams.WS_UPLOAD_FILE, new WsServerCommands.WsUploadFile());
_cmdMap.put(ServerParams.WS_DELETE_FILE, new WsServerCommands.WsDeleteFile());
_cmdMap.put(ServerParams.WS_MOVE_FILE, new WsServerCommands.WsMoveFile());
_cmdMap.put(ServerParams.WS_GET_METADATA, new WsServerCommands.WsGetMeta());
_cmdMap.put(ServerParams.WS_CREATE_FOLDER, new WsServerCommands.WsCreateParent());
_cmdMap.put(ServerParams.WS_PUT_TABLE_FILE, new WsServerCommands.WsPutTableFile());
_cmdMap.put(ServerParams.WS_PUT_IMAGE_FILE, new WsServerCommands.WsPutImgFile());

_cmdMap.put(ServerParams.RAW_DATA_SET, new SearchServerCommands.GetRawDataSet());
_cmdMap.put(ServerParams.JSON_DATA, new SearchServerCommands.GetJSONData());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ public WsResponse davGet(File outfile, String fromPath) throws IOException {
try {
if (!executeMethod(get, false)) {
// handle error
LOG.error("Unable to download file:" + fromPath + " -- " + get.getStatusText());
LOG.error("Unable to download file:" + fromPath + " , url " +url+" -- "+ get.getStatusText());
return WsUtil.error(get.getStatusCode(), get.getStatusText());
}
if (get.getResponseContentLength() > 0) {
Expand Down Expand Up @@ -374,7 +374,7 @@ public WsResponse delete(String uri) throws WsException {
try {
if (!executeMethod(rm, true)) {
// handle error
LOG.error("Unable to delete file:" + uri + " -- " + rm.getStatusText());
LOG.error("Unable to delete file uri:" + uri + " -- " + rm.getStatusText());
return WsUtil.error(rm);
}
} catch (Exception e) {
Expand Down Expand Up @@ -407,6 +407,25 @@ public WsResponse createParent(String newRelPath) throws WsException {
return WsUtil.success(200, "Created", getResourceUrl(newRelPath));
}

public WsResponse moveFile(String originalFileRelPath, String newPath, boolean overwrite) throws WsException {
WspaceMeta meta = new WspaceMeta(newPath);
String parent = meta.getParentPath();

//WARNING: WEBDAv forces me to create new parent first! UGLY!
createParent(parent);

String newUrl = getResourceUrl(newPath);

// This doesn't create parent , you must create the parent folder in order to move it
MoveMethod move = new MoveMethod(getResourceUrl(originalFileRelPath), newUrl, overwrite);
if (!executeMethod(move)) {
// handle error
LOG.error("Unable to move:" + originalFileRelPath + " based on url -- " +newUrl+" -- "+ move.getStatusText());
return WsUtil.error(move.getStatusCode(), move.getStatusLine().getReasonPhrase());
}
return WsUtil.success(move.getStatusCode(), move.getStatusText(), newUrl);
}

@Override
public WsResponse renameFile(String originalFileRelPath, String newfileName, boolean overwrite) throws WsException {
WspaceMeta meta = new WspaceMeta(originalFileRelPath);
Expand All @@ -417,7 +436,7 @@ public WsResponse renameFile(String originalFileRelPath, String newfileName, boo
MoveMethod move = new MoveMethod(getResourceUrl(originalFileRelPath), newUrl, overwrite);
if (!executeMethod(move)) {
// handle error
LOG.error("Unable to move:" + originalFileRelPath + " -- " + move.getStatusText());
LOG.error("Unable to move:" + originalFileRelPath + " based on url -- " +newUrl+" -- "+ move.getStatusText());
return WsUtil.error(move.getStatusCode(), move.getStatusLine().getReasonPhrase());
}
return WsUtil.success(move.getStatusCode(), move.getStatusText(), newUrl);
Expand All @@ -427,34 +446,6 @@ public WspaceMeta getMeta(String relPath) {
return getMeta(relPath, WspaceMeta.Includes.ALL);
}

public WsResponse search(String relPath) throws IOException {
String query = "" +
" " +
" SELECT *" +
" " +
"";
query = "//element(*,rep:root)";

OptionsMethod options = new OptionsMethod(getResourceUrl(relPath));
executeMethod(options);
String okMethod = "SEARCH";
if (!options.isAllowed(okMethod)) {
String s = "";
String[] allowedMethods = options.getAllowedMethods();
for (String method : allowedMethods) {
s += method + "\n";
}
return WsUtil.error(options.getStatusCode(), options.getStatusText(), okMethod + " is not allowed - only:\n" + s);
}
SearchMethod m = new SearchMethod(getResourceUrl(relPath), query, "xpath");
if (!executeMethod(m)) {
// handle error
LOG.error("Unable to move:" + relPath + " -- " + m.getStatusText());
return WsUtil.error(m.getStatusCode(), m.getStatusLine().getReasonPhrase());
}
return WsUtil.success(m.getStatusCode(), m.getStatusText(), relPath);
}

public WspaceMeta getMeta(String relPath, WspaceMeta.Includes includes) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to get a set of props for all the files. i.e. owner?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, getMeta("/", ALL). See unit test or I can explain later when we meet again.


// this can be optimized by retrieving only the props we care for.
Expand Down Expand Up @@ -594,7 +585,9 @@ private WspaceMeta convertToWspaceMeta(WspaceMeta meta, MultiStatusResponse res)
if (name.equals(DavConstants.PROPERTY_GETLASTMODIFIED)) {
meta.setLastModified(v);
} else if (name.equals(DavConstants.PROPERTY_GETCONTENTLENGTH)) {
meta.setSize(Long.parseLong(v));
long size = Long.parseLong(v);
meta.setSize(size);
meta.setIsFile(true); // WEBDAV/IRSA only set cvontent length for files. (?)TODO: is it WEBDav general behaviour?
} else if (name.equals(DavConstants.PROPERTY_GETCONTENTTYPE)) {
meta.setContentType(v);
} else if (p.getName().getNamespace().equals(IRSA_NS)) {
Expand Down Expand Up @@ -635,45 +628,45 @@ private boolean executeMethod(DavMethod method, boolean releaseConnection) {

}

// public static void main(String[] args) {
// WebDAVWorkspaceManager man = null;
//// man = (WebDAVWorkspaceManager) ServerContext.getRequestOwner().getWsManager();
//
// man = new WebDAVWorkspaceManager("ejoliet-tmp2");
//
public static void main(String[] args) {
WebDAVWorkspaceManager man = null;
// man = (WebDAVWorkspaceManager) ServerContext.getRequestOwner().getWsManager();

man = new WebDAVWorkspaceManager("ejoliet-tmp2");

simpleTest(man);

// AppProperties.setProperty("sso.server.url", "http://irsa.ipac.caltech.edu/account/");
// String session = JOSSOAdapter.createSession("", "");
// Map<String, String> cookies = new HashMap<String, String>();
// cookies.put(WebAuthModule.AUTH_KEY, session);
// WorkspaceManager man = new WorkspaceManager("<someuser>@ipac.caltech.edu", cookies);
// simpleTest(man);
//
//// AppProperties.setProperty("sso.server.url", "http://irsa.ipac.caltech.edu/account/");
//// String session = JOSSOAdapter.createSession("", "");
//// Map<String, String> cookies = new HashMap<String, String>();
//// cookies.put(WebAuthModule.AUTH_KEY, session);
//// WorkspaceManager man = new WorkspaceManager("<someuser>@ipac.caltech.edu", cookies);
//// simpleTest(man);
// }
//
// private static void simpleTest(WebDAVWorkspaceManager man) {
// String relPath = "124/";
// File f = man.davMakeDir(relPath);
// System.out.println("new directory: " + String.valueOf(f));
//
// WspaceMeta m = new WspaceMeta(null, relPath);
// m.setProperty("test1", "an awesome idea");
// m.setProperty("test", null);
// man.setMeta(m);
//
// String ufilePath = relPath + "gaia-binary.vot";
// WsResponse wsResponse = man.davPut(new File("/Users/ejoliet/devspace/branch/dev/firefly_test_data/edu/caltech/ipac/firefly/ws/gaia-binary.vot"), ufilePath, null);
//
// System.out.println(wsResponse);
//
// WspaceMeta meta = new WspaceMeta(null, ufilePath);
// meta.setProperty("added_by", man.userHome);
// man.setMeta(meta);
// man.setMeta(ufilePath, "added_by", man.userHome);
//
// WspaceMeta meta2 = man.getMeta("/", WspaceMeta.Includes.ALL_PROPS);
// System.out.println(meta2.getNodesAsString());
// }
}

private static void simpleTest(WebDAVWorkspaceManager man) {
String relPath = "124/";
File f = man.davMakeDir(relPath);
System.out.println("new directory: " + String.valueOf(f));

WspaceMeta m = new WspaceMeta(null, relPath);
m.setProperty("test1", "an awesome idea");
m.setProperty("test", null);
man.setMeta(m);

String ufilePath = relPath + "gaia-binary.vot";
WsResponse wsResponse = man.davPut(new File("/Users/ejoliet/devspace/branch/dev/firefly_test_data/edu/caltech/ipac/firefly/ws/gaia-binary.vot"), ufilePath, null);

System.out.println(wsResponse);

WspaceMeta meta = new WspaceMeta(null, ufilePath);
meta.setProperty("added_by", man.userHome);
man.setMeta(meta);
man.setMeta(ufilePath, "added_by", man.userHome);

WspaceMeta meta2 = man.getMeta("/", WspaceMeta.Includes.ALL_PROPS);
System.out.println(meta2.getNodesAsString());
}


class WebDAVGetMethod extends DavMethodBase {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
*/
public class LocalWorkspaceHandler implements WorkspaceHandler {

String p = AppProperties.getProperty("workspace.protocol.irsa.webdav", "edu.caltech.ipac.firefly.server.WebDAVWorkspaceManager");

/**
* map userKey and ws,
* TODO we don't want to create a new Wsmanager for every user access, do we?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/
public class WebDAVWorkspaceHandler implements WorkspaceHandler {

String managerClass = AppProperties.getProperty("workspace.protocol.irsa.webdav", "edu.caltech.ipac.firefly.server.WebDAVWorkspaceManager");


/**
* map userKey and ws,
Expand All @@ -31,10 +31,13 @@ public WorkspaceManager withCredentials(WsCredentials cred) {

synchronized (cred) {
String id = cred.getWsId();
Copy link
Contributor

Choose a reason for hiding this comment

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

cred did not override equals. I doubt if this synchronized does anything. Plus, I don't think a wsPool is necessary. The manager is meant as a way to encapsulate logic. It's should be fine for single use.

Copy link
Contributor

@robyww robyww Oct 14, 2017

Choose a reason for hiding this comment

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

I don't think it will work as expected either. However, I would like to know what you are trying to accomplish. What will happen it is is not synchronized? If it is necessary there might be another way to do it.

String protocol = AppProperties.getProperty("workspace.protocol", "webdav");
String managerClass = AppProperties.getProperty("workspace.protocol."+protocol.toLowerCase(), "edu.caltech.ipac.firefly.server.WebDAVWorkspaceManager");

if (wsPool.containsKey(id)) {
//TODO The problem is how do i know that the cookkie/session is authtenticated at this point?
/*if (wsPool.containsKey(id)) {
return wsPool.get(id);
}
}*/
WorkspaceManager ws = null;//new WebDAVWorkspaceManager(cred.getWsId(), cred.getCookies());
try {
Class clazz = Class.forName(managerClass);
Copy link
Contributor

Choose a reason for hiding this comment

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

I like using reflection here. This should be very flexible.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@
*/
public class WorkspaceFactory {

static String protocolProp = AppProperties.getProperty("workspace.protocol.irsa", "webdav");
static String protocolProp = AppProperties.getProperty("workspace.protocol", "webdav");
private static WebDAVWorkspaceHandler webDavHdlr;

/**
* @return Handler based on protocol read from properties workspace.protocol.irsa
* @return Handler based on protocol read from properties workspace.protocol
* @throws WsException
*/
public static WorkspaceHandler getWorkspaceHandler() {
Expand All @@ -31,7 +32,10 @@ public static WorkspaceHandler getWorkspaceHandler(WorkspaceManager.PROTOCOL p)
case LOCAL:
return new LocalWorkspaceHandler();
case WEBDAV:
return new WebDAVWorkspaceHandler();
if(webDavHdlr==null){
webDavHdlr = new WebDAVWorkspaceHandler();
}
return webDavHdlr;
case VOSPACE:
throw new ResourceNotFoundException(p.name() + " not implemented yet ");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,16 @@ public interface Workspaces {
*/
WsResponse renameFile(String originalFileRelPath, String newfileName, boolean shouldOverwrite) throws WsException;

/**
* @param originalFileRelPath relative current path
* @param newfilepath new path
* @param shouldOverwrite
* @return
* @throws WsException
*/
WsResponse moveFile(String originalFileRelPath, String newfilepath, boolean shouldOverwrite) throws WsException;


WspaceMeta getMeta(String uri, WspaceMeta.Includes includes);

boolean setMeta(WspaceMeta... metas);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ enum AUTH {NONE, TOKEN, DIGEST, BASIC}

public WsCredentials(String wsId, Map<String, String> cookies) {
this.wsId = wsId;

this.cookies = cookies;
}

Expand Down
Loading