From 0944a0d88314a0069d042695ffd1c385484d5446 Mon Sep 17 00:00:00 2001 From: hudajkhan Date: Thu, 5 Dec 2013 16:20:55 -0500 Subject: [PATCH 1/2] updates for page management issues vivo622 and 616 --- webapp/web/i18n/all.properties | 2 + webapp/web/js/menupage/pageManagementUtils.js | 45 ++++++++-- .../menupage--exampleMultipleContentTypes.ftl | 84 +++++++++++++++++++ .../freemarker/edit/forms/pageManagement.ftl | 3 +- 4 files changed, 128 insertions(+), 6 deletions(-) create mode 100644 webapp/web/templates/freemarker/body/menupage/menupage--exampleMultipleContentTypes.ftl diff --git a/webapp/web/i18n/all.properties b/webapp/web/i18n/all.properties index d201b66c9..818c27c4a 100644 --- a/webapp/web/i18n/all.properties +++ b/webapp/web/i18n/all.properties @@ -697,6 +697,7 @@ select_existing_collaborator = Select an existing Collaborator for {0} selected = Selected change_selection = change selection there_are_no_entries_for_selection = There are no entries in the system from which to select. +the_range_class_does_not_exist= The range class for this property does not exist in the system. editing_prohibited = This property is currently configured to prohibit editing. confirm_entry_deletion_from = Are you sure you want to delete the following entry from @@ -752,6 +753,7 @@ custom_template_containing_content = Custom template containing all content a_menu_page = This is a menu page menu_item_name = Menu Item Name if_blank_page_title_used = If left blank, the page title will be used. +multiple_content_default_template_error = With multiple content types, you must specify a custom template. label = label no_classes_to_select = There are no Classes in the system from which to select. diff --git a/webapp/web/js/menupage/pageManagementUtils.js b/webapp/web/js/menupage/pageManagementUtils.js index 20b95899b..54b6cfa45 100644 --- a/webapp/web/js/menupage/pageManagementUtils.js +++ b/webapp/web/js/menupage/pageManagementUtils.js @@ -205,13 +205,15 @@ var pageManagementUtils = { //Also clear custom template value so as not to submit it pageManagementUtils.clearInputs(pageManagementUtils.customTemplate); pageManagementUtils.rightSideDiv.show(); - pageManagementUtils.disablePageSave(); + //Check to see if there is already content on page, in which case save should be enabled + var pageContentSections = $("section[class='pageContent']"); + if(pageContentSections.length == 0) { + pageManagementUtils.disablePageSave(); + } }); this.customTemplateRadio.click( function() { - pageManagementUtils.customTemplate.removeClass('hidden'); - pageManagementUtils.rightSideDiv.show(); - pageManagementUtils.disablePageSave(); + pageManagementUtils.handleSelectCustomTemplate(); }); this.selfContainedTemplateRadio.click( function() { @@ -265,6 +267,16 @@ var pageManagementUtils = { }); }, + handleSelectCustomTemplate: function() { + pageManagementUtils.customTemplate.removeClass('hidden'); + pageManagementUtils.rightSideDiv.show(); + //Check to see if there is already content on page, in which case save should be enabled + var pageContentSections = $("section[class='pageContent']"); + if(pageContentSections.length == 0) { + pageManagementUtils.disablePageSave(); + } + }, + handleClickDone:function() { var selectedType = pageManagementUtils.contentTypeSelect.val(); var selectedTypeText = $("#typeSelect option:selected").text(); @@ -392,6 +404,9 @@ var pageManagementUtils = { pageManagementUtils.adjustSaveButtonHeight(); //Disable save button until the user has clicked done or cancel from the addition pageManagementUtils.disablePageSave(); + //If the default template is selected, there is already content on the page, and the user is selecting new content + //display alert message that they must select a custom template and select + pageManagementUtils.checkTemplateForMultipleContent(_this.contentTypeSelect.val()); }, disablePageSave:function() { pageManagementUtils.pageSaveButton.attr("disabled", "disabled"); @@ -430,6 +445,21 @@ var pageManagementUtils = { $el.find("select option:eq(0)").attr("selected", "selected"); }, + checkTemplateForMultipleContent:function(contentTypeSelected) { + if(contentTypeSelected != "") { + var pageContentSections = $("section[class='pageContent']"); + var selectedTemplateValue = $('input:radio[name=selectedTemplate]:checked').val(); + //A new section hasn't been added yet so check to see if there is at least one content type already on page + if(selectedTemplateValue == "default" && pageContentSections.length >= 1) { + //alert the user that they should be picking custom template instead + alert(pageManagementUtils.multipleContentWithDefaultTemplateError); + //pick custom template + $('input:radio[name=selectedTemplate][value="custom"]').attr("checked", true); + pageManagementUtils.handleSelectCustomTemplate(); + + } + } + }, //Clone content area //When adding a new content type, this function will copy the values from the new content form and generate //the content for the new section containing the content @@ -874,7 +904,12 @@ var pageManagementUtils = { if(pageContentSections.length == 0) { validationErrorMsg = pageManagementUtils.selectContentType + "
"; } else { - //For each, based on type, validate if a validation function exists + //If there are multiple content types, and the default template option is selected, then display error message + var selectedTemplateValue = $('input:radio[name=selectedTemplate]:checked').val(); + if(selectedTemplateValue == "default") { + validationErrorMsg += pageManagementUtils.multipleContentWithDefaultTemplateError; + } + //For each, based on type, validate if a validation function exists $.each(pageContentSections, function(i) { if(pageManagementUtils.processDataGetterUtils != null) { var dataGetterType = pageManagementUtils.processDataGetterUtils.selectDataGetterType($(this)); diff --git a/webapp/web/templates/freemarker/body/menupage/menupage--exampleMultipleContentTypes.ftl b/webapp/web/templates/freemarker/body/menupage/menupage--exampleMultipleContentTypes.ftl new file mode 100644 index 000000000..7eca46eac --- /dev/null +++ b/webapp/web/templates/freemarker/body/menupage/menupage--exampleMultipleContentTypes.ftl @@ -0,0 +1,84 @@ +<#-- $This file is distributed under the terms of the license in /doc/license.txt$ --> +<#--This is an example of including multiple content types in the same template, this combines the default templates for Fixed HTML, Class groups and Solr Individuals in one template--> +<#include "menupage-checkForData.ftl"> +<#--Fixed HTML portion--> +<#--Note that variableName is employed by both the fixed html and sparql query templates, this is used to store the +actual name of the variable that is used to store either the fixed html or sparql query results. If combining fixed html +and sparql query results in a custom template, the template can utilize the actual variable name e.g. "query results" instead of how +variableName is used below.--> +<#assign htmlExists = false/> + +<#if variableName?has_content> + <#assign htmlExists = true /> + +<#if htmlExists> + ${.globals[variableName]} +<#else> + ${i18n().no_html_specified} + + +<#--Class grou section--> +<#if !noData> + + + <#include "menupage-browse.ftl"> + + ${stylesheets.add('')} + + <#include "menupage-scripts.ftl"> +<#else> + ${noDataNotification} + + +<#--Solr Individuals section--> +<#import "lib-list.ftl" as l> + +<#include "individualList-checkForData.ftl"> + +${stylesheets.add('')} + +
+

${title} +

+ <#if subtitle?has_content> +

${subtitle}

+ + + <#if (!noData)> + <#if errorMessage?has_content> +

${errorMessage}

+ <#else> + <#assign pagination> + <#if (pages?has_content && pages?size > 1)> + ${i18n().pages}: + + + + + ${pagination} + + + + ${pagination} + + <#else> + ${noDataNotification} + +
\ No newline at end of file diff --git a/webapp/web/templates/freemarker/edit/forms/pageManagement.ftl b/webapp/web/templates/freemarker/edit/forms/pageManagement.ftl index 108caeb5b..2d248e65f 100644 --- a/webapp/web/templates/freemarker/edit/forms/pageManagement.ftl +++ b/webapp/web/templates/freemarker/edit/forms/pageManagement.ftl @@ -189,7 +189,8 @@ supplyPrettyUrl: '${i18n().supply_url}', startUrlWithSlash: '${i18n().start_url_with_slash}', supplyTemplate: '${i18n().supply_template}', - selectContentType: '${i18n().select_content_type}' + selectContentType: '${i18n().select_content_type}', + multipleContentWithDefaultTemplateError: '${i18n().multiple_content_default_template_error}' }; From d08c3f61ff77cbfe894d45079cb586450781f058 Mon Sep 17 00:00:00 2001 From: j2blake Date: Thu, 5 Dec 2013 16:36:26 -0500 Subject: [PATCH 2/2] VIVO-618 Revise SearchService to parse uploaded files correctly and to respond properly to authentication errors. --- .../webapp/auth/policy/PolicyHelper.java | 101 ++++---- .../api/NotAuthorizedToUseApiException.java | 22 ++ .../FileUploadServletRequest.java | 221 ++--------------- .../MultipartHttpServletRequest.java | 215 ++++++++++++++-- .../SimpleHttpServletRequestWrapper.java | 23 +- .../StreamingMultipartHttpServletRequest.java | 38 --- .../controller/SearchServiceController.java | 187 +++++++------- .../search/controller/UpdateUrisInIndex.java | 234 ++++++------------ .../controller/UpdateUrisInIndexTest.java | 125 ++++++---- .../body/search/searchService-help.ftl | 9 +- 10 files changed, 557 insertions(+), 618 deletions(-) create mode 100644 webapp/src/edu/cornell/mannlib/vitro/webapp/controller/api/NotAuthorizedToUseApiException.java delete mode 100644 webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/StreamingMultipartHttpServletRequest.java diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/PolicyHelper.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/PolicyHelper.java index b1250a855..dad123fc8 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/PolicyHelper.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/PolicyHelper.java @@ -28,7 +28,6 @@ import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.propstmt.DropDataPr import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.propstmt.DropObjectPropertyStatement; import edu.cornell.mannlib.vitro.webapp.beans.UserAccount; import edu.cornell.mannlib.vitro.webapp.controller.authenticate.Authenticator; -import edu.cornell.mannlib.vitro.webapp.controller.authenticate.BasicAuthenticator; /** * A collection of static methods to help determine whether requested actions @@ -56,7 +55,7 @@ public class PolicyHelper { IdentifierBundle ids = RequestIdentifiers.getIdBundleForRequest(req); return isAuthorizedForActions(ids, policy, actions); } - + /** * Are these actions authorized for these identifiers by these policies? */ @@ -66,47 +65,53 @@ public class PolicyHelper { } /** - * Is the email/password authorized for these actions? - * This should be used when a controller or something needs allow - * actions if the user passes in their email and password. + * Is the email/password authorized for these actions? This should be used + * when a controller or something needs allow actions if the user passes in + * their email and password. * - * It may be better to check this as part of a servlet Filter and - * add an identifier bundle. + * It may be better to check this as part of a servlet Filter and add an + * identifier bundle. */ - public static boolean isAuthorizedForActions( HttpServletRequest req, - String email, String password, - Actions actions){ - - if( password == null || email == null || - password.isEmpty() || email.isEmpty()){ - return false; - } - - try{ - Authenticator auth = Authenticator.getInstance(req); - UserAccount user = auth.getAccountForInternalAuth( email ); - log.debug("userAccount is " + user==null?"null":user.getUri() ); - - if( ! auth.isCurrentPassword( user, password ) ){ - log.debug(String.format("UNAUTHORIZED, password not accepted for %s, account URI: %s", - user.getEmailAddress(), user.getUri())); - return false; - }else{ - log.debug(String.format("password accepted for %s, account URI: %s", - user.getEmailAddress(), user.getUri() )); - // figure out if that account can do the actions - IdentifierBundle ids = - ActiveIdentifierBundleFactories.getUserIdentifierBundle(req,user); - PolicyIface policy = ServletPolicyList.getPolicies(req); - return PolicyHelper.isAuthorizedForActions( ids, policy, actions ); - } - - }catch(Exception ex){ - log.error("Error while attempting to authorize actions " + actions.toString(), ex); - return false; - } + public static boolean isAuthorizedForActions(HttpServletRequest req, + String email, String password, Actions actions) { + + if (password == null || email == null || password.isEmpty() + || email.isEmpty()) { + return false; + } + + try { + Authenticator auth = Authenticator.getInstance(req); + UserAccount user = auth.getAccountForInternalAuth(email); + if (user == null) { + log.debug("No account for '" + email + "'"); + return false; + } + + String uri = user.getUri(); + log.debug("userAccount is '" + uri + "'"); + + if (!auth.isCurrentPassword(user, password)) { + log.debug(String.format("UNAUTHORIZED, password not accepted " + + "for %s, account URI: %s", email, uri)); + return false; + } + log.debug(String.format("password accepted for %s, " + + "account URI: %s", email, uri)); + + // figure out if that account can do the actions + IdentifierBundle ids = ActiveIdentifierBundleFactories + .getUserIdentifierBundle(req, user); + PolicyIface policy = ServletPolicyList.getPolicies(req); + return PolicyHelper.isAuthorizedForActions(ids, policy, actions); + } catch (Exception ex) { + log.error( + "Error while attempting to authorize actions " + + actions.toString(), ex); + return false; + } } - + /** * Do the current policies authorize the current user to add this statement * to this model? @@ -120,8 +125,8 @@ public class PolicyHelper { } Resource subject = stmt.getSubject(); - edu.cornell.mannlib.vitro.webapp.beans.Property predicate = - new edu.cornell.mannlib.vitro.webapp.beans.Property(stmt.getPredicate().getURI()); + edu.cornell.mannlib.vitro.webapp.beans.Property predicate = new edu.cornell.mannlib.vitro.webapp.beans.Property( + stmt.getPredicate().getURI()); RDFNode objectNode = stmt.getObject(); if ((subject == null) || (predicate == null) || (objectNode == null)) { return false; @@ -130,8 +135,8 @@ public class PolicyHelper { RequestedAction action; if (objectNode.isResource()) { action = new AddObjectPropertyStatement(modelToBeModified, - subject.getURI(), predicate, objectNode - .asResource().getURI()); + subject.getURI(), predicate, objectNode.asResource() + .getURI()); } else { action = new AddDataPropertyStatement(modelToBeModified, subject.getURI(), predicate.getURI(), objectNode @@ -153,8 +158,7 @@ public class PolicyHelper { } Resource subject = stmt.getSubject(); - edu.cornell.mannlib.vitro.webapp.beans.Property predicate = - new edu.cornell.mannlib.vitro.webapp.beans.Property(); + edu.cornell.mannlib.vitro.webapp.beans.Property predicate = new edu.cornell.mannlib.vitro.webapp.beans.Property(); predicate.setURI(stmt.getPredicate().getURI()); RDFNode objectNode = stmt.getObject(); if ((subject == null) || (predicate == null) || (objectNode == null)) { @@ -164,8 +168,8 @@ public class PolicyHelper { RequestedAction action; if (objectNode.isResource()) { action = new DropObjectPropertyStatement(modelToBeModified, - subject.getURI(), predicate, objectNode - .asResource().getURI()); + subject.getURI(), predicate, objectNode.asResource() + .getURI()); } else { action = new DropDataPropertyStatement(modelToBeModified, subject.getURI(), predicate.getURI(), objectNode @@ -310,7 +314,6 @@ public class PolicyHelper { + stmt.getObject() + ">"; } - /** * No need to instantiate this helper class - all methods are static. */ diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/api/NotAuthorizedToUseApiException.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/api/NotAuthorizedToUseApiException.java new file mode 100644 index 000000000..8484ecc39 --- /dev/null +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/api/NotAuthorizedToUseApiException.java @@ -0,0 +1,22 @@ +/* $This file is distributed under the terms of the license in /doc/license.txt$ */ + +package edu.cornell.mannlib.vitro.webapp.controller.api; + +/** + * When you try to use an API that you aren't authorized for, we don't redirect + * you to the login page. We complain. + */ +public class NotAuthorizedToUseApiException extends Exception { + public NotAuthorizedToUseApiException(String message) { + super(message); + } + + public NotAuthorizedToUseApiException(Throwable cause) { + super(cause); + } + + public NotAuthorizedToUseApiException(String message, Throwable cause) { + super(message, cause); + } + +} diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/FileUploadServletRequest.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/FileUploadServletRequest.java index ae34cabf8..519e2f07c 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/FileUploadServletRequest.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/FileUploadServletRequest.java @@ -3,15 +3,8 @@ package edu.cornell.mannlib.vitro.webapp.filestorage.uploadrequest; import java.io.IOException; -import java.io.UnsupportedEncodingException; -import java.net.URLDecoder; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Enumeration; -import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequestWrapper; @@ -19,8 +12,6 @@ import javax.servlet.http.HttpServletRequestWrapper; import org.apache.commons.fileupload.FileItem; import org.apache.commons.fileupload.FileUploadException; import org.apache.commons.fileupload.servlet.ServletFileUpload; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; /** *

@@ -51,41 +42,22 @@ import org.apache.commons.logging.LogFactory; * implemented in the sub-classes. *

*/ -@SuppressWarnings("deprecation") public abstract class FileUploadServletRequest extends HttpServletRequestWrapper { - - private static final Log log = LogFactory - .getLog(FileUploadServletRequest.class); - public static final String FILE_ITEM_MAP = "file_item_map"; public static final String FILE_UPLOAD_EXCEPTION = "file_upload_exception"; - private Map> parameters; - private Map> files; - private FileUploadException fileUploadException; - - private static final String[] EMPTY_ARRAY = new String[0]; - // ---------------------------------------------------------------------- // The factory method // ---------------------------------------------------------------------- /** * Wrap this {@link HttpServletRequest} in an appropriate wrapper class. - * set maxTempFileSize to 0 or -1 if streaming is desired. Set it to > 0 if - * you want any parts uploaded to a temporary directory */ - public static FileUploadServletRequest parseRequest( - HttpServletRequest request, int maxTempFileSize) throws IOException { - + public static FileUploadServletRequest parseRequest( + HttpServletRequest request, int maxFileSize) throws IOException { boolean isMultipart = ServletFileUpload.isMultipartContent(request); - if (isMultipart) { - if( maxTempFileSize <= 0 ){ - return new StreamingMultipartHttpServletRequest(request); - }else{ - return new MultipartHttpServletRequest(request, maxTempFileSize); - } + return new MultipartHttpServletRequest(request, maxFileSize); } else { return new SimpleHttpServletRequestWrapper(request); } @@ -112,184 +84,29 @@ public abstract class FileUploadServletRequest extends HttpServletRequestWrapper /** Was this a multipart HTTP request? */ public abstract boolean isMultipart(); - - protected void stashParametersInRequest(HttpServletRequest request, ServletFileUpload upload){ - Map> parameters = new HashMap>(); - Map> files = new HashMap>(); - - parseQueryString(request.getQueryString(), parameters); - - try { - List items = upload.parseRequest( request ); - - for (FileItem item : items) { - // Process a regular form field - if (item.isFormField()) { - addToParameters(parameters, item.getFieldName(), item - .getString("UTF-8")); - log.debug("Form field (parameter) " + item.getFieldName() - + "=" + item.getString()); - } else { - addToFileItems(files, item); - log.debug("File " + item.getFieldName() + ": " - + item.getName()); - } - } - } catch (FileUploadException e) { - fileUploadException = e; - request.setAttribute( - FileUploadServletRequest.FILE_UPLOAD_EXCEPTION, e); - } catch (UnsupportedEncodingException e) { - log.error("could not convert to UTF-8",e); - } - - this.parameters = Collections.unmodifiableMap(parameters); - log.debug("Parameters are: " + this.parameters); - this.files = Collections.unmodifiableMap(files); - log.debug("Files are: " + this.files); - request.setAttribute(FILE_ITEM_MAP, this.files); - } - - /** - * Pull any parameters out of the URL. + * Get the map of file items, by name. */ - private void parseQueryString(String queryString, - Map> parameters) { - log.debug("Query string is : '" + queryString + "'"); - if (queryString != null) { - String[] pieces = queryString.split("&"); - - for (String piece : pieces) { - int equalsHere = piece.indexOf('='); - if (piece.trim().isEmpty()) { - // Ignore an empty piece. - } else if (equalsHere <= 0) { - // A parameter without a value. - addToParameters(parameters, decode(piece), ""); - } else { - // A parameter with a value. - String key = piece.substring(0, equalsHere); - String value = piece.substring(equalsHere + 1); - addToParameters(parameters, decode(key), decode(value)); - } - } - } - log.debug("Parameters from query string are: " + parameters); - } + public abstract Map> getFiles(); /** - * Remove any special URL-style encoding. + * Find a non-empty file item with this name. + * + * @return the first such file item, or null if no matching, + * non-empty items were found. */ - private String decode(String encoded) { - try { - return URLDecoder.decode(encoded, "UTF-8"); - } catch (UnsupportedEncodingException e) { - log.error(e, e); - return encoded; - } - } + public abstract FileItem getFileItem(String string); + /** + * Was there an exception when uploading the file items? + */ + public abstract boolean hasFileUploadException(); - /** Either create a new List for the value, or add to an existing List. */ - private void addToParameters(Map> map, String name, - String value) { - if (!map.containsKey(name)) { - map.put(name, new ArrayList()); - } - map.get(name).add(value); - } - - /** Either create a new List for the file, or add to an existing List. */ - private void addToFileItems(Map> map, FileItem file) { - String name = file.getFieldName(); - if (!map.containsKey(name)) { - map.put(name, new ArrayList()); - } - map.get(name).add(file); - } - - - public FileUploadException getFileUploadException() { - return fileUploadException; - } - - public boolean hasFileUploadException() { - return fileUploadException != null; - } - - // ---------------------------------------------------------------------- - // Parameter-related methods won't find anything on the delegate request, - // since parsing consumed the parameters. So we need to look to the parsed - // info for the answers. - // ---------------------------------------------------------------------- - - @Override - public String getParameter(String name) { - if (parameters.containsKey(name)) { - return parameters.get(name).get(0); - } else { - return null; - } - } - - @Override - public Enumeration getParameterNames() { - return Collections.enumeration(parameters.keySet()); - } - - @Override - public String[] getParameterValues(String name) { - if (parameters.containsKey(name)) { - return parameters.get(name).toArray(EMPTY_ARRAY); - } else { - return null; - } - } - - @Override - public Map getParameterMap() { - Map result = new HashMap(); - for (Entry> entry : parameters.entrySet()) { - result.put(entry.getKey(), entry.getValue().toArray(EMPTY_ARRAY)); - } - log.debug("resulting parameter map: " + result); - return result; - } - - - /** - * {@inheritDoc} - *

- * There may be more than one file item with the given name. If the first - * one is empty (size is zero), keep looking for a non-empty one. - *

- */ - public FileItem getFileItem(String name) { - List items = files.get(name); - if (items == null) { - return null; - } - - for (FileItem item : items) { - if (item.getSize() > 0L) { - return item; - } - } - - return null; - } - - /** - * Gets a map of parameter names to files. - * This will should return null. - */ - public Map> getFiles() { - if( files == null ) - return Collections.emptyMap(); - else - return files; - } + /** + * Get the exception that occurred when uploading the file items. If no such + * exception, return null. + */ + public abstract FileUploadException getFileUploadException(); } diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/MultipartHttpServletRequest.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/MultipartHttpServletRequest.java index a276e2275..56cf480fc 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/MultipartHttpServletRequest.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/MultipartHttpServletRequest.java @@ -29,33 +29,100 @@ import org.apache.commons.logging.LogFactory; * any parameter-related requests. File-related information will also be held * here, to answer file-related requests. */ -public class MultipartHttpServletRequest extends FileUploadServletRequest { +class MultipartHttpServletRequest extends FileUploadServletRequest { private static final Log log = LogFactory .getLog(MultipartHttpServletRequest.class); - private int maxFileSize = 0; - private File tempDir = null; - + private static final String[] EMPTY_ARRAY = new String[0]; + + private final Map> parameters; + private final Map> files; + private FileUploadException fileUploadException; + /** * Parse the multipart request. Store the info about the request parameters - * and the uploaded files to a temporary directory. - * - * This offers a simple way to deal with uploaded files by having a size - * limit. This limit may be rather large if desired. Many megabytes for example. + * and the uploaded files. */ public MultipartHttpServletRequest(HttpServletRequest request, int maxFileSize) throws IOException { super(request); - - this.maxFileSize = maxFileSize; - this.tempDir = figureTemporaryDirectory(request); - //use an upload handler that will stash the file items in a temporary directory. - ServletFileUpload upload = createUploadHandler( this.maxFileSize, this.tempDir ); - stashParametersInRequest( request , upload ); + Map> parameters = new HashMap>(); + Map> files = new HashMap>(); + + File tempDir = figureTemporaryDirectory(request); + ServletFileUpload upload = createUploadHandler(maxFileSize, tempDir); + + parseQueryString(request.getQueryString(), parameters); + + try { + List items = parseRequestIntoFileItems(request, upload); + + for (FileItem item : items) { + // Process a regular form field + if (item.isFormField()) { + addToParameters(parameters, item.getFieldName(), item + .getString("UTF-8")); + log.debug("Form field (parameter) " + item.getFieldName() + + "=" + item.getString()); + } else { + addToFileItems(files, item); + log.debug("File " + item.getFieldName() + ": " + + item.getName()); + } + } + } catch (FileUploadException e) { + fileUploadException = e; + request.setAttribute( + FileUploadServletRequest.FILE_UPLOAD_EXCEPTION, e); + } + + this.parameters = Collections.unmodifiableMap(parameters); + log.debug("Parameters are: " + this.parameters); + this.files = Collections.unmodifiableMap(files); + log.debug("Files are: " + this.files); + request.setAttribute(FILE_ITEM_MAP, this.files); + } + + /** + * Pull any parameters out of the URL. + */ + private void parseQueryString(String queryString, + Map> parameters) { + log.debug("Query string is : '" + queryString + "'"); + if (queryString != null) { + String[] pieces = queryString.split("&"); + + for (String piece : pieces) { + int equalsHere = piece.indexOf('='); + if (piece.trim().isEmpty()) { + // Ignore an empty piece. + } else if (equalsHere <= 0) { + // A parameter without a value. + addToParameters(parameters, decode(piece), ""); + } else { + // A parameter with a value. + String key = piece.substring(0, equalsHere); + String value = piece.substring(equalsHere + 1); + addToParameters(parameters, decode(key), decode(value)); + } + } + } + log.debug("Parameters from query string are: " + parameters); + } + + /** + * Remove any special URL-style encoding. + */ + private String decode(String encoded) { + try { + return URLDecoder.decode(encoded, "UTF-8"); + } catch (UnsupportedEncodingException e) { + log.error(e, e); + return encoded; + } } - /** * Find the temporary storage directory for this webapp. */ @@ -63,7 +130,7 @@ public class MultipartHttpServletRequest extends FileUploadServletRequest { return (File) request.getSession().getServletContext().getAttribute( "javax.servlet.context.tempdir"); } - + /** * Create an upload handler that will throw an exception if the file is too * large. @@ -79,10 +146,116 @@ public class MultipartHttpServletRequest extends FileUploadServletRequest { return upload; } - @Override - public boolean isMultipart() { - return true; - } + /** Either create a new List for the value, or add to an existing List. */ + private void addToParameters(Map> map, String name, + String value) { + if (!map.containsKey(name)) { + map.put(name, new ArrayList()); + } + map.get(name).add(value); + } + + /** Either create a new List for the file, or add to an existing List. */ + private void addToFileItems(Map> map, FileItem file) { + String name = file.getFieldName(); + if (!map.containsKey(name)) { + map.put(name, new ArrayList()); + } + map.get(name).add(file); + } + + /** Minimize the code that uses the unchecked cast. */ + @SuppressWarnings("unchecked") + private List parseRequestIntoFileItems(HttpServletRequest req, + ServletFileUpload upload) throws FileUploadException { + return upload.parseRequest(req); + } + + // ---------------------------------------------------------------------- + // This is a multipart request, so make the file info available. If there + // was an exception during parsing, make that available too. + // ---------------------------------------------------------------------- + + @Override + public boolean isMultipart() { + return true; + } + + @Override + public Map> getFiles() { + return files; + } + + /** + * {@inheritDoc} + *

+ * There may be more than one file item with the given name. If the first + * one is empty (size is zero), keep looking for a non-empty one. + *

+ */ + @Override + public FileItem getFileItem(String name) { + List items = files.get(name); + if (items == null) { + return null; + } + + for (FileItem item : items) { + if (item.getSize() > 0L) { + return item; + } + } + + return null; + } + + @Override + public FileUploadException getFileUploadException() { + return fileUploadException; + } + + @Override + public boolean hasFileUploadException() { + return fileUploadException != null; + } + + // ---------------------------------------------------------------------- + // Parameter-related methods won't find anything on the delegate request, + // since parsing consumed the parameters. So we need to look to the parsed + // info for the answers. + // ---------------------------------------------------------------------- + + @Override + public String getParameter(String name) { + if (parameters.containsKey(name)) { + return parameters.get(name).get(0); + } else { + return null; + } + } + + @Override + public Enumeration getParameterNames() { + return Collections.enumeration(parameters.keySet()); + } + + @Override + public String[] getParameterValues(String name) { + if (parameters.containsKey(name)) { + return parameters.get(name).toArray(EMPTY_ARRAY); + } else { + return null; + } + } + + @Override + public Map getParameterMap() { + Map result = new HashMap(); + for (Entry> entry : parameters.entrySet()) { + result.put(entry.getKey(), entry.getValue().toArray(EMPTY_ARRAY)); + } + log.debug("resulting parameter map: " + result); + return result; + } - } diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/SimpleHttpServletRequestWrapper.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/SimpleHttpServletRequestWrapper.java index 754226317..246377048 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/SimpleHttpServletRequestWrapper.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/SimpleHttpServletRequestWrapper.java @@ -33,7 +33,26 @@ class SimpleHttpServletRequestWrapper extends FileUploadServletRequest { return false; } - + @Override + public Map> getFiles() { + return Collections.emptyMap(); + } + + @Override + public FileItem getFileItem(String string) { + return null; + } + + @Override + public FileUploadException getFileUploadException() { + return null; + } + + @Override + public boolean hasFileUploadException() { + return false; + } + // ---------------------------------------------------------------------- // Since this is not a multipart request, the parameter methods can be // delegated. @@ -45,7 +64,7 @@ class SimpleHttpServletRequestWrapper extends FileUploadServletRequest { } @Override - public Map getParameterMap() { + public Map getParameterMap() { return getDelegate().getParameterMap(); } diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/StreamingMultipartHttpServletRequest.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/StreamingMultipartHttpServletRequest.java deleted file mode 100644 index e5e32c5c9..000000000 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/StreamingMultipartHttpServletRequest.java +++ /dev/null @@ -1,38 +0,0 @@ -/* $This file is distributed under the terms of the license in /doc/license.txt$ */ - -package edu.cornell.mannlib.vitro.webapp.filestorage.uploadrequest; - -import javax.servlet.http.HttpServletRequest; - -import org.apache.commons.fileupload.servlet.ServletFileUpload; - -/** - * Wrapping ServletRequest that does multipart. In order to allow - * streaming, this class does NOT save the parts to a temporary directory. - * - * - */ -public class StreamingMultipartHttpServletRequest extends - FileUploadServletRequest { - /** - * Parse the multipart request. Store the info about the request parameters. - * Don't store the uploaded files to a temporary directory to allow streaming. - * - * Only use this if you plan to consume the FileItems using streaming - * to deal with inputs of very large sizes. - * - */ - public StreamingMultipartHttpServletRequest(HttpServletRequest request) { - super(request); - - //use a file uploader that does not save the files to a temporary directory. - stashParametersInRequest( request , new ServletFileUpload()); - } - - @Override - public boolean isMultipart() { - return true; - } - - -} diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/search/controller/SearchServiceController.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/search/controller/SearchServiceController.java index b2dd876ab..88175db88 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/search/controller/SearchServiceController.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/search/controller/SearchServiceController.java @@ -2,7 +2,12 @@ package edu.cornell.mannlib.vitro.webapp.search.controller; +import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN; +import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; + import java.io.IOException; +import java.util.HashMap; +import java.util.Map; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; @@ -10,124 +15,130 @@ import javax.servlet.http.HttpServletRequest; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import edu.cornell.mannlib.vitro.webapp.auth.identifier.ActiveIdentifierBundleFactories; -import edu.cornell.mannlib.vitro.webapp.auth.identifier.IdentifierBundle; import edu.cornell.mannlib.vitro.webapp.auth.permissions.SimplePermission; import edu.cornell.mannlib.vitro.webapp.auth.policy.PolicyHelper; -import edu.cornell.mannlib.vitro.webapp.auth.policy.ServletPolicyList; -import edu.cornell.mannlib.vitro.webapp.auth.policy.ifaces.PolicyIface; -import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.Actions; -import edu.cornell.mannlib.vitro.webapp.beans.UserAccount; import edu.cornell.mannlib.vitro.webapp.controller.VitroRequest; -import edu.cornell.mannlib.vitro.webapp.controller.authenticate.Authenticator; -import edu.cornell.mannlib.vitro.webapp.controller.authenticate.BasicAuthenticator; +import edu.cornell.mannlib.vitro.webapp.controller.api.NotAuthorizedToUseApiException; import edu.cornell.mannlib.vitro.webapp.controller.freemarker.FreemarkerHttpServlet; import edu.cornell.mannlib.vitro.webapp.controller.freemarker.responsevalues.ExceptionResponseValues; import edu.cornell.mannlib.vitro.webapp.controller.freemarker.responsevalues.ResponseValues; import edu.cornell.mannlib.vitro.webapp.controller.freemarker.responsevalues.TemplateResponseValues; import edu.cornell.mannlib.vitro.webapp.filestorage.uploadrequest.FileUploadServletRequest; -import edu.cornell.mannlib.vitro.webapp.filestorage.uploadrequest.MultipartHttpServletRequest; import edu.cornell.mannlib.vitro.webapp.search.indexing.IndexBuilder; /** - * Accepts requests to update a set of URIs in the search index. + * Accepts requests to update a set of URIs in the search index. */ @SuppressWarnings("serial") public class SearchServiceController extends FreemarkerHttpServlet { - private static final Log log = LogFactory.getLog(SearchServiceController.class); + private static final Log log = LogFactory + .getLog(SearchServiceController.class); - /** - * Attempt to check if there is a email and password in the request parameters. - * If there is not, fall back on the normal usage pattern of requestedAction(). - * If there is, then try to authenticate and authorize the - * userAccount associated with the email. - */ - @Override - public Actions requiredActions(VitroRequest vreq) { - try{ - // Works by side effect: parse the multi-part request and stash FileItems in request - FileUploadServletRequest.parseRequest(vreq, 0); + /** Limit file size to 1 Gigabyte. */ + public static final int MAXIMUM_FILE_SIZE = 1024 * 1024 * 1024; - //first figure out if the password and email login to an account with a password - String pw = vreq.getParameter("password"); - String email = vreq.getParameter("email"); - - if( pw == null || email == null || pw.isEmpty() || email.isEmpty()){ - return SimplePermission.MANAGE_SEARCH_INDEX.ACTIONS; - } - - if( PolicyHelper.isAuthorizedForActions(vreq, email, pw, - SimplePermission.MANAGE_SEARCH_INDEX.ACTIONS ) ){ - return Actions.AUTHORIZED; - }else{ - log.debug(email + " is unauthorized to manage the search index. " + - "client IP "+vreq.getClientAddr()); - return Actions.UNAUTHORIZED; - } - - }catch(Exception ex){ - log.error("Error while client IP "+ vreq.getClientAddr() + " attempting to log in " + - "to SearchServiceController: " + ex.getMessage()); - return Actions.UNAUTHORIZED; - } - } - - /** * Handle the different actions. If not specified, the default action is to * show the help page. - */ + */ @Override protected ResponseValues processRequest(VitroRequest req) { try { - - //figure out what action to perform - String pathInfo = req.getPathInfo(); - - if( pathInfo == null || pathInfo.trim().isEmpty() || "/".equals(pathInfo.trim()) ){ - return doHelpForm(req); - } - - pathInfo = pathInfo.substring(1); //get rid of leading slash - - if (VERBS.UPDATE_URIS_IN_SEARCH.verb.equals( pathInfo )) { + req = new VitroRequest(FileUploadServletRequest.parseRequest(req, + MAXIMUM_FILE_SIZE)); + + // Check the authorization here, because we don't want to redirect + // to the login page if they are not authorized. (The file upload + // would be lost. + confirmAuthorization(req); + + switch (figureActionVerb(req)) { + case UPDATE_URIS_IN_SEARCH: return doUpdateUrisInSearch(req); - } else { - return doHelpForm(req); + default: + return doHelpForm(); } + } catch (NotAuthorizedToUseApiException e) { + Map map = new HashMap<>(); + map.put("errorMessage", e.getMessage()); + return new TemplateResponseValues("error-message.ftl", map, + SC_FORBIDDEN); } catch (Exception e) { - return new ExceptionResponseValues(e); + return new ExceptionResponseValues(e, SC_INTERNAL_SERVER_ERROR); } } + /** + * Process requests to the web service to update a list of URIs in the + * search index. + */ + private ResponseValues doUpdateUrisInSearch(HttpServletRequest req) + throws IOException, ServletException { + IndexBuilder builder = IndexBuilder.getBuilder(getServletContext()); + int uriCount = new UpdateUrisInIndex().doUpdateUris(req, builder); - /** - * Process requests to the web service to update a list of URIs in the search index. */ - public ResponseValues doUpdateUrisInSearch(HttpServletRequest req ) - throws IOException, ServletException { - - IndexBuilder builder = IndexBuilder.getBuilder(getServletContext()); - if( builder == null ) - throw new ServletException( "Could not get search index builder from context. Check smoke test"); + Map body = new HashMap<>(); + body.put("msg", "Received " + uriCount + " URIs."); - new UpdateUrisInIndex().doUpdateUris( req, builder); + return new TemplateResponseValues("searchService-help.ftl", body); + } - TemplateResponseValues trv = new TemplateResponseValues( "" ); - return trv; - } + private ResponseValues doHelpForm() { + return new TemplateResponseValues("searchService-help.ftl"); + } - - public ResponseValues doHelpForm(HttpServletRequest req){ - return new TemplateResponseValues( "searchService-help.ftl"); - } - - public enum VERBS{ - UPDATE_URIS_IN_SEARCH("updateUrisInSearch"); - - public final String verb; - VERBS(String verb){ - this.verb = verb; - } - } - + private void confirmAuthorization(VitroRequest vreq) + throws NotAuthorizedToUseApiException { + Verb verb = figureActionVerb(vreq); + String pw = vreq.getParameter("password"); + String email = vreq.getParameter("email"); + + // If you just want the help screen, no problem. + if (verb == Verb.VIEW_HELP_FORM) { + return; + } + // For other functions, your credentials must have moxie. + if (PolicyHelper.isAuthorizedForActions(vreq, email, pw, + SimplePermission.MANAGE_SEARCH_INDEX.ACTIONS)) { + return; + } + // Otherwise, you can't do this. + throw new NotAuthorizedToUseApiException(email + + " is not authorized to manage the search index."); + } + + private Verb figureActionVerb(VitroRequest vreq) { + String pathInfo = vreq.getPathInfo(); + if (pathInfo == null) { + pathInfo = ""; + } + if (pathInfo.startsWith("/")) { + pathInfo = pathInfo.substring(1); + } + return Verb.fromString(pathInfo); + } + + // ---------------------------------------------------------------------- + // Helper class + // ---------------------------------------------------------------------- + + public enum Verb { + VIEW_HELP_FORM("viewHelpForm"), UPDATE_URIS_IN_SEARCH( + "updateUrisInSearch"); + + public final String verb; + + Verb(String verb) { + this.verb = verb; + } + + static Verb fromString(String s) { + for (Verb v : values()) { + if (v.verb.equals(s)) { + return v; + } + } + return VIEW_HELP_FORM; + } + } } diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/search/controller/UpdateUrisInIndex.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/search/controller/UpdateUrisInIndex.java index 94998759d..5c3e1636f 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/search/controller/UpdateUrisInIndex.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/search/controller/UpdateUrisInIndex.java @@ -2,191 +2,101 @@ package edu.cornell.mannlib.vitro.webapp.search.controller; -import java.io.BufferedReader; +import static edu.cornell.mannlib.vitro.webapp.filestorage.uploadrequest.FileUploadServletRequest.FILE_ITEM_MAP; + import java.io.IOException; -import java.io.InputStream; import java.io.InputStreamReader; import java.io.Reader; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Iterator; +import java.nio.charset.Charset; import java.util.List; +import java.util.Map; +import java.util.Scanner; import java.util.regex.Pattern; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import org.apache.commons.fileupload.FileItemIterator; -import org.apache.commons.fileupload.FileItemStream; -import org.apache.commons.fileupload.FileUploadException; -import org.apache.commons.fileupload.servlet.ServletFileUpload; +import org.apache.commons.fileupload.FileItem; +import org.apache.commons.lang.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import edu.cornell.mannlib.vitro.webapp.search.indexing.IndexBuilder; /** - * Class that performs the update of the uris in the search index - * for the SearchService. + * Class that performs the update of the uris in the search index for the + * SearchService. */ public class UpdateUrisInIndex { - private static final Log log = LogFactory.getLog(UpdateUrisInIndex.class); - - /** - * Web service for update in search index of a list of URIs. - * @throws IOException - */ - protected void doUpdateUris(HttpServletRequest req, IndexBuilder builder) - throws ServletException, IOException{ - - if( ! ServletFileUpload.isMultipartContent(req) ) - throw new ServletException("Expected Multipart Content"); - - String enc = getEncoding(req); - try{ - //loop over the fileds and add any URIs to the IndexBuilder queue - ServletFileUpload upload = new ServletFileUpload(); - FileItemIterator iter = upload.getItemIterator(req); - while( iter.hasNext()){ - FileItemStream item = iter.next(); - String name = item.getFieldName(); - if( "email".equals(name) || "password".equals(name) ) - continue; //skip the password and email fields + private static final Log log = LogFactory.getLog(UpdateUrisInIndex.class); - InputStream stream = item.openStream(); - try{ - addToSearchQueue(builder, stream, enc); - }finally{ - stream.close(); - } - } - }catch (FileUploadException fex){ - throw new ServletException("Could not upload file to SearchServiceController", fex); - }finally{ - builder.doUpdateIndex(); - } - } + /** Pattern to split URIs on whitespace and commas. */ + public static final Pattern DELIMITER = Pattern.compile("[,\\s]+"); - /** - * Get the encoding of the request, default to UTF-8 - * since that is in the vitro install instructions - * to put on the connector. - */ - private String getEncoding(HttpServletRequest req){ - String enc = req.getCharacterEncoding(); - if( enc == null || enc.isEmpty() ){ - log.debug("No encoding on POST request, That is acceptable."); - enc = "UTF-8"; - }else if( enc.length() > 30){ - log.debug("Ignoring odd encoding of '" + enc + "'"); - enc = "UTF-8"; - }else{ - log.debug("Encoding set on POST request: " + enc); - } - log.debug("Reading POSTed URIs with encoding " + enc); - return enc; - } + /** + * Web service for update in search index of a list of URIs. + * + * @throws IOException + */ + protected int doUpdateUris(HttpServletRequest req, IndexBuilder builder) + throws ServletException, IOException { + @SuppressWarnings("unchecked") + Map> map = (Map>) req + .getAttribute(FILE_ITEM_MAP); + if (map == null) { + throw new ServletException("Expected Multipart Content"); + } + + Charset enc = getEncoding(req); - /** - * Adds URIs from Reader to search queue. - */ - private void addToSearchQueue( IndexBuilder builder, InputStream stream , String charEncoding ) - throws IOException{ + int uriCount = 0; + for (String name : map.keySet()) { + for (FileItem item : map.get(name)) { + log.debug("Found " + item.getSize() + " byte file for '" + name + "'"); + uriCount += processFileItem(builder, name, item, enc); + } + } + return uriCount; + } - Iterator uris = - new UrisFromInputIterator( new InputStreamReader(stream, charEncoding) ); + private int processFileItem(IndexBuilder builder, String name, + FileItem item, Charset enc) throws IOException { + int count = 0; + Reader reader = new InputStreamReader(item.getInputStream(), enc.name()); + try (Scanner scanner = createScanner(reader)) { + while (scanner.hasNext()) { + String uri = scanner.next(); + log.debug("Request to index uri '" + uri + "'"); + builder.addToChanged(uri); + count++; + } + } + return count; + } - while(uris.hasNext()){ - String uri = uris.next(); - log.debug("Request to index uri '" + uri + "'"); - builder.addToChanged( uri ); - } - } + @SuppressWarnings("resource") + protected Scanner createScanner(Reader in) { + return new Scanner(in).useDelimiter(DELIMITER); + } - - /** - * Iterator for URIs in a reader to make top level methods simpler. - */ - public static class UrisFromInputIterator implements Iterator { - BufferedReader reader; - Iterator uris; - - public UrisFromInputIterator(Reader in ){ - this.reader = new BufferedReader(in); - } - - public void remove(){ throw new UnsupportedOperationException() ; } - - public boolean hasNext(){ - if( uris != null && uris.hasNext() ){ - return true; - }else{ - try { - return getFromBuffer(); - } catch (IOException e) { - // TODO Auto-generated catch block - e.printStackTrace(); - return false; - } - } - } - - public String next(){ - return uris.next(); - } - - /** Returns true if there are uris to get. - * @throws IOException */ - private boolean getFromBuffer() throws IOException{ - uris = null; - - while( uris == null || !uris.hasNext() ){ - String chunk = reader.readLine(); - if( chunk == null ){ //at end of input - break; - } else if( chunk.trim().isEmpty() ){ - continue; - }else{ - uris = lineToUris(chunk).iterator(); - if( uris.hasNext() ){ - return true; - } - } - } - return false; - } - } - - /** - * Removes null and empty elements from in. - * Returned list will not be null. - */ - private static List removeNullAndEmpty(List in ){ - ArrayList out = new ArrayList(); - if( in == null ) - return out; - - for( String s : in ){ - if( s != null && !s.trim().isEmpty() ){ - out.add(s); - } - } - return out; - } - - /** - * Parses a line to a list of URIs. - * Retruned list will not be null. - * No elements in returned list will be empty or null. - */ - protected static List lineToUris(String line){ - List parts = removeNullAndEmpty( Arrays.asList(commaAndWhitespace.split( line ) )); - return parts; - } - - /** Pattern to split URIs on whitespace and commas. */ - private static final Pattern commaAndWhitespace = Pattern.compile("[,\\s]"); + /** + * Get the encoding of the request, default to UTF-8 since that is in the + * vitro install instructions to put on the connector. + */ + private Charset getEncoding(HttpServletRequest req) { + String enc = req.getCharacterEncoding(); + if (StringUtils.isBlank(enc)) { + log.debug("No encoding on POST request, That is acceptable."); + enc = "UTF-8"; + } else if (enc.length() > 30) { + log.debug("Ignoring odd encoding of '" + enc + "'"); + enc = "UTF-8"; + } else { + log.debug("Encoding set on POST request: " + enc); + } + log.debug("Reading POSTed URIs with encoding " + enc); + return Charset.forName(enc); + } } diff --git a/webapp/test/edu/cornell/mannlib/vitro/webapp/search/controller/UpdateUrisInIndexTest.java b/webapp/test/edu/cornell/mannlib/vitro/webapp/search/controller/UpdateUrisInIndexTest.java index 662c68e95..98780f1db 100644 --- a/webapp/test/edu/cornell/mannlib/vitro/webapp/search/controller/UpdateUrisInIndexTest.java +++ b/webapp/test/edu/cornell/mannlib/vitro/webapp/search/controller/UpdateUrisInIndexTest.java @@ -2,65 +2,88 @@ package edu.cornell.mannlib.vitro.webapp.search.controller; +import java.io.Reader; import java.io.StringReader; -import java.util.Arrays; -import java.util.Collections; import java.util.Iterator; import org.junit.Assert; import org.junit.Test; /** - * Accepts requests to update a set of URIs in the search index. + * Accepts requests to update a set of URIs in the search index. */ public class UpdateUrisInIndexTest { - @Test - public void lineToUrisTest(){ - Assert.assertEquals(Arrays.asList("uri1"), UpdateUrisInIndex.lineToUris( "uri1")); - Assert.assertEquals(Arrays.asList("uri1", "uri2"), UpdateUrisInIndex.lineToUris( "uri1,uri2")); - - Assert.assertEquals(Arrays.asList("uri1"), UpdateUrisInIndex.lineToUris( "uri1\n")); - Assert.assertEquals(Arrays.asList("uri1","uri2"), UpdateUrisInIndex.lineToUris( "uri1\nuri2")); - - Assert.assertEquals(Collections.EMPTY_LIST, UpdateUrisInIndex.lineToUris( "" )); - Assert.assertEquals(Collections.EMPTY_LIST, UpdateUrisInIndex.lineToUris( "," )); - Assert.assertEquals(Collections.EMPTY_LIST, UpdateUrisInIndex.lineToUris( " , " )); - } - - - @Test - public void UrisFromInputIteratorTest(){ - doUrisFromInputIterator("",0); - doUrisFromInputIterator(" ",0); - doUrisFromInputIterator(" , ",0); - doUrisFromInputIterator("\n",0); - doUrisFromInputIterator("\n\n\n",0); - doUrisFromInputIterator("http://bogus.com/n234",1); - doUrisFromInputIterator("http://bogus.com/n234\nhttp://bogus.com/n442",2); - doUrisFromInputIterator("http://bogus.com/n234, http://bogus.com/n442",2); - doUrisFromInputIterator("http://bogus.com/n234,\nhttp://bogus.com/n442\n",2); - - doUrisFromInputIterator("http://bogus.com/n234\n",1); - doUrisFromInputIterator("\nhttp://bogus.com/n234",1); - doUrisFromInputIterator("\nhttp://bogus.com/n234\n",1); - - } - - public void doUrisFromInputIterator(String input, int expectedUris){ - Iterator it = new UpdateUrisInIndex.UrisFromInputIterator( new StringReader(input) ); - int count = 0; - while( it.hasNext()){ - String uri = it.next(); - if( uri == null) - Assert.fail("UrisFromInputIterator should not return null strings \n " + - "Null string for uri #" + count + " for input '" + input + "'"); - if( uri.isEmpty()) - Assert.fail("UrisFromInputIterator should not return empty strings \n " + - "Empty string for uri #" + count + " for input '" + input + "'"); - count++; - } - Assert.assertEquals("Incorrect number of URIs from input '" + input + "'", expectedUris, count); - } - + @Test(expected = NullPointerException.class) + public void nullString() { + scan(null, 0); + } + + @Test + public void emptyString() { + scan("", 0); + } + + @Test + public void nothingButDelimiters() { + scan(" , ", 0); + scan("\n", 0); + scan("\n\n\n", 0); + scan("\n, \t\r ,\n\n", 0); + } + + @Test + public void oneTokenNoDelimiters() { + scan("http://bogus.com/n234", 1); + } + + @Test + public void oneTokenAssortedDelimiters() { + scan("http://bogus.com/n234\n", 1); + scan("\nhttp://bogus.com/n234", 1); + scan("\nhttp://bogus.com/n234\n", 1); + } + + @Test + public void twoTokensAssortedDelimiters() { + scan("http://bogus.com/n234\nhttp://bogus.com/n442", 2); + scan("http://bogus.com/n234, http://bogus.com/n442", 2); + scan("http://bogus.com/n234,\nhttp://bogus.com/n442\n", 2); + } + + @Test + public void nonBreakingSpace() { + scan("non\u00A0breaking\u00A0space", 1); + } + + @Test + public void omnibus() { + scan(" a , b,c d\t,\re", 5); + } + + // ---------------------------------------------------------------------- + // Helper methods + // ---------------------------------------------------------------------- + + public void scan(String input, int expectedUris) { + Reader reader = (input == null) ? null : new StringReader(input); + Iterator it = new UpdateUrisInIndex().createScanner(reader); + int count = 0; + while (it.hasNext()) { + String uri = it.next(); + if (uri == null) { + Assert.fail("Scanner should not return null strings \n " + + "Null string for uri #" + count + " for input '" + + input + "'"); + } else if (uri.isEmpty()) { + Assert.fail("Scanner should not return empty strings \n " + + "Empty string for uri #" + count + " for input '" + + input + "'"); + } + count++; + } + Assert.assertEquals("Incorrect number of URIs from input '" + input + + "'", expectedUris, count); + } + } diff --git a/webapp/web/templates/freemarker/body/search/searchService-help.ftl b/webapp/web/templates/freemarker/body/search/searchService-help.ftl index d1e61061c..d834370eb 100644 --- a/webapp/web/templates/freemarker/body/search/searchService-help.ftl +++ b/webapp/web/templates/freemarker/body/search/searchService-help.ftl @@ -12,9 +12,8 @@

This service will update the search index for the list of URIs it receives. It expectes a POST with an encoding of multpart/form-data. The service inspect all parts of this POST for -lists of URIs to reindex. The URIs should be comma or space -seperated. If no information can be found for a URI it will be -ignored.

+lists of URIs to reindex. The URIs should be separated by commas and/or white space. +If no information can be found for a URI it will be ignored.

The request parameters email and password allow the form to be submitted for a user account. Only internal accounts are supported, external authentication @@ -33,8 +32,8 @@ is not supported.

- - + + \ No newline at end of file