snoopdave commented on code in PR #233:
URL: https://github.com/apache/roller/pull/233#discussion_r4474331495
##########
app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileEdit.java:
##########
@@ -181,6 +216,42 @@ public String save() {
return INPUT;
}
+ public MediaFile getMediaFile() {
+ return mediaFile;
+ }
+
+ public String getReturnQuery() {
+ return returnQuery;
+ }
+
+ /**
+ * The library page to return to. Only the query string comes from the
+ * request, and only URL-safe characters are kept. getLibraryURL() sets
+ * the weblog itself, so the link always points at this weblog's library.
+ */
+ @StrutsParameter
+ public void setReturnQuery(String returnQuery) {
+ this.returnQuery = returnQuery != null && returnQuery.length() <= 2000
+ && returnQuery.matches("[A-Za-z0-9._~%&=+-]*") ? returnQuery :
null;
Review Comment:
🤖Claude: fixed in ea38279c6 by adding * to the allowed characters in
setReturnQuery(). Test: detailsPageKeepsOnlyAUrlSafeReturnQuery now covers
bean.name=IMG*.
##########
app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileAdd.java:
##########
@@ -191,50 +218,108 @@ public String save() {
if
(!WebloggerFactory.getWeblogger().getFileContentManager().canSave(
getActionWeblog(), fileName, declaredType,
uploadFile.length(), errors)) {
+ failed(fileName, messageText(errors));
continue;
}
mediaFile.setContentType(MediaTypePolicy.storedTypeFor(
mediaFile.getName(), declaredType,
RollerContext.getServletContext()::getMimeType));
manager.createMediaFile(getActionWeblog(), mediaFile,
errors);
+ if (errors.getErrorCount() > 0) {
+ failed(fileName, messageText(errors));
+ continue;
+ }
WebloggerFactory.getWeblogger().flush();
if (mediaFile.isImageFile()) {
newImages.add(mediaFile);
} else {
newFiles.add(mediaFile);
}
-
- uploaded.add(mediaFile);
+ uploadResults.add(UploadResult.uploaded(mediaFile));
} catch (Exception e) {
- log.error("Error uploading media file", e);
- addError("mediaFileAdd.errorUploading",
bean.getName());
+ log.error("Error uploading media file " + fileName, e);
+ failed(fileName, getText("mediaFileAdd.errorUploading",
+ Collections.singletonList(fileName)));
}
}
- for (Iterator<RollerMessage> it = errors.getErrors();
it.hasNext();) {
- RollerMessage msg = it.next();
- addError(msg.getKey(), Arrays.asList(msg.getArgs()));
+ if (newImages.isEmpty() && newFiles.isEmpty()) {
+ // nothing was saved; the errors are on the upload form
+ return result(INPUT);
}
- if (!uploaded.isEmpty() && !this.errorsExist()) {
- addMessage("uploadFiles.uploadedFiles");
- for (MediaFile upload : uploaded) {
- addMessage("uploadFiles.uploadedFile",
- upload.getPermalink());
- }
+ // Files that were saved are listed on the success page, with
+ // any that failed, so a partial upload is never hidden.
+ this.pageTitle = "mediaFileAddSuccess.title";
Review Comment:
🤖Claude: fixed in ea38279c6 by calling clearActionErrors() before the
success page, since it lists the failed files. The overlay upload keeps the
errors, because its success page has no failed list.
##########
app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/MediaFileDisplay.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. The ASF licenses this file to You
+ * under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License. For additional information regarding
+ * copyright in this work, please see the NOTICE file in the top level
+ * directory of this distribution.
+ */
+
+package org.apache.roller.weblogger.ui.struts2.editor;
+
+import java.util.Locale;
+
+import org.apache.commons.lang3.StringUtils;
+
+/**
+ * Short, human-readable labels for media files, shared by the media library
+ * and the editor's media chooser.
+ */
+public final class MediaFileDisplay {
+
+ private static final String[] UNITS = {"KB", "MB", "GB", "TB"};
+
+ private MediaFileDisplay() {
+ }
+
+ /**
+ * Size for display: "512 bytes", "1.5 KB", "23 MB". One decimal below 10
+ * of a unit, none above.
+ */
+ public static String formatSize(long bytes) {
+ if (bytes < 1024) {
+ return bytes == 1 ? "1 byte" : bytes + " bytes";
+ }
+ double value = bytes;
+ int unit = -1;
+ do {
+ value /= 1024;
+ unit++;
+ } while (value >= 1024 && unit < UNITS.length - 1);
Review Comment:
🤖Claude: fixed in ea38279c6 by moving to the next unit at 1023.5, the point
where %.0f rounds to 1024. Tests for 1048575 and 1073741823 bytes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
