Hi Vincent, On 09/13/2010 10:08 AM, Vincent Massol wrote:

Hi Marius,

I think there are some changes we need to discuss in this commit:

- new config api methods

I'll limit these to the XWiki specific implementation.

- new wiki model api methods

It's just one method: String getImageURL(String documentName, String attachmentName, Map<String, String> parameters) I don't know the rendering module that well to add methods by myself so as you can imagine I talked before with Thomas. In fact, Thomas gave me a patch of the rendering module to start with. As Thomas wrote in the javadoc, this method is useful because "in some implementation we want to make a distinction between displayed image and a simple link targeting an attachment file". Note that checking if the attachment is an image is not enough because sometimes you really want to make a link to an attached image without displaying it. So we needed a separate method. Also, we said that we are going to add this new method to WikiModel in http://lists.xwiki.org/pipermail/users/2010-September/021807.html (Thomas) and http://lists.xwiki.org/pipermail/devs/2010-September/019800.html (me)

- new deps introduced in the rendering module (makes it harder to use without maven)

It's just one new dependency: cssparser. Since this is not the only dependency of xwiki-rendering-xwiki I don't see why it makes xwiki-rendering-xwiki harder to use without maven. Wouldn't you handle this dependency outside maven as you handle the others? I discussed about adding this new dependency in the "Resize images on the server side to reduce page loading time" thread and Caleb was the only one who questioned this but he wasn't against it in the end. Thanks, Marius

These 2 are important as they are important public apis. The WikiModel one is the most important and we need to be 100% sure we want to add a new methods. There must absolutely be the minimal number of methods in this class. Also we're going to break people using the rendering module outside of xwiki (guys from eXo for example, so we need to be careful).

I haven't thought too much about it yet but my first feeling is that the methods added look quite specific. I'd like to have Thomas's opinion on this since he's the one who's the most knowledgeable about the rendering module.

Thanks -Vincent

On Sep 12, 2010, at 1:30 PM, mflorea (SVN) wrote:

...

Author: mflorea Date: 2010-09-12 13:30:15 +0200 (Sun, 12 Sep 2010) New Revision: 31053

Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/configuration/RenderingConfiguration.java platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/internal/configuration/DefaultRenderingConfiguration.java platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/internal/renderer/xhtml/DefaultXHTMLImageRenderer.java platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/wiki/WikiModel.java platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/pom.xml platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/main/java/org/xwiki/rendering/internal/configuration/XWikiRenderingConfiguration.java platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/main/java/org/xwiki/rendering/internal/wiki/XWikiWikiModel.java platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/test/java/org/xwiki/rendering/internal/configuration/XWikiRenderingConfigurationTest.java platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/test/java/org/xwiki/rendering/internal/wiki/XWikiWikiModelTest.java Log: XWIKI-5479: Include image dimensions in the image URL for server side image resizing

Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/configuration/RenderingConfiguration.java =================================================================== --- platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/configuration/RenderingConfiguration.java 2010-09-11 17:15:46 UTC (rev 31052) +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/configuration/RenderingConfiguration.java 2010-09-12 11:30:15 UTC (rev 31053) @@ -50,7 +50,7 @@ *</ul> * Note that if the page title is empty or not defined then it defaults to %p. This is also the case * if the title cannot be retrieved for the document. - * + * * The default is "%p". Some examples: "%s.%p", "%w:%s.%p". * * @return the format to use to display link labels when the user hasn't specified a label @@ -63,4 +63,42 @@ * descriptor (ie defined by the macro author) will be used */ Properties getMacroCategories(); + + /** + * @return {@code true} to include the image dimensions extracted from the image parameters in the image URL, + * {@code false} otherwise; when image dimensions are included in the image URL the image can be resized + * on the server side before being downloaded. + * @since 2.5M2 + */ + boolean isIncludeImageDimensionsInImageURL(); + + /** + * One way to improve page load speed is to resize images on the server side just before rendering the page. The + * rendering module can use the image width provided by the user to scale the image. When the user doesn't specify + * the image width the rendering module can limit the width of the image based on this configuration parameter. + *<p> + * The default value is {@code -1} which means image width is not limited by default. Use a value greater than 0 to + * limit the image width (pixels). Note that the aspect ratio is kept even when both the width and the height of the + * image are limited. + * + * @return the maximum image width when there's no user supplied width + * @see #isIncludeImageDimensionsInImageURL() + * @since 2.5M2 + */ + int getImageWidthLimit(); + + /** + * One way to improve page load speed is to resize images on the server side just before rendering the page. The + * rendering module can use the image height provided by the user to scale the image. When the user doesn't specify + * the image height the rendering module can limit the height of the image based on this configuration parameter. + *<p> + * The default value is {@code -1} which means image height is not limited by default. Use a value greater than 0 to + * limit the image height (pixels). Note that the aspect ratio is kept even when both the width and the height of + * the image are limited. + * + * @return the maximum image height when there's no user supplied height + * @see #isIncludeImageDimensionsInImageURL() + * @since 2.5M2 + */ + int getImageHeightLimit(); }

Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/internal/configuration/DefaultRenderingConfiguration.java =================================================================== --- platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/internal/configuration/DefaultRenderingConfiguration.java 2010-09-11 17:15:46 UTC (rev 31052) +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/internal/configuration/DefaultRenderingConfiguration.java 2010-09-12 11:30:15 UTC (rev 31053) @@ -45,6 +45,21 @@ private Properties macroCategories = new Properties();

