Skip to content

Commit 899f057

Browse files
surlisurli
committed
XWIKI-23236: Using the office macro inside the async macro isn't possible anymore
 * Ensure to check that the session exists for any call performed in DefaultTemporaryAttchmentSessionManager
1 parent 3dd2432 commit 899f057

File tree

3 files changed

+139
-27
lines changed

3 files changed

+139
-27
lines changed

‎xwiki-platform-core/xwiki-platform-store/xwiki-platform-store-filesystem-oldcore/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
<name>XWiki Platform - Store - Filesystem - Old Core</name>
3232
<description>Implement various oldcore store APIs based on filesystem.</description>
3333
<properties>
34-
<xwiki.jacoco.instructionRatio>0.40</xwiki.jacoco.instructionRatio>
34+
<xwiki.jacoco.instructionRatio>0.41</xwiki.jacoco.instructionRatio>
3535
<!-- Old names of this module used for retro compatibility when resolving dependencies of old extensions -->
3636
<xwiki.extension.features>org.xwiki.platform:xwiki-platform-store-filesystem-attachments</xwiki.extension.features>
3737
</properties>

‎xwiki-platform-core/xwiki-platform-store/xwiki-platform-store-filesystem-oldcore/src/main/java/org/xwiki/store/filesystem/internal/DefaultTemporaryAttachmentSessionsManager.java

Lines changed: 59 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import java.io.IOException;
2323
import java.util.Collection;
24+
import java.util.Collections;
2425
import java.util.List;
2526
import java.util.Optional;
2627

@@ -32,6 +33,7 @@
3233
import jakarta.servlet.http.Part;
3334

3435
import org.apache.commons.lang3.StringUtils;
36+
import org.slf4j.Logger;
3537
import org.xwiki.attachment.validation.AttachmentValidationException;
3638
import org.xwiki.attachment.validation.AttachmentValidator;
3739
import org.xwiki.component.annotation.Component;
@@ -68,21 +70,28 @@ public class DefaultTemporaryAttachmentSessionsManager implements TemporaryAttac
6870
@Inject
6971
private Provider<Container> container;
7072

73+
@Inject
74+
private Logger logger;
75+
7176
private HttpSession getSession()
7277
{
7378
return ((ServletSession) this.container.get().getSession()).getSession();
7479
}
7580

