Skip to content

Commit dedc5f4

Browse files
committed
fix(image): populate temp metadata on image-tool save and expose focalPoint on read
The _imageToolSaveFile save block created the temp from the empty file, so it came back with metadata:null/image:false; re-wrap it after copying the rendered bytes so it carries real metadata (mirrors TempFileAPI.createTempFile). Expose focalPoint on DefaultTransformStrategy.addBinaries (mirrors BinaryViewStrategy) so the editor can re-seed the marker on reopen.
1 parent 96414df commit dedc5f4

3 files changed

Lines changed: 134 additions & 2 deletions

File tree

dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/transform/strategy/DefaultTransformStrategy.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import com.dotmarketing.business.IdentifierAPI;
1515
import com.dotmarketing.exception.DotDataException;
1616
import com.dotmarketing.exception.DotSecurityException;
17+
import com.dotmarketing.image.focalpoint.FocalPointAPI;
1718
import com.dotmarketing.portlets.categories.model.Category;
1819
import com.dotmarketing.portlets.contentlet.model.Contentlet;
1920
import com.dotmarketing.portlets.contentlet.model.ContentletVersionInfo;
@@ -264,9 +265,16 @@ private void addBinaries(final Contentlet contentlet, final Map<String, Object>
264265
if (null != metadata) {
265266
Map<String, Serializable> metaMap = new HashMap<>(metadata.getMap());
266267
metaMap.remove("path");
268+
// Focal point lives in the binary's custom metadata under a prefixed key, so it
269+
// is not exposed under the clean "focalPoint" key by metadata.getMap(). Surface
270+
// it explicitly so the image editor can re-seed its marker when reopened.
271+
final String focalPoint = Try.of(() ->
272+
metadata.getCustomMeta().getOrDefault(FocalPointAPI.FOCAL_POINT, "0.0").toString()
273+
).getOrElse("0.0");
274+
metaMap.put(FocalPointAPI.FOCAL_POINT, focalPoint);
267275
map.put(velocityVarName + "MetaData", metaMap);
268276
putBinaryLinks(velocityVarName, metadata.getName(), contentlet, map);
269-
}
277+
}
270278
} catch (final Exception e) {
271279
Logger.warn(this,
272280
String.format("An error occurred when retrieving the Binary file from field"

dotCMS/src/main/java/com/dotmarketing/servlets/BinaryExporterServlet.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,10 +412,17 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws Servl
412412
// THIS IS WHERE THE MAGIC HAPPENS
413413
// this creates a temp resource using the altered file
414414
if (req.getParameter(WebKeys.IMAGE_TOOL_SAVE_FILES) != null && user!=null && !user.equals(APILocator.getUserAPI().getAnonymousUser())) {
415-
final DotTempFile temp = tempFileAPI.createEmptyTempFile(inputFile.getName(), req);
415+
DotTempFile temp = tempFileAPI.createEmptyTempFile(inputFile.getName(), req);
416416
FileUtil.copyFile(data.getDataFile(), temp.file);
417417
//Temp files time-mark must be updated so they can be recognized by the tempFileAPI
418418
temp.file.setLastModified(System.currentTimeMillis());
419+
// createEmptyTempFile derives metadata from the still-empty file, so the rendered
420+
// image would otherwise come back as metadata:null/image:false/mimeType:unknown.
421+
// Re-wrap to regenerate metadata from the now-populated file (mirrors
422+
// TempFileAPI.createTempFile) so consumers get a usable image preview.
423+
if (temp.metadata == null && temp.file.exists()) {
424+
temp = new DotTempFile(temp.id, temp.file);
425+
}
419426
copyMetadata(uuid, fieldVarName, temp);
420427
resp.getWriter().println(DotObjectMapperProvider.getInstance().getDefaultObjectMapper().writeValueAsString(temp));
421428
resp.getWriter().close();
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
package com.dotmarketing.portlets.contentlet.transform.strategy;
2+
3+
import static com.dotmarketing.portlets.contentlet.transform.strategy.TransformOptions.BINARIES;
4+
import static org.junit.Assert.assertEquals;
5+
import static org.junit.Assert.assertTrue;
6+
7+
import com.dotcms.api.APIProvider;
8+
import com.dotcms.contenttype.model.field.BinaryField;
9+
import com.dotcms.contenttype.model.field.Field;
10+
import com.dotcms.contenttype.model.type.ContentType;
11+
import com.dotcms.storage.model.Metadata;
12+
import com.dotmarketing.image.focalpoint.FocalPointAPI;
13+
import com.dotmarketing.portlets.contentlet.model.Contentlet;
14+
import java.io.Serializable;
15+
import java.lang.reflect.Method;
16+
import java.util.HashMap;
17+
import java.util.List;
18+
import java.util.Map;
19+
import java.util.Set;
20+
import org.junit.Test;
21+
import org.mockito.Mockito;
22+
23+
public class DefaultTransformStrategyTest {
24+
25+
private static final String FIELD_VAR = "fileField";
26+
private static final String META_KEY = FIELD_VAR + "MetaData";
27+
28+
/**
29+
* Custom metadata is persisted in the backing map under the {@link Metadata#CUSTOM_PROP_PREFIX}
30+
* ("dot:") prefix; {@code Metadata.getCustomMeta()} strips that prefix when exposing it.
31+
*/
32+
private static final String CUSTOM_FOCAL_POINT_KEY =
33+
Metadata.CUSTOM_PROP_PREFIX + FocalPointAPI.FOCAL_POINT;
34+
35+
/**
36+
* Invokes the private {@code addBinaries} method in isolation so the focal-point behavior can be
37+
* exercised without standing up the rest of the transform pipeline.
38+
*/
39+
@SuppressWarnings("unchecked")
40+
private void invokeAddBinaries(final DefaultTransformStrategy strategy,
41+
final Contentlet contentlet, final Map<String, Object> map) throws Exception {
42+
final Method addBinaries = DefaultTransformStrategy.class.getDeclaredMethod(
43+
"addBinaries", Contentlet.class, Map.class, Set.class);
44+
addBinaries.setAccessible(true);
45+
addBinaries.invoke(strategy, contentlet, map, Set.of(BINARIES));
46+
}
47+
48+
private Contentlet mockContentletWithBinary(final Metadata metadata) throws Exception {
49+
final Field field = Mockito.mock(BinaryField.class);
50+
Mockito.when(field.variable()).thenReturn(FIELD_VAR);
51+
52+
final ContentType contentType = Mockito.mock(ContentType.class);
53+
Mockito.when(contentType.fields(BinaryField.class)).thenReturn(List.of(field));
54+
55+
final Contentlet contentlet = Mockito.mock(Contentlet.class);
56+
Mockito.when(contentlet.getContentType()).thenReturn(contentType);
57+
Mockito.when(contentlet.isFileAsset()).thenReturn(false);
58+
Mockito.when(contentlet.getIdentifier()).thenReturn("identifier-1");
59+
Mockito.when(contentlet.getInode()).thenReturn("inode-1");
60+
Mockito.when(contentlet.getBinaryMetadata(FIELD_VAR)).thenReturn(metadata);
61+
return contentlet;
62+
}
63+
64+
/**
65+
* When the binary's custom metadata carries a focal point, it must be surfaced under
66+
* {@code {field}MetaData.focalPoint} on the REST read path so the image editor can re-seed its
67+
* marker. Regression coverage for <a href="https://github.com/dotCMS/core/issues/36067">#36067</a>.
68+
*/
69+
@Test
70+
public void testAddBinaries_whenCustomMetaHasFocalPoint_surfacesItInMetaDataMap()
71+
throws Exception {
72+
73+
final Map<String, Serializable> fieldsMeta = new HashMap<>();
74+
fieldsMeta.put("name", "image.png");
75+
fieldsMeta.put(CUSTOM_FOCAL_POINT_KEY, "0.25,0.75");
76+
final Metadata metadata = new Metadata(FIELD_VAR, fieldsMeta);
77+
78+
final APIProvider toolBox = Mockito.mock(APIProvider.class);
79+
final DefaultTransformStrategy strategy = new DefaultTransformStrategy(toolBox);
80+
final Contentlet contentlet = mockContentletWithBinary(metadata);
81+
82+
final Map<String, Object> map = new HashMap<>();
83+
invokeAddBinaries(strategy, contentlet, map);
84+
85+
assertTrue("Expected the field MetaData entry to be present", map.containsKey(META_KEY));
86+
@SuppressWarnings("unchecked")
87+
final Map<String, Serializable> metaMap = (Map<String, Serializable>) map.get(META_KEY);
88+
assertEquals("Focal point from custom metadata must be exposed under the focalPoint key",
89+
"0.25,0.75", metaMap.get(FocalPointAPI.FOCAL_POINT));
90+
}
91+
92+
/**
93+
* When the binary has no focal point in its custom metadata, the read path must default the
94+
* {@code focalPoint} entry to "0.0" rather than omit it, matching the GraphQL/Velocity view.
95+
*/
96+
@Test
97+
public void testAddBinaries_whenCustomMetaHasNoFocalPoint_defaultsToZero()
98+
throws Exception {
99+
100+
final Map<String, Serializable> fieldsMeta = new HashMap<>();
101+
fieldsMeta.put("name", "image.png");
102+
final Metadata metadata = new Metadata(FIELD_VAR, fieldsMeta);
103+
104+
final APIProvider toolBox = Mockito.mock(APIProvider.class);
105+
final DefaultTransformStrategy strategy = new DefaultTransformStrategy(toolBox);
106+
final Contentlet contentlet = mockContentletWithBinary(metadata);
107+
108+
final Map<String, Object> map = new HashMap<>();
109+
invokeAddBinaries(strategy, contentlet, map);
110+
111+
assertTrue("Expected the field MetaData entry to be present", map.containsKey(META_KEY));
112+
@SuppressWarnings("unchecked")
113+
final Map<String, Serializable> metaMap = (Map<String, Serializable>) map.get(META_KEY);
114+
assertEquals("focalPoint must default to 0.0 when absent from custom metadata",
115+
"0.0", metaMap.get(FocalPointAPI.FOCAL_POINT));
116+
}
117+
}

0 commit comments

Comments
 (0)