/** + * @see #getImageWidthLimit() + */ + private int imageWidthLimit = -1; + + /** + * @see #getImageHeightLimit() + */ + private int imageHeightLimit = -1; + + /** + * @see #isIncludeImageDimensionsInImageURL() + */ + private boolean includeImageDimensionsInImageURL = true; + + /** * {@inheritDoc} * * @see org.xwiki.rendering.configuration.RenderingConfiguration#getLinkLabelFormat() @@ -84,4 +99,66 @@ // to work even without a configuration store. this.macroCategories.setProperty(macroId.toString(), category); } + + /** + * {@inheritDoc} + * + * @see org.xwiki.rendering.configuration.RenderingConfiguration#getImageWidthLimit() + */ + public int getImageWidthLimit() + { + return this.imageWidthLimit; + } + + /** + * @param imageWidthLimit the maximum image width when there's no user supplied width + */ + public void setImageWidthLimit(int imageWidthLimit) + { + // This method is useful for those using the XWiki Rendering in standalone mode since it allows the rendering + // to work even without a configuration store. + this.imageWidthLimit = imageWidthLimit; + } + + /** + * {@inheritDoc} + * + * @see org.xwiki.rendering.configuration.RenderingConfiguration#getImageHeightLimit() + */ + public int getImageHeightLimit() + { + return this.imageHeightLimit; + } + + /** + * @param imageHeightLimit the maximum image height when there's no user supplied height + */ + public void setImageHeightLimit(int imageHeightLimit) + { + // This method is useful for those using the XWiki Rendering in standalone mode since it allows the rendering + // to work even without a configuration store. + this.imageHeightLimit = imageHeightLimit; + } + + /** + * {@inheritDoc} + * + * @see org.xwiki.rendering.configuration.RenderingConfiguration#isIncludeImageDimensionsInImageURL() + */ + public boolean isIncludeImageDimensionsInImageURL() + { + return this.includeImageDimensionsInImageURL; + } + + /** + * @param includeImageDimensionsInImageURL {@code true} to include image dimensions extracted from the image + * parameters in the image URL so that the image can be resized on the server side before being + * downloaded, {@code false} otherwise + */ + public void setIncludeImageDimensionsInImageURL(boolean includeImageDimensionsInImageURL) + { + // This method is useful for those using the XWiki Rendering in standalone mode since it allows the rendering + // to work even without a configuration store. + this.includeImageDimensionsInImageURL = includeImageDimensionsInImageURL; + } }

Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/internal/renderer/xhtml/DefaultXHTMLImageRenderer.java =================================================================== --- platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/internal/renderer/xhtml/DefaultXHTMLImageRenderer.java 2010-09-11 17:15:46 UTC (rev 31052) +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/internal/renderer/xhtml/DefaultXHTMLImageRenderer.java 2010-09-12 11:30:15 UTC (rev 31053) @@ -42,10 +42,9 @@ * Default implementation for rendering images as XHTML. We handle both cases: *<ul> *<li>when inside a wiki (ie when an implementation of {@link WikiModel} is provided.</li> - *<li>when outside of a wiki. In this case we only handle external images and document images don't - * display anything.</li> + *<li>when outside of a wiki. In this case we only handle external images and document images don't display anything.</li> *</ul> - * + * * @version $Id$ * @since 2.0M3 */ @@ -68,6 +67,7 @@

/** * {@inheritDoc} + * * @see Initializable#initialize() */ public void initialize() throws InitializationException @@ -83,7 +83,7 @@

/** * {@inheritDoc} - * + * * @see XHTMLImageRenderer#setXHTMLWikiPrinter(XHTMLWikiPrinter) */ public void setXHTMLWikiPrinter(XHTMLWikiPrinter printer) @@ -103,7 +103,7 @@

/** * {@inheritDoc} - * + * * @see XHTMLImageRenderer#onImage(Image, boolean, Map) */ public void onImage(Image image, boolean isFreeStandingURI, Map<String, String> parameters) @@ -118,8 +118,9 @@ // that would not honor this contract. if (this.wikiModel != null) { DocumentImage documentImage = DocumentImage.class.cast(image); - imageURL = this.wikiModel.getAttachmentURL(documentImage.getDocumentName(), - documentImage.getAttachmentName()); + imageURL = + this.wikiModel.getImageURL(documentImage.getDocumentName(), documentImage.getAttachmentName(), + parameters); } else { throw new RuntimeException("Invalid Image type. In non wiki mode, all image types must be URL images."); }

Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/wiki/WikiModel.java =================================================================== --- platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/wiki/WikiModel.java 2010-09-11 17:15:46 UTC (rev 31052) +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/wiki/WikiModel.java 2010-09-12 11:30:15 UTC (rev 31053) @@ -19,13 +19,15 @@ */ package org.xwiki.rendering.wiki;

+import java.util.Map; + import org.xwiki.component.annotation.ComponentRole;

/** * Bridge between the Rendering module and a Wiki Model. Contains wiki APIs required by Rendering classes such as - * Renderers. For example the XHTML Link Renderer needs to know if a wiki document exists in order to know how to - * generate the HTML (in order to display a question mark for non existing documents) and it also needs to get the - * URL pointing the wiki document. + * Renderers. For example the XHTML Link Renderer needs to know if a wiki document exists in order to know how to + * generate the HTML (in order to display a question mark for non existing documents) and it also needs to get the URL + * pointing the wiki document. * * @version $Id$ * @since 2.0M1 @@ -34,36 +36,48 @@ public interface WikiModel { /** - * @param documentName the document name of the document containing the attachment. The syntax used depends on - * the underlying wiki system used. For example for XWiki a valid documentName would be - * {@code wiki:Space.Page} + * @param documentName the document name of the document containing the attachment. The syntax used depends on the + * underlying wiki system used. For example for XWiki a valid documentName would be + * {@code wiki:Space.Page} * @param attachmentName the name of the attachment in the passed document wikip page * @return the URL to the attachment */ String getAttachmentURL(String documentName, String attachmentName);

/** - * @param documentName the document name as a String. The syntax used depends on - * the underlying wiki system used. For example for XWiki a valid documentName would be - * {@code wiki:Space.Page} + * Generate image specific URL. The difference with {@link #getAttachmentURL(String, String)} is that in some + * implementation we want to make a distinction between displayed image and a simple link targeting an attachment + * file. + * + * @param documentName the document name of the document containing the attachment. The syntax used depends on the + * underlying wiki system used. For example for XWiki a valid documentName would be + * {@code wiki:Space.Page} + * @param attachmentName the name of the attachment in the passed document wiki page + * @param parameters custom parameters + * @return the URL to the image + * @since 2.5M2 + */ + String getImageURL(String documentName, String attachmentName, Map<String, String> parameters); + + /** + * @param documentName the document name as a String. The syntax used depends on the underlying wiki system used. + * For example for XWiki a valid documentName would be {@code wiki:Space.Page} * @return true if the document exists and can be viewed or false otherwise */ boolean isDocumentAvailable(String documentName);

/** - * @param documentName the document name as a String. The syntax used depends on - * the underlying wiki system used. For example for XWiki a valid documentName would be - * {@code wiki:Space.Page} + * @param documentName the document name as a String. The syntax used depends on the underlying wiki system used. + * For example for XWiki a valid documentName would be {@code wiki:Space.Page} * @param anchor an anchor pointing to some place inside the document or null * @param queryString a query string specifying some parameters or null * @return the URL to view the specified wiki document */ String getDocumentViewURL(String documentName, String anchor, String queryString); - + /** - * @param documentName the document name as a String. The syntax used depends on - * the underlying wiki system used. For example for XWiki a valid documentName would be - * {@code wiki:Space.Page} + * @param documentName the document name as a String. The syntax used depends on the underlying wiki system used. + * For example for XWiki a valid documentName would be {@code wiki:Space.Page} * @param anchor an anchor pointing to some place inside the document or null * @param queryString a query string specifying some parameters or null * @return the URL to edit the specified wiki document

Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/pom.xml =================================================================== --- platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/pom.xml 2010-09-11 17:15:46 UTC (rev 31052) +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/pom.xml 2010-09-12 11:30:15 UTC (rev 31053) @@ -54,6 +54,12 @@ <artifactId>xwiki-core-script</artifactId> <version>${pom.version}</version> </dependency> +<dependency> +<!-- For parsing the CSS from the style attribute. --> +<groupId>net.sourceforge.cssparser</groupId> +<artifactId>cssparser</artifactId> +<version>0.9.5</version> +</dependency> <!-- Testing dependencies --> <dependency> <groupId>org.xwiki.platform</groupId>

Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/main/java/org/xwiki/rendering/internal/configuration/XWikiRenderingConfiguration.java =================================================================== --- platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/main/java/org/xwiki/rendering/internal/configuration/XWikiRenderingConfiguration.java 2010-09-11 17:15:46 UTC (rev 31052) +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/main/java/org/xwiki/rendering/internal/configuration/XWikiRenderingConfiguration.java 2010-09-12 11:30:15 UTC (rev 31053) @@ -19,13 +19,13 @@ */ package org.xwiki.rendering.internal.configuration;

+import java.util.Properties; + import org.xwiki.component.annotation.Component; import org.xwiki.component.annotation.Requirement; import org.xwiki.configuration.ConfigurationSource; import org.xwiki.rendering.configuration.RenderingConfiguration;

-import java.util.Properties; - /** * All configuration options for the rendering subsystem. * @@ -46,7 +46,7 @@ private static final String DEFAULT_LINK_LABEL_FORMAT = "%p";

/** - * Defines from where to read the rendering configuration data. + * Defines from where to read the rendering configuration data. */ @Requirement private ConfigurationSource configuration; @@ -58,16 +58,46 @@ */ public String getLinkLabelFormat() { - return this.configuration.getProperty(PREFIX + "linkLabelFormat", DEFAULT_LINK_LABEL_FORMAT); + return this.configuration.getProperty(PREFIX + "linkLabelFormat", DEFAULT_LINK_LABEL_FORMAT); }

/** * {@inheritDoc} - * + * * @see org.xwiki.rendering.configuration.RenderingConfiguration#getMacroCategories() */ public Properties getMacroCategories() { return this.configuration.getProperty(PREFIX + "macroCategories", Properties.class); } + + /** + * {@inheritDoc} + * + * @see org.xwiki.rendering.configuration.RenderingConfiguration#getImageWidthLimit() + */ + public int getImageWidthLimit() + { + return this.configuration.getProperty(PREFIX + "imageWidthLimit", -1); + } + + /** + * {@inheritDoc} + * + * @see org.xwiki.rendering.configuration.RenderingConfiguration#getImageHeightLimit() + */ + public int getImageHeightLimit() + { + return this.configuration.getProperty(PREFIX + "imageHeightLimit", -1); + } + + /** + * {@inheritDoc} + * + * @see org.xwiki.rendering.configuration.RenderingConfiguration#isIncludeImageDimensionsInImageURL() + */ + public boolean isIncludeImageDimensionsInImageURL() + { + return this.configuration.getProperty(PREFIX + "includeImageDimensionsInImageURL", true); + } }

Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/main/java/org/xwiki/rendering/internal/wiki/XWikiWikiModel.java =================================================================== --- platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/main/java/org/xwiki/rendering/internal/wiki/XWikiWikiModel.java 2010-09-11 17:15:46 UTC (rev 31052) +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/main/java/org/xwiki/rendering/internal/wiki/XWikiWikiModel.java 2010-09-12 11:30:15 UTC (rev 31053) @@ -19,17 +19,25 @@ */ package org.xwiki.rendering.internal.wiki;

+import java.io.IOException; +import java.io.StringReader; import java.io.UnsupportedEncodingException; import java.net.URLEncoder; +import java.util.Map;

import org.apache.commons.lang.StringUtils; +import org.w3c.css.sac.InputSource; +import org.w3c.dom.css.CSSStyleDeclaration; import org.xwiki.bridge.DocumentAccessBridge; -import org.xwiki.model.reference.DocumentReference; import org.xwiki.component.annotation.Component; import org.xwiki.component.annotation.Requirement; +import org.xwiki.model.reference.DocumentReference; import org.xwiki.model.reference.EntityReferenceSerializer; +import org.xwiki.rendering.configuration.RenderingConfiguration; import org.xwiki.rendering.wiki.WikiModel;

+import com.steadystate.css.parser.CSSOMParser; + /** * Implementation using the Document Access Bridge ({@link DocumentAccessBridge}). * @@ -39,14 +47,47 @@ @Component public class XWikiWikiModel implements WikiModel { + /** + * The suffix used to mark an amount of pixels. + */ + private static final String PIXELS = "px"; + + /** + * The name of the {@code width} image parameter. + */ + private static final String WIDTH = "width"; + + /** + * The name of the {@code height} image parameter. + */ + private static final String HEIGHT = "height"; + + /** + * The component used to access configuration parameters. + */ @Requirement + private RenderingConfiguration renderingConfiguration; + + /** + * The component used to access the underlying XWiki model. + */ + @Requirement private DocumentAccessBridge documentAccessBridge;

+ /** + * The component used to serialize entity references to strings. + */ @Requirement private EntityReferenceSerializer<String> entityReferenceSerializer;

/** + * The object used to parse the CSS from the image style parameter. + */ + private final CSSOMParser cssParser = new CSSOMParser(); + + /** * {@inheritDoc} + * * @see WikiModel#getAttachmentURL(String, String) */ public String getAttachmentURL(String documentName, String attachmentName) @@ -55,7 +96,112 @@ }

