Alexander Wels has posted comments on this change.
Change subject: core: Locale docs servlet.
......................................................................
Patch Set 14: (34 inline comments)
....................................................
File backend/manager/modules/root/pom.xml
Line 21
Line 22
Line 23
Line 24
Line 25
Good point I don't know, I will put it back.
Line 66: <artifactId>commons-lang</artifactId>
Line 67: </dependency>
Line 68: <dependency>
Line 69: <groupId>org.mockito</groupId>
Line 70: <artifactId>mockito-all</artifactId>
I didn't know that at the time I added this.
Line 71: <version>${mockito.version}</version>
Line 72: <scope>test</scope>
Line 73: </dependency>
Line 74: <dependency>
Line 75: <groupId>commons-logging</groupId>
Line 76: <artifactId>commons-logging</artifactId>
Line 77: <version>1.1</version>
Line 78: <exclusions>
Line 79: <exclusion> <!-- declare the exclusion here -->
Since this was a relatively specific case, I didn't want to move this into the
root pom as it would affect other modules as well which might be using the
older servlet api.
Line 80: <groupId>javax.servlet</groupId>
Line 81: <artifactId>servlet-api</artifactId>
Line 82: </exclusion>
Line 83: </exclusions>
....................................................
File
backend/manager/modules/root/src/main/java/org/ovirt/engine/core/DocsServlet.java
Line 16: import org.ovirt.engine.core.utils.ServletUtils;
Line 17:
Line 18: /**
Line 19: * @author awels
Line 20: *
Done
Line 21: */
Line 22: public class DocsServlet extends FileServlet {
Line 23: // The log:
Line 24: private static final Logger log =
Logger.getLogger(FileServlet.class);
Line 33: private static final String LANG_JSP = "/WEB-INF/help/no_lang.jsp";
Line 34:
Line 35: @Override
Line 36: /**
Line 37: *
Done
Line 38: */
Line 39: protected void doGet(HttpServletRequest request,
HttpServletResponse response) throws ServletException, IOException {
Line 40: // Locate the requested file:
Line 41: File originalFile =
ServletUtils.makeFileFromSanePath(request.getPathInfo(), base);
Line 41: File originalFile =
ServletUtils.makeFileFromSanePath(request.getPathInfo(), base);
Line 42: Locale locale = getLocaleFromRequest(request);
Line 43: File file = determineActualFile(request, locale);
Line 44: file = checkForIndex(request, response, file,
request.getPathInfo());
Line 45: if (null == file) {
Done
Line 46: response.sendError(HttpServletResponse.SC_NOT_FOUND);
Line 47: } else if (!response.isCommitted()){ //If the response is
committed, we have already redirected.
Line 48: Object languagePageShown =
request.getSession(true).getAttribute(LANG_PAGE_SHOWN);
Line 49: if (!file.equals(originalFile)) {
Line 48: Object languagePageShown =
request.getSession(true).getAttribute(LANG_PAGE_SHOWN);
Line 49: if (!file.equals(originalFile)) {
Line 50: //We determined that we are going to redirect the user
to the English version URI.
Line 51: String redirect = request.getServletPath() +
replaceLocaleWithUSLocale(request.getPathInfo(), locale);
Line 52: if ((null == languagePageShown ||
!Boolean.parseBoolean(languagePageShown.toString()))) {
Done
Line 53:
request.getSession(true).setAttribute(LANG_PAGE_SHOWN, true);
Line 54: request.setAttribute(LOCALE, locale);
Line 55: request.setAttribute(ENGLISH_HREF, redirect);
Line 56: RequestDispatcher dispatcher =
request.getRequestDispatcher(LANG_JSP);
Line 72: /**
Line 73: *
Line 74: * @param request
Line 75: * @param locale
Line 76: * @return
Done
Line 77: */
Line 78: protected File determineActualFile(final HttpServletRequest
request, Locale locale) {
Line 79: File file =
ServletUtils.makeFileFromSanePath(request.getPathInfo(), base);
Line 80: // Check if file is found. If not found go ahead and try and
look up the English US locale version.
Line 77: */
Line 78: protected File determineActualFile(final HttpServletRequest
request, Locale locale) {
Line 79: File file =
ServletUtils.makeFileFromSanePath(request.getPathInfo(), base);
Line 80: // Check if file is found. If not found go ahead and try and
look up the English US locale version.
Line 81: if (null != file && !ServletUtils.canReadFile(file)) {
Done
Line 82: file =
ServletUtils.makeFileFromSanePath(replaceLocaleWithUSLocale(request.getPathInfo(),
locale), base);
Line 83: }
Line 84: return file;
Line 85: }
Line 104: */
Line 105: protected Locale getLocaleFromRequest(final HttpServletRequest
request) {
Line 106: String localeString = getLocaleStringFromReferer(request);
Line 107: //Unable to determine locale string from referer (preferred
way)
Line 108: if (null == localeString) {
Done
Line 109: //Note this fails if the is something like /menu.css
Line 110: localeString =
getLocaleStringFromPath(request.getPathInfo());
Line 111: }
Line 112: //Validate that the locale string is valid.
Line 123: * the found locale is a file and not a directory, return null as
the locale is a filename.
Line 124: */
Line 125: protected String getLocaleStringFromPath(final String path) {
Line 126: String result = null;
Line 127: if (null != path) {
Done
Line 128: if (!path.startsWith("/")) {
Line 129: throw new IllegalArgumentException("Path should start
with a '/'");
Line 130: }
Line 131: //Attempt to determine locale from path info.
Line 125: protected String getLocaleStringFromPath(final String path) {
Line 126: String result = null;
Line 127: if (null != path) {
Line 128: if (!path.startsWith("/")) {
Line 129: throw new IllegalArgumentException("Path should start
with a '/'");
Done
Line 130: }
Line 131: //Attempt to determine locale from path info.
Line 132: String[] pathElements = path.substring(1).split("/");
Line 133: File localeFile = new File(base, pathElements[0]);
Line 154: final URI refererURL;
Line 155: String result = null;
Line 156: try {
Line 157: String referer = request.getHeader(REFERER);
Line 158: if (null != referer) {
Done
Line 159: refererURL = new URI(referer);
Line 160: String query = refererURL.getQuery();
Line 161: if (null != query) {
Line 162: String[] parameters = query.split("&");
Line 156: try {
Line 157: String referer = request.getHeader(REFERER);
Line 158: if (null != referer) {
Line 159: refererURL = new URI(referer);
Line 160: String query = refererURL.getQuery();
Because the preferred way to get query parameters from a string is to use the
available tools namely URI, and use getQuery. This way we know it is done the
best way available to us. Doing an indexOf and substring may or may not work
(it probably will). But this way I KNOW it will work.
Line 161: if (null != query) {
Line 162: String[] parameters = query.split("&");
Line 163: for (int i = 0; i < parameters.length; i++) {
Line 164: String[] keyValues = parameters[i].split("=");
Line 157: String referer = request.getHeader(REFERER);
Line 158: if (null != referer) {
Line 159: refererURL = new URI(referer);
Line 160: String query = refererURL.getQuery();
Line 161: if (null != query) {
Done
Line 162: String[] parameters = query.split("&");
Line 163: for (int i = 0; i < parameters.length; i++) {
Line 164: String[] keyValues = parameters[i].split("=");
Line 165: if (LOCALE.equalsIgnoreCase(keyValues[0])) {
....................................................
File
backend/manager/modules/root/src/main/java/org/ovirt/engine/core/FileServlet.java
Line 91: protected void doGet(HttpServletRequest request,
HttpServletResponse response) throws ServletException, IOException {
Line 92: // Locate the requested file:
Line 93: File file =
ServletUtils.makeFileFromSanePath(request.getPathInfo(), base);
Line 94: file = checkForIndex(request, response, file,
request.getPathInfo());
Line 95: if (null == file) {
Done
Line 96: response.sendError(HttpServletResponse.SC_NOT_FOUND);
Line 97: } else {
Line 98: // Send the content of the file:
Line 99: // type is the default MIME type of the Servlet.
Line 103:
Line 104: protected File checkForIndex(HttpServletRequest request,
HttpServletResponse response, File file, String path) throws IOException {
Line 105: // If the requested file is a directory then try to replace
it with the
Line 106: // corresponding index page (if it exists):
Line 107: if (null != file && file.isDirectory()) {
Done
Line 108: File index = new File(file, INDEX);
Line 109: log.info("Index is \"" + index.getAbsolutePath() + "\".");
Line 110: if (index.isFile()) {
Line 111: String redirect = null;
....................................................
File
backend/manager/modules/root/src/test/java/org/ovirt/engine/core/DocsServletTest.java
Line 37: import org.ovirt.engine.core.utils.ServletUtils;
Line 38:
Line 39: /**
Line 40: * @author awels
Line 41: *
Done
Line 42: */
Line 43: @RunWith(MockitoJUnitRunner.class)
Line 44: public class DocsServletTest {
Line 45: DocsServlet testServlet;
Line 52: @Mock
Line 53: ServletConfig mockConfig;
Line 54:
Line 55: /**
Line 56: * @throws java.lang.Exception
Done
Line 57: */
Line 58: @Before
Line 59: public void setUp() throws Exception {
Line 60: testServlet = new DocsServlet();
Line 70: * @throws IOException
Line 71: * @throws ServletException
Line 72: */
Line 73: @Test
Line 74: public void testDoGetHttpServletRequestHttpServletResponseIndex()
throws ServletException, IOException {
Done
Line 75: //Because we would have found the index file, we do a redirect
and thus the response is committed.
Line 76: when(mockResponse.isCommitted()).thenReturn(Boolean.TRUE);
Line 77: when(mockRequest.getServletPath()).thenReturn("/docs");
Line 78: testServlet.doGet(mockRequest, mockResponse);
....................................................
File
backend/manager/modules/root/src/test/java/org/ovirt/engine/core/FileServletTest.java
Line 1: /**
Line 2: *
Done
Line 3: */
Line 4: package org.ovirt.engine.core;
Line 5:
Line 6: import static org.junit.Assert.assertEquals;
Line 33: import org.mockito.runners.MockitoJUnitRunner;
Line 34:
Line 35: /**
Line 36: * @author awels
Line 37: *
Done
Line 38: */
Line 39: @RunWith(MockitoJUnitRunner.class)
Line 40: public class FileServletTest {
Line 41: FileServlet testServlet;
Line 47: @Mock
Line 48: HttpServletResponse mockResponse;
Line 49:
Line 50: /**
Line 51: * @throws java.lang.Exception
Done
Line 52: */
Line 53: @Before
Line 54: public void setUp() throws Exception {
Line 55: testServlet = new FileServlet();
Line 61: * Test method for {@link
org.ovirt.engine.core.FileServlet#init(javax.servlet.ServletConfig)}.
Line 62: * @throws ServletException
Line 63: */
Line 64: @Test(expected=ServletException.class)
Line 65: public void testInitServletConfigNoInitParams() throws
ServletException {
Done
Line 66: testServlet.init(mockConfig);
Line 67: fail("Should not get here");
Line 68: }
Line 69:
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/LocaleUtils.java
Line 15: return getLocaleFromString(localeString, false);
Line 16: }
Line 17:
Line 18: /**
Line 19: *
Done
Line 20: * @param localeString
Line 21: * @param returnNull
Line 22: * @return
Line 23: */
Line 24: public static Locale getLocaleFromString(String localeString,
boolean returnDefaultLocale) {
Line 25: Locale result = returnDefaultLocale ? null : Locale.US;
Line 26: try {
Line 27: result =
org.apache.commons.lang.LocaleUtils.toLocale(localeString);
Line 28: if(null == result && returnDefaultLocale) {
Done
Line 29: result = Locale.US;
Line 30: }
Line 31: }
Line 32: catch(IllegalArgumentException e) {
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ServletUtils.java
Line 43: }
Line 44:
Line 45: // Anything longer than this is considered a large file and a
warning
Line 46: // will be generating when serving it:
Line 47: private static final long LARGE = 1048576; // 1 MiB
Because I copied a lot of this from another file.
Line 48:
Line 49: /**
Line 50: * Send a file to the output stream of the response passed into
the method.
Line 51: * @param request The {@code HttpServletRequest} so we can get the
path of the file.
Line 152:
Line 153: // All checks passed, the path is sane:
Line 154: return true;
Line 155: }
Line 156:
Done
Line 157: public static File makeFileFromSanePath(String path, File base) {
Line 158: File file = null;
Line 159:
Line 160: if (path == null) {
Line 153: // All checks passed, the path is sane:
Line 154: return true;
Line 155: }
Line 156:
Line 157: public static File makeFileFromSanePath(String path, File base) {
Well in this case we need to make it clear that we are doing the sanity check.
That is why I put it in the name.
Line 158: File file = null;
Line 159:
Line 160: if (path == null) {
Line 161: file = base;
....................................................
File
backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/LocaleUtilsTest.java
Line 9:
Line 10: public class LocaleUtilsTest {
Line 11:
Line 12: @Test
Line 13: public void testGetLocaleFromStringString() {
Done
Line 14: assertNull("The locale should be null",
LocaleUtils.getLocaleFromString(null));
Line 15: assertNull("The locale should be null",
LocaleUtils.getLocaleFromString("notalocale"));
Line 16: assertNull("The locale should be null",
LocaleUtils.getLocaleFromString("index.html"));
Line 17: assertEquals("The locale should be jp", Locale.JAPANESE,
LocaleUtils.getLocaleFromString("ja"));
....................................................
File
backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/ServletUtilsTest.java
Line 1: /**
Line 2: *
Done
Line 3: */
Line 4: package org.ovirt.engine.core.utils;
Line 5:
Line 6: import static org.junit.Assert.assertEquals;
Line 27: import org.junit.Test;
Line 28:
Line 29: /**
Line 30: * @author awels
Line 31: *
Done
Line 32: */
Line 33: public class ServletUtilsTest {
Line 34:
Line 35: /**
Line 32: */
Line 33: public class ServletUtilsTest {
Line 34:
Line 35: /**
Line 36: * @throws java.lang.Exception
Done
Line 37: */
Line 38: @Before
Line 39: public void setUp() throws Exception {
Line 40: }
Line 42: /**
Line 43: * Test method for {@link
org.ovirt.engine.core.ServletUtils#canReadFile(java.io.File)}.
Line 44: */
Line 45: @Test
Line 46: public void testCanReadFile() {
Done
Line 47: //Does not exist.
Line 48: File file = new File("/doesnotexist/iamprettysure");
Line 49: assertFalse("We should not be able to read this file.",
ServletUtils.canReadFile(file));
Line 50: //Exists, but should not be readable.
--
To view, visit http://gerrit.ovirt.org/10065
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I37156061cbdd7d2df3290c88dee933c41e0087c5
Gerrit-PatchSet: 14
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
