Skip to content

Commit b60aba5

Browse files
committed
frontend:make DirectoryListSource Polimorph
Motivation The DirectoryListSource should not include logic unrelated to its core responsibility. Specifically, methods for virtual directory listing and labels listing do not belong in this interface. Modification To improve separation of concerns, two new interfaces have been introduced: VirtualDirectoryListSource for virtual directory listings LabelsListSource for label listings (not in this putch) As a first step, a new VirtualDirectoryListHandler has been added. Result The listVirtualDirectory functionality has been extracted from DirectoryListSource, improving modularity and maintainability. Target: master PAtch: https://rb.dcache.org/r/14357/ Acked-by:Tigran Mkrtchyan
1 parent 61b7dbf commit b60aba5

File tree

8 files changed

+315
-72
lines changed

8 files changed

+315
-72
lines changed

modules/dcache-frontend/src/main/java/org/dcache/restful/resources/labels/LabelsResources.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
import org.dcache.util.list.DirectoryEntry;
6363
import org.dcache.util.list.DirectoryStream;
6464
import org.dcache.util.list.ListDirectoryHandler;
65+
import org.dcache.util.list.VirtualDirectoryListHandler;
6566
import org.slf4j.Logger;
6667
import org.slf4j.LoggerFactory;
6768
import org.springframework.stereotype.Component;
@@ -92,7 +93,7 @@ public class LabelsResources {
9293
private PathMapper pathMapper;
9394

9495
@Inject
95-
private ListDirectoryHandler listDirectoryHandler;
96+
private VirtualDirectoryListHandler virtualDirectoryListHandler;
9697

9798
@Inject
9899
@Named("pool-manager-stub")
@@ -102,9 +103,6 @@ public class LabelsResources {
102103
@Named("pinManagerStub")
103104
private CellStub pinmanager;
104105

105-
@Inject
106-
@Named("pnfs-stub")
107-
private CellStub pnfsmanager;
108106

109107

110108
@GET
@@ -165,7 +163,7 @@ public JsonFileAttributes getFileAttributes(@ApiParam("Path of file or directory
165163
try {
166164
List<JsonFileAttributes> children = new ArrayList<>();
167165

168-
DirectoryStream stream = listDirectoryHandler.listVirtualDirectory(
166+
DirectoryStream stream = virtualDirectoryListHandler.listVirtualDirectory(
169167
HttpServletRequests.roleAwareSubject(request),
170168
HttpServletRequests.roleAwareRestriction(request),
171169
path,

modules/dcache-frontend/src/main/resources/org/dcache/frontend/frontend.xml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,15 @@
8888
</bean>
8989
</constructor-arg>
9090
</bean>
91+
92+
<bean id="virtual-dir-list-handler" class="org.dcache.util.list.VirtualDirectoryListHandler">
93+
<description>Client stub for directory listing</description>
94+
<constructor-arg>
95+
<bean class="diskCacheV111.util.PnfsHandler">
96+
<constructor-arg ref="pnfs-stub"/>
97+
</bean>
98+
</constructor-arg>
99+
</bean>
91100
<bean id="path-mapper" class="org.dcache.http.PathMapper">
92101
<description>Mapping between request paths and dCache paths</description>
93102
<property name="rootPath" value="${frontend.root}"/>

modules/dcache/src/main/java/org/dcache/auth/RemoteNameSpaceProvider.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import org.dcache.util.list.DirectoryEntry;
3939
import org.dcache.util.list.DirectoryStream;
4040
import org.dcache.util.list.ListDirectoryHandler;
41+
import org.dcache.util.list.VirtualDirectoryListHandler;
4142
import org.dcache.vehicles.FileAttributes;
4243

4344
/**
@@ -48,16 +49,19 @@ public class RemoteNameSpaceProvider implements NameSpaceProvider {
4849

4950
private final PnfsHandler _pnfs;
5051
private final ListDirectoryHandler _handler;
52+
private final VirtualDirectoryListHandler _virtualDirectoryHandler;
53+
5154

5255

5356
public RemoteNameSpaceProvider(PnfsHandler pnfsHandler,
54-
ListDirectoryHandler listHandler) {
57+
ListDirectoryHandler listHandler, VirtualDirectoryListHandler virtualDirectoryListHandler) {
5558
_pnfs = pnfsHandler;
5659
_handler = listHandler;
60+
_virtualDirectoryHandler = virtualDirectoryListHandler;
5761
}
5862

5963
public RemoteNameSpaceProvider(PnfsHandler pnfsHandler) {
60-
this(pnfsHandler, new ListDirectoryHandler(pnfsHandler));
64+
this(pnfsHandler, new ListDirectoryHandler(pnfsHandler), new VirtualDirectoryListHandler(pnfsHandler));
6165
}
6266

6367
@Override
@@ -205,7 +209,7 @@ public void listVirtualDirectory(Subject subject, String path,
205209
Range<Integer> range, Set<FileAttribute> attrs, ListHandler handler)
206210
throws CacheException
207211
{
208-
try (DirectoryStream stream = _handler.listVirtualDirectory(subject, Restrictions.none(), FsPath.create(path), range, attrs)) {
212+
try (DirectoryStream stream = _virtualDirectoryHandler.listVirtualDirectory(subject, Restrictions.none(), FsPath.create(path), range, attrs)) {
209213
for (DirectoryEntry entry : stream) {
210214
handler.addEntry(entry.getName(), entry.getFileAttributes());
211215
}

modules/dcache/src/main/java/org/dcache/util/list/DirectoryListSource.java

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -62,24 +62,6 @@ DirectoryStream list(Subject subject, Restriction restriction, FsPath path,
6262
Set<FileAttribute> attrs)
6363
throws InterruptedException, CacheException;
6464

65-
/**
66-
* Lists the content of a virtual directory. The content is returned as a
67-
* directory stream containing files with a given label (path).
68-
* @param subject The Subject of the user performing the
69-
* operation; may be null
70-
* @param restriction a login attribute; may be zero or more
71-
* @param path Path to virtual directory to list (which is the label value of a streamed files)
72-
* @param range The range of entries to return; may be null
73-
* @param attrs The file attributes to query for each entry
74-
* @return A DirectoryStream of the entries in the directory
75-
76-
*/
77-
DirectoryStream listVirtualDirectory(Subject subject, Restriction restriction, FsPath path,
78-
Range<Integer> range,
79-
Set<FileAttribute> attrs)
80-
throws InterruptedException, CacheException;
81-
82-
8365
/**
8466
* Prints a file using a DirectoryListPrinter.
8567
*

modules/dcache/src/main/java/org/dcache/util/list/ListDirectoryHandler.java

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -107,51 +107,6 @@ public ListDirectoryHandler(PnfsHandler pnfs) {
107107
}
108108
}
109109

110-
111-
112-
/**
113-
* Sends a virtual directory list request to PnfsManager. The result is
114-
* provided as a stream of files having the label value equale to the value of path param.
115-
* <p>
116-
* The method blocks until the first set of entries have
117-
* been received from the server. Hence errors like
118-
* FILE_NOT_FOUND are thrown by the call to the list method rather
119-
* than while iterating over the stream.
120-
* <p>
121-
* Note that supplied subject and restriction values will be overwritten if
122-
* {@link PnfsHandler#setSubject} or {@link PnfsHandler#setRestriction} have
123-
* been called on the underlying PnfsHandler instance.
124-
*/
125-
126-
@Override
127-
public DirectoryStream
128-
listVirtualDirectory(Subject subject, Restriction restriction, FsPath path,
129-
Range<Integer> range, Set<FileAttribute> attributes)
130-
throws InterruptedException, CacheException
131-
{
132-
String dir = path.toString();
133-
PnfsListDirectoryMessage msg =
134-
new PnfsListDirectoryMessage(dir, null, range, attributes);
135-
UUID uuid = msg.getUUID();
136-
boolean success = false;
137-
Stream stream = new Stream(dir, uuid);
138-
try {
139-
msg.setPathType(PnfsListDirectoryMessage.PathType.LABEL);
140-
msg.setSubject(subject);
141-
142-
msg.setRestriction(restriction);
143-
_replies.put(uuid, stream);
144-
_pnfs.send(msg);
145-
stream.waitForMoreEntries();
146-
success = true;
147-
return stream;
148-
} finally {
149-
if (!success) {
150-
_replies.remove(uuid);
151-
}
152-
}
153-
}
154-
155110
@Override
156111
public void printFile(Subject subject, Restriction restriction,
157112
DirectoryListPrinter printer, FsPath path)
Lines changed: 230 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,230 @@
1+
/*
2+
* dCache - http://www.dcache.org/
3+
*
4+
* Copyright (C) 2025 Deutsches Elektronen-Synchrotron
5+
*
6+
* This program is free software: you can redistribute it and/or modify
7+
* it under the terms of the GNU Affero General Public License as
8+
* published by the Free Software Foundation, either version 3 of the
9+
* License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
14+
* GNU Affero General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Affero General Public License
17+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
18+
*/
19+
package org.dcache.util.list;
20+
21+
import com.google.common.collect.Range;
22+
import diskCacheV111.util.CacheException;
23+
import diskCacheV111.util.FsPath;
24+
import diskCacheV111.util.PnfsHandler;
25+
import dmg.cells.nucleus.CellMessageReceiver;
26+
import java.util.Iterator;
27+
import java.util.Map;
28+
import java.util.NoSuchElementException;
29+
import java.util.Set;
30+
import java.util.UUID;
31+
import java.util.concurrent.BlockingQueue;
32+
import java.util.concurrent.ConcurrentHashMap;
33+
import java.util.concurrent.LinkedBlockingQueue;
34+
import java.util.concurrent.TimeUnit;
35+
import javax.security.auth.Subject;
36+
import org.dcache.auth.attributes.Restriction;
37+
import org.dcache.namespace.FileAttribute;
38+
import org.dcache.util.CacheExceptionFactory;
39+
40+
import org.dcache.vehicles.PnfsListDirectoryMessage;
41+
import org.slf4j.Logger;
42+
import org.slf4j.LoggerFactory;
43+
44+
/**
45+
* VirtualDirectoryListSource which delegates the virtual directory listing operation to the PnfsManager.
46+
* <p>
47+
* Large virtual directories are broken into several reply messages by the PnfsManager. For that reason the
48+
* regular Cells callback mechanism for replies cannot be used. Instead messages of type
49+
* PnfsListDirectoryMessage must be routed to the VirtualDirectoryListHandler. This also has the
50+
* consequence that a VirtualDirectoryListHandler cannot be used from the Cells messages thread. Any
51+
* attempt to do so will cause the message thread to block, as the replies cannot be delivered to
52+
* the VirtualDirectoryListHandler.
53+
*/
54+
public class VirtualDirectoryListHandler
55+
implements CellMessageReceiver, VirtualDirectoryListSource {
56+
57+
private static final Logger LOGGER =
58+
LoggerFactory.getLogger(ListDirectoryHandler.class);
59+
60+
private final PnfsHandler _pnfs;
61+
private final Map<UUID, Stream> _replies =
62+
new ConcurrentHashMap<>();
63+
64+
public VirtualDirectoryListHandler(PnfsHandler pnfs) {
65+
_pnfs = pnfs;
66+
}
67+
68+
69+
@Override
70+
public DirectoryStream
71+
listVirtualDirectory(Subject subject, Restriction restriction, FsPath path,
72+
Range<Integer> range, Set<FileAttribute> attributes)
73+
throws InterruptedException, CacheException
74+
{
75+
String dir = path.toString();
76+
PnfsListDirectoryMessage msg =
77+
new PnfsListDirectoryMessage(dir, null, range, attributes);
78+
UUID uuid = msg.getUUID();
79+
boolean success = false;
80+
Stream stream = new Stream(dir, uuid);
81+
try {
82+
msg.setPathType(PnfsListDirectoryMessage.PathType.LABEL);
83+
msg.setSubject(subject);
84+
85+
msg.setRestriction(restriction);
86+
_replies.put(uuid, stream);
87+
_pnfs.send(msg);
88+
stream.waitForMoreEntries();
89+
success = true;
90+
return stream;
91+
} finally {
92+
if (!success) {
93+
_replies.remove(uuid);
94+
}
95+
}
96+
}
97+
98+
/**
99+
* Callback for delivery of replies from PnfsManager. PnfsListDirectoryMessage have to be routed
100+
* to this message.
101+
*/
102+
public void messageArrived(PnfsListDirectoryMessage reply) {
103+
if (reply.isReply()) {
104+
try {
105+
UUID uuid = reply.getUUID();
106+
VirtualDirectoryListHandler.Stream stream = _replies.get(uuid);
107+
if (stream != null) {
108+
stream.put(reply);
109+
} else {
110+
LOGGER.warn(
111+
"Received list result for an unknown request. Virtual Directory listing was possibly incomplete.");
112+
}
113+
} catch (InterruptedException e) {
114+
Thread.currentThread().interrupt();
115+
}
116+
}
117+
}
118+
119+
120+
/**
121+
* Implementation of DirectoryStream, translating PnfsListDirectoryMessage replies to a stream
122+
* of DirectoryEntries.
123+
* <p>
124+
* The stream acts as its own iterator, and multiple iterators are not supported.
125+
*/
126+
public class Stream
127+
implements DirectoryStream, Iterator<DirectoryEntry> {
128+
129+
private final BlockingQueue<PnfsListDirectoryMessage> _queue =
130+
new LinkedBlockingQueue<>();
131+
private final UUID _uuid;
132+
private final String _path;
133+
private boolean _isFinal;
134+
private Iterator<DirectoryEntry> _iterator;
135+
private int _count;
136+
private int _total;
137+
138+
public Stream(String path, UUID uuid) {
139+
_path = path;
140+
_uuid = uuid;
141+
}
142+
143+
@Override
144+
public void close() {
145+
_replies.remove(_uuid);
146+
}
147+
148+
private void put(PnfsListDirectoryMessage msg)
149+
throws InterruptedException {
150+
_queue.put(msg);
151+
}
152+
153+
private void waitForMoreEntries()
154+
throws InterruptedException, CacheException {
155+
if (_isFinal) {
156+
_iterator = null;
157+
return;
158+
}
159+
160+
PnfsListDirectoryMessage msg =
161+
_queue.poll(_pnfs.getPnfsTimeout(), TimeUnit.MILLISECONDS);
162+
if (msg == null) {
163+
throw new CacheException(CacheException.TIMEOUT,
164+
"Timeout during virtual directory listing.");
165+
}
166+
167+
if (msg.isFinal()) {
168+
_total = msg.getMessageCount();
169+
}
170+
_count++;
171+
if (_count == _total) {
172+
_isFinal = true;
173+
}
174+
175+
if (msg.getReturnCode() != 0) {
176+
throw CacheExceptionFactory.exceptionOf(msg);
177+
}
178+
179+
_iterator = msg.getEntries().iterator();
180+
181+
/* If the message is empty, then the iterator has no next
182+
* element. In that case we wait for the next reply. This
183+
* may in particular happen with the final message.
184+
*/
185+
if (!_iterator.hasNext()) {
186+
waitForMoreEntries();
187+
}
188+
}
189+
190+
@Override
191+
public Iterator<DirectoryEntry> iterator() {
192+
return this;
193+
}
194+
195+
@Override
196+
public boolean hasNext() {
197+
try {
198+
if (_iterator == null || !_iterator.hasNext()) {
199+
waitForMoreEntries();
200+
if (_iterator == null) {
201+
return false;
202+
}
203+
}
204+
} catch (CacheException e) {
205+
LOGGER.error("Listing of {} incomplete: {}", _path, e.getMessage());
206+
return false;
207+
} catch (InterruptedException e) {
208+
Thread.currentThread().interrupt();
209+
return false;
210+
}
211+
212+
return true;
213+
}
214+
215+
@Override
216+
public DirectoryEntry next() {
217+
if (!hasNext()) {
218+
throw new NoSuchElementException();
219+
}
220+
221+
return _iterator.next();
222+
}
223+
224+
@Override
225+
public void remove() {
226+
throw new UnsupportedOperationException();
227+
}
228+
}
229+
230+
}

0 commit comments

Comments
 (0)