/** + * Extracts the specified image dimension from the image parameters. + * + * @param dimension either {@code width} or {@code height} + * @param imageParameters the image parameters; may include the {@code width}, {@code height} and {@code style} + * parameters + * @return the value of the passed dimension if it is specified in the image parameters, {@code null} otherwise + */ + private String getImageDimension(String dimension, Map<String, String> imageParameters) + { + // Check first if the style parameter contains information about the given dimension. In-line style has priority + // over the dimension parameters. + String value = null; + String style = imageParameters.get("style"); + if (StringUtils.isNotBlank(style)) { + try { + CSSStyleDeclaration sd = cssParser.parseStyleDeclaration(new InputSource(new StringReader(style))); + value = sd.getPropertyValue(dimension); + } catch (IOException e) { + // Ignore the style parameter. + } + } + if (StringUtils.isBlank(value)) { + // Fall back on the value of the dimension parameter. + value = imageParameters.get(dimension); + } + return value; + } + + /** + * Creates the query string that can be added to an image URL to resize the image on the server side. + * + * @param imageParameters image parameters, including width and height then they are specified + * @return the query string to be added to an image URL in order to resize the image on the server side + */ + private StringBuilder getImageURLQueryString(Map<String, String> imageParameters) + { + String width = StringUtils.chomp(getImageDimension(WIDTH, imageParameters), PIXELS); + String height = StringUtils.chomp(getImageDimension(HEIGHT, imageParameters), PIXELS); + boolean useHeight = StringUtils.isNotEmpty(height)&& StringUtils.isNumeric(height); + StringBuilder queryString = new StringBuilder(); + if (StringUtils.isEmpty(width) || !StringUtils.isNumeric(width)) { + // Width is unspecified or is not measured in pixels. + if (useHeight) { + // Height is specified in pixels. + queryString.append('&').append(HEIGHT).append('=').append(height); + } else { + // If image width and height are unspecified or if they are not expressed in pixels then limit the image + // size to best fit the rectangle specified in the configuration (keeping aspect ratio). + int widthLimit = renderingConfiguration.getImageWidthLimit(); + if (widthLimit> 0) { + queryString.append('&').append(WIDTH).append('=').append(widthLimit); + } + int heightLimit = renderingConfiguration.getImageHeightLimit(); + if (heightLimit> 0) { + queryString.append('&').append(HEIGHT).append('=').append(heightLimit); + } + if (widthLimit> 0&& heightLimit> 0) { + queryString.append("&keepAspectRatio=").append(true); + } + } + } else { + // Width is specified in pixels. + queryString.append('&').append(WIDTH).append('=').append(width); + if (useHeight) { + // Height is specified in pixels. + queryString.append('&').append(HEIGHT).append('=').append(height); + } + } + return queryString; + } + + /** * {@inheritDoc} + * + * @see org.xwiki.rendering.wiki.WikiModel#getImageURL(java.lang.String, java.lang.String, java.util.Map) + */ + public String getImageURL(String documentName, String attachmentName, Map<String, String> parameters) + { + String url = getAttachmentURL(documentName, attachmentName); + if (!renderingConfiguration.isIncludeImageDimensionsInImageURL()) { + return url; + } + + StringBuilder queryString = getImageURLQueryString(parameters); + if (queryString.length() == 0) { + return url; + } + + // Determine the insertion point. + int insertionPoint = url.lastIndexOf('#'); + if (insertionPoint< 0) { + // No fragment identifier. + insertionPoint = url.length(); + } + if (url.lastIndexOf('?', insertionPoint)< 0) { + // No query string. + queryString.setCharAt(0, '?'); + } + + // Insert the query string. + return new StringBuilder(url).insert(insertionPoint, queryString).toString(); + } + + /** + * {@inheritDoc} + * * @see WikiModel#isDocumentAvailable(String) */ public boolean isDocumentAvailable(String documentName) @@ -65,6 +211,7 @@

/** * {@inheritDoc} + * * @see WikiModel#getDocumentViewURL(String, String, String) */ public String getDocumentViewURL(String documentName, String anchor, String queryString) @@ -74,6 +221,7 @@

/** * {@inheritDoc} + * * @see WikiModel#getDocumentEditURL(String, String, String) */ public String getDocumentEditURL(String documentName, String anchor, String queryString) @@ -89,17 +237,17 @@ // See http://www.w3.org/TR/html40/appendix/notes.html#non-ascii-chars // TODO: Once the xwiki-url module is usable, refactor this code to use it and remove the need to // perform explicit encoding here. - modifiedQueryString = + modifiedQueryString = "parent=" + URLEncoder.encode(this.entityReferenceSerializer.serialize(reference), "UTF-8"); } catch (UnsupportedEncodingException e) { // Not supporting UTF-8 as a valid encoding for some reasons. We consider XWiki cannot work // without that encoding. - throw new RuntimeException("Failed to URL encode [" + this.entityReferenceSerializer.serialize( - reference) + "] using UTF-8.", e); + throw new RuntimeException("Failed to URL encode [" + + this.entityReferenceSerializer.serialize(reference) + "] using UTF-8.", e); } } } - + return this.documentAccessBridge.getURL(documentName, "create", modifiedQueryString, anchor); } }

Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/test/java/org/xwiki/rendering/internal/configuration/XWikiRenderingConfigurationTest.java =================================================================== --- platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/test/java/org/xwiki/rendering/internal/configuration/XWikiRenderingConfigurationTest.java 2010-09-11 17:15:46 UTC (rev 31052) +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/test/java/org/xwiki/rendering/internal/configuration/XWikiRenderingConfigurationTest.java 2010-09-12 11:30:15 UTC (rev 31053) @@ -22,9 +22,9 @@ import java.util.Properties;

import org.jmock.Expectations; -import org.xwiki.configuration.ConfigurationSource; import org.junit.Assert; import org.junit.Test; +import org.xwiki.configuration.ConfigurationSource; import org.xwiki.test.AbstractMockingComponentTestCase; import org.xwiki.test.annotation.MockingRequirement;

@@ -43,10 +43,13 @@ public void testGetLinkLabelFormat() throws Exception { final ConfigurationSource source = getComponentManager().lookup(ConfigurationSource.class); - getMockery().checking(new Expectations() {{ - allowing(source).getProperty("rendering.linkLabelFormat", "%p"); + getMockery().checking(new Expectations() + { + { + allowing(source).getProperty("rendering.linkLabelFormat", "%p"); will(returnValue("%p")); - }}); + } + });

Assert.assertEquals("%p", this.configuration.getLinkLabelFormat()); } @@ -55,12 +58,60 @@ public void testGetMacroCategories() throws Exception { final ConfigurationSource source = getComponentManager().lookup(ConfigurationSource.class); - getMockery().checking(new Expectations() {{ - allowing(source).getProperty("rendering.macroCategories", Properties.class); + getMockery().checking(new Expectations() + { + { + allowing(source).getProperty("rendering.macroCategories", Properties.class); will(returnValue(new Properties())); - }}); + } + });

Assert.assertNotNull(this.configuration.getMacroCategories()); Assert.assertEquals(0, this.configuration.getMacroCategories().size()); } + + @Test + public void testGetImageWidthLimit() throws Exception + { + final ConfigurationSource source = getComponentManager().lookup(ConfigurationSource.class); + getMockery().checking(new Expectations() + { + { + allowing(source).getProperty("rendering.imageWidthLimit", -1); + will(returnValue(100)); + } + }); + + Assert.assertEquals(100, this.configuration.getImageWidthLimit()); + } + + @Test + public void testGetImageHeightLimit() throws Exception + { + final ConfigurationSource source = getComponentManager().lookup(ConfigurationSource.class); + getMockery().checking(new Expectations() + { + { + allowing(source).getProperty("rendering.imageHeightLimit", -1); + will(returnValue(150)); + } + }); + + Assert.assertEquals(150, this.configuration.getImageHeightLimit()); + } + + @Test + public void testIsIncludeImageDimensionsInImageURL() throws Exception + { + final ConfigurationSource source = getComponentManager().lookup(ConfigurationSource.class); + getMockery().checking(new Expectations() + { + { + allowing(source).getProperty("rendering.includeImageDimensionsInImageURL", true); + will(returnValue(false)); + } + }); + + Assert.assertFalse(this.configuration.isIncludeImageDimensionsInImageURL()); + } }

Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/test/java/org/xwiki/rendering/internal/wiki/XWikiWikiModelTest.java =================================================================== --- platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/test/java/org/xwiki/rendering/internal/wiki/XWikiWikiModelTest.java 2010-09-11 17:15:46 UTC (rev 31052) +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-xwiki/src/test/java/org/xwiki/rendering/internal/wiki/XWikiWikiModelTest.java 2010-09-12 11:30:15 UTC (rev 31053) @@ -19,12 +19,20 @@ */ package org.xwiki.rendering.internal.wiki;