76-
private TemporaryAttachmentSession getOrCreateSession()
81+
private Optional<TemporaryAttachmentSession> getOrCreateSession()
7782
{
78-
TemporaryAttachmentSession temporaryAttachmentSession;
83+
Optional<TemporaryAttachmentSession> result = Optional.empty();
7984
HttpSession session = this.getSession();
80-
temporaryAttachmentSession = (TemporaryAttachmentSession) session.getAttribute(ATTRIBUTE_KEY);
81-
if (temporaryAttachmentSession == null) {
82-
temporaryAttachmentSession = new TemporaryAttachmentSession(session.getId());
83-
session.setAttribute(ATTRIBUTE_KEY, temporaryAttachmentSession);
85+
if (session != null) {
86+
TemporaryAttachmentSession temporaryAttachmentSession =
87+
(TemporaryAttachmentSession) session.getAttribute(ATTRIBUTE_KEY);
88+
if (temporaryAttachmentSession == null) {
89+
temporaryAttachmentSession = new TemporaryAttachmentSession(session.getId());
90+
session.setAttribute(ATTRIBUTE_KEY, temporaryAttachmentSession);
91+
}
92+
result = Optional.of(temporaryAttachmentSession);
8493
}
85-
return temporaryAttachmentSession;
94+
return result;
8695
}
8796

8897
@Override
@@ -104,7 +113,11 @@ public XWikiAttachment uploadAttachment(DocumentReference documentReference, jav
104113
public XWikiAttachment uploadAttachment(DocumentReference documentReference, Part part, String filename)
105114
throws TemporaryAttachmentException, AttachmentValidationException
106115
{
107-
TemporaryAttachmentSession temporaryAttachmentSession = getOrCreateSession();
116+
Optional<TemporaryAttachmentSession> optionalSession = getOrCreateSession();
117+
if (optionalSession.isEmpty()) {
118+
throw new TemporaryAttachmentException("Cannot find a user http session.");
119+
}
120+
TemporaryAttachmentSession temporaryAttachmentSession = optionalSession.get();
108121
XWikiContext context = this.contextProvider.get();
109122
try {
110123
XWikiAttachment xWikiAttachment = new XWikiAttachment();
@@ -134,36 +147,64 @@ public XWikiAttachment uploadAttachment(DocumentReference documentReference, Par
134147
@Override
135148
public void temporarilyAttach(XWikiAttachment attachment, DocumentReference documentReference)
136149
{
137-
TemporaryAttachmentSession temporaryAttachmentSession = getOrCreateSession();
138-
temporaryAttachmentSession.addAttachment(documentReference, attachment);
150+
Optional<TemporaryAttachmentSession> optionalSession = getOrCreateSession();
151+
if (optionalSession.isPresent()) {
152+
TemporaryAttachmentSession temporaryAttachmentSession = optionalSession.get();
153+
temporaryAttachmentSession.addAttachment(documentReference, attachment);
154+
} else {
155+
this.logger.error("Cannot find a user http session to attach [{}] to [{}].", attachment, documentReference);
156+
}
139157
}
140158

141159
@Override
142160
public Collection<XWikiAttachment> getUploadedAttachments(DocumentReference documentReference)
143161
{
144-
TemporaryAttachmentSession temporaryAttachmentSession = getOrCreateSession();
145-
return temporaryAttachmentSession.getAttachments(documentReference);
162+
Optional<TemporaryAttachmentSession> optionalSession = getOrCreateSession();
163+
if (optionalSession.isEmpty()) {
164+
return Collections.emptyList();
165+
} else {
166+
TemporaryAttachmentSession temporaryAttachmentSession = optionalSession.get();
167+
return temporaryAttachmentSession.getAttachments(documentReference);
168+
}
146169
}
147170

148171
@Override
149172
public Optional<XWikiAttachment> getUploadedAttachment(DocumentReference documentReference, String filename)
150173
{
151-
TemporaryAttachmentSession temporaryAttachmentSession = getOrCreateSession();
152-
return temporaryAttachmentSession.getAttachment(documentReference, filename);
174+
Optional<TemporaryAttachmentSession> optionalSession = getOrCreateSession();
175+
if (optionalSession.isPresent()) {
176+
TemporaryAttachmentSession temporaryAttachmentSession = optionalSession.get();
177+
return temporaryAttachmentSession.getAttachment(documentReference, filename);
178+
} else {
179+
return Optional.empty();
180+
}
153181
}
154182

155183
@Override
156184
public boolean removeUploadedAttachment(DocumentReference documentReference, String filename)
157185
{
158-
TemporaryAttachmentSession temporaryAttachmentSession = getOrCreateSession();
159-
return temporaryAttachmentSession.removeAttachment(documentReference, filename);
186+
Optional<TemporaryAttachmentSession> optionalSession = getOrCreateSession();
187+
if (optionalSession.isPresent()) {
188+
TemporaryAttachmentSession temporaryAttachmentSession = optionalSession.get();
189+
return temporaryAttachmentSession.removeAttachment(documentReference, filename);
190+
} else {
191+
this.logger.warn("Cannot find a user http session to remove attachment [{}] from [{}].", filename,
192+
documentReference);
193+
return false;
194+
}
160195
}
161196

162197
@Override
163198
public boolean removeUploadedAttachments(DocumentReference documentReference)
164199
{
165-
TemporaryAttachmentSession temporaryAttachmentSession = getOrCreateSession();
166-
return temporaryAttachmentSession.removeAttachments(documentReference);
200+
Optional<TemporaryAttachmentSession> optionalSession = getOrCreateSession();
201+
if (optionalSession.isPresent()) {
202+
TemporaryAttachmentSession temporaryAttachmentSession = optionalSession.get();
203+
return temporaryAttachmentSession.removeAttachments(documentReference);
204+
} else {
205+
this.logger.warn("Cannot find a user http session to remove attachments from [{}].", documentReference);
206+
return false;
207+
}
167208
}
168209

169210
@Override

‎xwiki-platform-core/xwiki-platform-store/xwiki-platform-store-filesystem-oldcore/src/test/java/org/xwiki/store/filesystem/internal/DefaultTemporaryAttachmentSessionsManagerTest.java

Lines changed: 79 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,12 @@
2929
import javax.inject.Provider;
3030
import javax.servlet.http.Part;
3131

32+
import ch.qos.logback.classic.Level;
3233
import jakarta.servlet.http.HttpSession;
3334

3435
import org.junit.jupiter.api.BeforeEach;
3536
import org.junit.jupiter.api.Test;
37+
import org.junit.jupiter.api.extension.RegisterExtension;
3638
import org.junit.jupiter.params.ParameterizedTest;
3739
import org.junit.jupiter.params.provider.NullSource;
3840
import org.junit.jupiter.params.provider.ValueSource;
@@ -44,8 +46,10 @@
4446
import org.xwiki.model.reference.DocumentReference;
4547
import org.xwiki.model.reference.SpaceReference;
4648
import org.xwiki.store.TemporaryAttachmentException;
49+
import org.xwiki.test.LogLevel;
4750
import org.xwiki.test.TestEnvironment;
4851
import org.xwiki.test.annotation.ComponentList;
52+
import org.xwiki.test.junit5.LogCaptureExtension;
4953
import org.xwiki.test.junit5.mockito.ComponentTest;
5054
import org.xwiki.test.junit5.mockito.InjectMockComponents;
5155
import org.xwiki.test.junit5.mockito.MockComponent;
@@ -60,6 +64,7 @@
6064
import static com.xpn.xwiki.plugin.fileupload.FileUploadPlugin.UPLOAD_MAXSIZE_PARAMETER;
6165
import static java.nio.charset.StandardCharsets.UTF_8;
6266
import static org.junit.jupiter.api.Assertions.assertEquals;
67+
import static org.junit.jupiter.api.Assertions.assertFalse;
6368
import static org.junit.jupiter.api.Assertions.assertNotNull;
6469
import static org.junit.jupiter.api.Assertions.assertThrows;
6570
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -97,6 +102,9 @@ class DefaultTemporaryAttachmentSessionsManagerTest
97102
@MockComponent
98103
private Container container;
99104

105+
@RegisterExtension
106+
private LogCaptureExtension logCapture = new LogCaptureExtension(LogLevel.WARN);
107+
100108
@Mock
101109
private AttachmentValidator attachmentValidator;
102110

@@ -106,28 +114,41 @@ class DefaultTemporaryAttachmentSessionsManagerTest
106114
@Mock
107115
private HttpSession httpSession;
108116

117+
@Mock
118+
private ServletSession servletSession;
119+
109120
@BeforeEach
110121
void setup(MockitoComponentManager mockitoComponentManager) throws Exception
111122
{
112123
when(this.contextProvider.get()).thenReturn(this.context);
113-
114-
ServletSession session = mock(ServletSession.class);
115-
when(session.getSession()).thenReturn(this.httpSession);
116-
when(this.container.getSession()).thenReturn(session);
124+
when(this.container.getSession()).thenReturn(this.servletSession);
117125
Utils.setComponentManager(mockitoComponentManager);
118126

119127
when(this.attachmentValidatorProvider.get()).thenReturn(this.attachmentValidator);
120128
}
121129

130+
private void setupSession()
131+
{
132+
when(this.servletSession.getSession()).thenReturn(this.httpSession);
133+
}
134+
122135
@Test
123136
void uploadAttachment() throws Exception
124137
{
125-
String sessionId = "mySession";
126-
when(this.httpSession.getId()).thenReturn(sessionId);
127-
128138
DocumentReference documentReference = new DocumentReference("xwiki", "Space", "Document");
129139
Part part = mock(Part.class);
130140

141+
// session is null
142+
TemporaryAttachmentException temporaryAttachmentException =
143+
assertThrows(TemporaryAttachmentException.class, () -> {
144+
this.attachmentManager.uploadAttachment(documentReference, part);
145+
});
146+
assertEquals("Cannot find a user http session.", temporaryAttachmentException.getMessage());
147+
148+
setupSession();
149+
String sessionId = "mySession";
150+
when(this.httpSession.getId()).thenReturn(sessionId);
151+
131152
String filename = "fileFoo.xml";
132153
when(part.getSubmittedFileName()).thenReturn(filename);
133154
InputStream inputStream = new ByteArrayInputStream("foo".getBytes(UTF_8));
@@ -166,6 +187,14 @@ void uploadAttachmentWithFilename() throws Exception
166187
XWiki xwiki = mock(XWiki.class);
167188
Part part = mock(Part.class);
168189

190+
// session is null
191+
TemporaryAttachmentException temporaryAttachmentException =
192+
assertThrows(TemporaryAttachmentException.class, () -> {
193+
this.attachmentManager.uploadAttachment(documentReference, part, "newFilename");
194+
});
195+
assertEquals("Cannot find a user http session.", temporaryAttachmentException.getMessage());
196+
197+
setupSession();
169198
when(this.context.getWiki()).thenReturn(xwiki);
170199
when(xwiki.getSpacePreference(UPLOAD_MAXSIZE_PARAMETER, spaceReference, this.context))
171200
.thenReturn("42");
@@ -181,6 +210,7 @@ void uploadAttachmentWithFilename() throws Exception
181210
@Test
182211
void uploadAttachmentInvalid() throws Exception
183212
{
213+
setupSession();
184214
DocumentReference documentReference = new DocumentReference("xwiki", "XWiki", "Document");
185215
SpaceReference spaceReference = documentReference.getLastSpaceReference();
186216

@@ -215,6 +245,7 @@ void uploadAttachmentInvalid() throws Exception
215245
})
216246
void uploadAttachmentWithEmptyFilename(String filename) throws Exception
217247
{
248+
setupSession();
218249
DocumentReference documentReference = new DocumentReference("xwiki", "XWiki", "Document");
219250
SpaceReference spaceReference = documentReference.getLastSpaceReference();
220251

@@ -248,6 +279,10 @@ void getUploadedAttachments()
248279
List<XWikiAttachment> expectedList = Arrays.asList(attachment1, attachment2, attachment3);
249280
when(temporaryAttachmentSession.getAttachments(documentReference)).thenReturn(expectedList);
250281

282+
// session is null
283+
assertEquals(Collections.emptyList(), this.attachmentManager.getUploadedAttachments(documentReference));
284+
285+
setupSession();
251286
assertEquals(expectedList,
252287
this.attachmentManager.getUploadedAttachments(documentReference));
253288
}
@@ -267,6 +302,11 @@ void getUploadedAttachment()
267302

268303
when(temporaryAttachmentSession.getAttachment(documentReference, filename))
269304
.thenReturn(Optional.of(attachment1));
305+
306+
// session is null
307+
assertEquals(Optional.empty(), this.attachmentManager.getUploadedAttachment(documentReference, filename));
308+
309+
setupSession();
270310
assertEquals(Optional.of(attachment1),
271311
this.attachmentManager.getUploadedAttachment(documentReference, filename));
272312
}
@@ -280,8 +320,18 @@ void removeUploadedAttachment()
280320
when(httpSession.getAttribute(ATTRIBUTE_KEY)).thenReturn(temporaryAttachmentSession);
281321

282322
DocumentReference documentReference = mock(DocumentReference.class);
323+
when(documentReference.toString()).thenReturn("reference");
283324
String filename = "foobar";
284325
when(temporaryAttachmentSession.removeAttachment(documentReference, filename)).thenReturn(true);
326+
327+
// session is null
328+
assertFalse(this.attachmentManager.removeUploadedAttachment(documentReference, filename));
329+
assertEquals(1, this.logCapture.size());
330+
assertEquals("Cannot find a user http session to remove attachment [foobar] from [reference].",
331+
this.logCapture.getMessage(0));
332+
assertEquals(Level.WARN, this.logCapture.getLogEvent(0).getLevel());
333+
334+
setupSession();
285335
assertTrue(this.attachmentManager.removeUploadedAttachment(documentReference, filename));
286336
}
287337

@@ -293,20 +343,40 @@ void removeUploadedAttachments()
293343
TemporaryAttachmentSession temporaryAttachmentSession = mock(TemporaryAttachmentSession.class);
294344
when(httpSession.getAttribute(ATTRIBUTE_KEY)).thenReturn(temporaryAttachmentSession);
295345
DocumentReference documentReference = mock(DocumentReference.class);
346+
when(documentReference.toString()).thenReturn("reference");
296347
when(temporaryAttachmentSession.removeAttachments(documentReference)).thenReturn(true);
348+
349+
// session is null
350+
assertFalse(this.attachmentManager.removeUploadedAttachments(documentReference));
351+
assertEquals(1, this.logCapture.size());
352+
assertEquals("Cannot find a user http session to remove attachments from [reference].",
353+
this.logCapture.getMessage(0));
354+
assertEquals(Level.WARN, this.logCapture.getLogEvent(0).getLevel());
355+
356+
setupSession();
297357
assertTrue(this.attachmentManager.removeUploadedAttachments(documentReference));
298358
}
299359

300360
@Test
301-
void temporarilyAttach() throws TemporaryAttachmentException
361+
void temporarilyAttach()
302362
{
303363
DocumentReference documentReference = mock(DocumentReference.class);
364+
when(documentReference.toString()).thenReturn("reference");
304365
XWikiAttachment attachment = mock(XWikiAttachment.class);
366+
when(attachment.toString()).thenReturn("attachment");
305367
String sessionId = "removeUploadedAttachmentsPlural";
306368
when(httpSession.getId()).thenReturn(sessionId);
307369
TemporaryAttachmentSession temporaryAttachmentSession = mock(TemporaryAttachmentSession.class);
308370
when(httpSession.getAttribute(ATTRIBUTE_KEY)).thenReturn(temporaryAttachmentSession);
309371

372+
// session is null
373+
this.attachmentManager.temporarilyAttach(attachment, documentReference);
374+
assertEquals(1, this.logCapture.size());
375+
assertEquals("Cannot find a user http session to attach [attachment] to [reference].",
376+
this.logCapture.getMessage(0));
377+
assertEquals(Level.ERROR, this.logCapture.getLogEvent(0).getLevel());
378+
379+
setupSession();
310380
this.attachmentManager.temporarilyAttach(attachment, documentReference);
311381
verify(temporaryAttachmentSession).addAttachment(documentReference, attachment);
312382

@@ -343,6 +413,7 @@ void attachTemporaryAttachmentsInDocument()
343413
when(temporaryAttachmentSession.getAttachment(documentReference, file1)).thenReturn(Optional.of(attachment1));
344414
when(temporaryAttachmentSession.getAttachment(documentReference, file2)).thenReturn(Optional.of(attachment2));
345415

416+
setupSession();
346417
this.attachmentManager.attachTemporaryAttachmentsInDocument(document, List.of(
347418
"foo",
348419
file1,

0 commit comments

Comments
 (0)