+import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +import junit.framework.Assert; + import org.jmock.Expectations; -import org.jmock.Mockery; +import org.junit.Test; import org.xwiki.bridge.DocumentAccessBridge; import org.xwiki.model.reference.DocumentReference; -import org.xwiki.component.util.ReflectionUtils; import org.xwiki.model.reference.EntityReferenceSerializer; +import org.xwiki.rendering.configuration.RenderingConfiguration; +import org.xwiki.test.AbstractMockingComponentTestCase; +import org.xwiki.test.annotation.MockingRequirement;

/** * Unit tests for {@link XWikiWikiModel}. @@ -32,31 +40,321 @@ * @version $Id$ * @since 2.0M1 */ -public class XWikiWikiModelTest +public class XWikiWikiModelTest extends AbstractMockingComponentTestCase { - @org.junit.Test + @MockingRequirement + private XWikiWikiModel wikiModel; + + @Test public void testGetDocumentEditURLWhenNoQueryStringSpecified() throws Exception { - Mockery mockery = new Mockery(); - XWikiWikiModel wikiModel = new XWikiWikiModel(); + final EntityReferenceSerializer< ?> entityReferenceSerializer = + getComponentManager().lookup(EntityReferenceSerializer.class); + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final DocumentReference documentReference = new DocumentReference("wiki", "Space", "Page"); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getCurrentDocumentReference(); + will(returnValue(documentReference)); + oneOf(entityReferenceSerializer).serialize(documentReference); + will(returnValue("wiki:Space.Page\u20AC"));

- final EntityReferenceSerializer mockEntityReferenceSerializer = mockery.mock(EntityReferenceSerializer.class); - ReflectionUtils.setFieldValue(wikiModel, "entityReferenceSerializer", mockEntityReferenceSerializer); + // The test is here: we verify that getURL is called with the query string already encoded since + // getURL() doesn't encode it. + oneOf(documentAccessBridge).getURL("Space.Page\u20AC", "create", "parent=wiki%3ASpace.Page%E2%82%AC", + "anchor"); + } + });

- final DocumentAccessBridge mockDocumentAccessBridge = mockery.mock(DocumentAccessBridge.class); - ReflectionUtils.setFieldValue(wikiModel, "documentAccessBridge", mockDocumentAccessBridge); + wikiModel.getDocumentEditURL("Space.Page\u20AC", "anchor", null); + }

- final DocumentReference docReference = new DocumentReference("wiki", "Space", "Page"); - mockery.checking(new Expectations() {{ - oneOf(mockDocumentAccessBridge).getCurrentDocumentReference(); will(returnValue(docReference)); - oneOf(mockEntityReferenceSerializer).serialize(docReference); will(returnValue("wiki:Space.Page\u20AC")); + /** + * Tests that the proper image URL is generated when both the width and the height image parameters are specified. + * + * @throws Exception if an exception occurs while running the test + */ + @Test + public void testGetImageURLWhenBothWidthAndHeightAttributesAreSpecified() throws Exception + { + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final RenderingConfiguration configuration = getComponentManager().lookup(RenderingConfiguration.class); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getAttachmentURL("", ""); + will(returnValue("")); + oneOf(configuration).isIncludeImageDimensionsInImageURL(); + will(returnValue(true)); + } + }); + Map<String, String> parameters = new HashMap<String, String>(); + parameters.put("width", "100px"); + parameters.put("height", "50"); + Assert.assertEquals("?width=100&height=50", wikiModel.getImageURL("", "", parameters)); + }

- // The test is here: we verify that getURL is called with the query string already encoded since getURL() - // doesn't encode it. - oneOf(mockDocumentAccessBridge).getURL("Space.Page\u20AC", "create", "parent=wiki%3ASpace.Page%E2%82%AC", - "anchor"); - }}); + /** + * Tests that the proper image URL is generated when both the width and the height image parameters are specified + * but including them in the image URL is forbidden from the rendering configuration. + * + * @throws Exception if an exception occurs while running the test + */ + @Test + public void testGetImageURLWhenIncludingImageDimensionsIsForbidden() throws Exception + { + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final RenderingConfiguration configuration = getComponentManager().lookup(RenderingConfiguration.class); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getAttachmentURL("", ""); + will(returnValue("")); + oneOf(configuration).isIncludeImageDimensionsInImageURL(); + will(returnValue(false)); + } + }); + Map<String, String> parameters = new HashMap<String, String>(); + parameters.put("width", "101px"); + parameters.put("height", "55px"); + Assert.assertEquals("", wikiModel.getImageURL("", "", parameters)); + }

- wikiModel.getDocumentEditURL("Space.Page\u20AC", "anchor", null); + /** + * Tests that the proper image URL is generated when both the width and the height CSS properties are specified. + * + * @throws Exception if an exception occurs while running the test + */ + @Test + public void testGetImageURLWhenBothWidthAndHeightCSSPropertiesAreSpecified() throws Exception + { + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final RenderingConfiguration configuration = getComponentManager().lookup(RenderingConfiguration.class); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getAttachmentURL("", ""); + will(returnValue("")); + oneOf(configuration).isIncludeImageDimensionsInImageURL(); + will(returnValue(true)); + } + }); + Map<String, String> parameters = new HashMap<String, String>(); + parameters.put("style", "border: 1px; height: 30px; margin-top: 2em; width: 70px"); + Assert.assertEquals("?width=70&height=30", wikiModel.getImageURL("", "", parameters)); } + + /** + * Tests that the proper image URL is generated when only the width image parameter is specified. + * + * @throws Exception if an exception occurs while running the test + */ + @Test + public void testGetImageURLWhenOnlyWidthAttributeIsSpecified() throws Exception + { + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final RenderingConfiguration configuration = getComponentManager().lookup(RenderingConfiguration.class); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getAttachmentURL("", ""); + will(returnValue("")); + oneOf(configuration).isIncludeImageDimensionsInImageURL(); + will(returnValue(true)); + } + }); + Map<String, String> parameters = new HashMap<String, String>(); + parameters.put("width", "150"); + parameters.put("height", "30%"); + Assert.assertEquals("?width=150", wikiModel.getImageURL("", "", parameters)); + } + + /** + * Tests that the proper image URL is generated when only the height CSS property is specified. + * + * @throws Exception if an exception occurs while running the test + */ + @Test + public void testGetImageURLWhenOnlyHeightCSSPropertyIsSpecified() throws Exception + { + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final RenderingConfiguration configuration = getComponentManager().lookup(RenderingConfiguration.class); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getAttachmentURL("", ""); + will(returnValue("")); + oneOf(configuration).isIncludeImageDimensionsInImageURL(); + will(returnValue(true)); + } + }); + Map<String, String> parameters = new HashMap<String, String>(); + parameters.put("style", "width: 5cm; height: 80px"); + Assert.assertEquals("?height=80", wikiModel.getImageURL("", "", parameters)); + } + + /** + * Tests that the proper image URL is generated when both the width and the height are unspecified and image size is + * limited in the configuration. + * + * @throws Exception if an exception occurs while running the test + */ + @Test + public void testGetImageURLWhenBothWidthAndHeightAreUnspecifiedAndImageSizeIsLimited() throws Exception + { + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final RenderingConfiguration configuration = getComponentManager().lookup(RenderingConfiguration.class); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getAttachmentURL("", ""); + will(returnValue("")); + oneOf(configuration).isIncludeImageDimensionsInImageURL(); + will(returnValue(true)); + oneOf(configuration).getImageWidthLimit(); + will(returnValue(200)); + oneOf(configuration).getImageHeightLimit(); + will(returnValue(170)); + } + }); + Map<String, String> parameters = Collections.emptyMap(); + Assert.assertEquals("?width=200&height=170&keepAspectRatio=true", wikiModel.getImageURL("", "", parameters)); + } + + /** + * Tests that the proper image URL is generated when both the width and the height are unspecified and only the + * image width is limited in the configuration. + * + * @throws Exception if an exception occurs while running the test + */ + @Test + public void testGetImageURLWhenBothWidthAndHeightAreUnspecifiedAndOnlyImageWidthIsLimited() throws Exception + { + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final RenderingConfiguration configuration = getComponentManager().lookup(RenderingConfiguration.class); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getAttachmentURL("", ""); + will(returnValue("")); + oneOf(configuration).isIncludeImageDimensionsInImageURL(); + will(returnValue(true)); + oneOf(configuration).getImageWidthLimit(); + will(returnValue(25)); + oneOf(configuration).getImageHeightLimit(); + will(returnValue(-1)); + } + }); + Map<String, String> parameters = new HashMap<String, String>(); + parameters.put("width", "45%"); + parameters.put("style", "height:10em"); + Assert.assertEquals("?width=25", wikiModel.getImageURL("", "", parameters)); + } + + /** + * Tests that the proper image URL is generated when both the width and the height are unspecified and the image + * size is not limited in the configuration. + * + * @throws Exception if an exception occurs while running the test + */ + @Test + public void testGetImageURLWhenBothWidthAndHeightAreUnspecifiedAndImageSizeIsNotLimited() throws Exception + { + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final RenderingConfiguration configuration = getComponentManager().lookup(RenderingConfiguration.class); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getAttachmentURL("", ""); + will(returnValue("")); + oneOf(configuration).isIncludeImageDimensionsInImageURL(); + will(returnValue(true)); + oneOf(configuration).getImageWidthLimit(); + will(returnValue(-1)); + oneOf(configuration).getImageHeightLimit(); + will(returnValue(-1)); + } + }); + Map<String, String> parameters = new HashMap<String, String>(); + parameters.put("style", "bad CSS declaration"); + Assert.assertEquals("", wikiModel.getImageURL("", "", parameters)); + } + + /** + * Tests that the proper image URL is generated when the attachment URL has a fragment identifier. + * + * @throws Exception if an exception occurs while running the test + */ + @Test + public void testGetImageURLWhenAttachmentURLHasFragmentIdentifier() throws Exception + { + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final RenderingConfiguration configuration = getComponentManager().lookup(RenderingConfiguration.class); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getAttachmentURL("", ""); + will(returnValue("test#fragment")); + oneOf(configuration).isIncludeImageDimensionsInImageURL(); + will(returnValue(true)); + } + }); + Map<String, String> parameters = new HashMap<String, String>(); + parameters.put("width", "23"); + Assert.assertEquals("test?width=23#fragment", wikiModel.getImageURL("", "", parameters)); + } + + /** + * Tests that the proper image URL is generated when the attachment URL has a query string and a fragment + * identifier. + * + * @throws Exception if an exception occurs while running the test + */ + @Test + public void testGetImageURLWhenAttachmentURLHasQueryStringAndFragmentIdentifier() throws Exception + { + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final RenderingConfiguration configuration = getComponentManager().lookup(RenderingConfiguration.class); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getAttachmentURL("", ""); + will(returnValue("test?param=value#fragment")); + oneOf(configuration).isIncludeImageDimensionsInImageURL(); + will(returnValue(true)); + } + }); + Map<String, String> parameters = new HashMap<String, String>(); + parameters.put("height", "17"); + Assert.assertEquals("test?param=value&height=17#fragment", wikiModel.getImageURL("", "", parameters)); + } + + /** + * Tests that the proper image URL is generated when both the style and the dimension parameters are specified. + * + * @throws Exception if an exception occurs while running the test + */ + @Test + public void testGetImageURLWhenBothStyleAndDimensionParametersAreSpecified() throws Exception + { + final DocumentAccessBridge documentAccessBridge = getComponentManager().lookup(DocumentAccessBridge.class); + final RenderingConfiguration configuration = getComponentManager().lookup(RenderingConfiguration.class); + getMockery().checking(new Expectations() + { + { + oneOf(documentAccessBridge).getAttachmentURL("", ""); + will(returnValue("")); + oneOf(configuration).isIncludeImageDimensionsInImageURL(); + will(returnValue(true)); + } + }); + Map<String, String> parameters = new HashMap<String, String>(); + parameters.put("height", "46"); + parameters.put("width", "101px"); + parameters.put("style", "width: 20%; height:75px"); + // Note that the style parameter take precedence over the dimension parameters and the width is actually 20% but + // we can't use it for resizing the image on the server side so it's omitted from the query string. + Assert.assertEquals("?height=75", wikiModel.getImageURL("", "", parameters)); + } }

_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs