gerlowskija commented on code in PR #3047:
URL: https://github.com/apache/solr/pull/3047#discussion_r1941803992


##########
solr/core/src/java/org/apache/solr/filestore/NodeFileStore.java:
##########
@@ -76,135 +66,54 @@ public SolrJerseyResponse getFile(String path, Boolean 
sync, String getFrom, Boo
     final var response = instantiateJerseyResponse(SolrJerseyResponse.class);
 
     if (Boolean.TRUE.equals(sync)) {
-      try {
-        fileStore.syncToAllNodes(path);
-        return response;
-      } catch (IOException e) {
-        throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Error 
getting file ", e);
-      }
+      ClusterFileStore.syncToAllNodes(fileStore, path);
+      return response;
     }
 
     if (path == null) {
       path = "";
     }
     final var pathCopy = path;
     if (getFrom != null) {
-      coreContainer
-          .getUpdateShardHandler()
-          .getUpdateExecutor()
-          .submit(
-              () -> {
-                log.debug("Downloading file {}", pathCopy);
-                try {
-                  fileStore.fetch(pathCopy, getFrom);
-                } catch (Exception e) {
-                  log.error("Failed to download file: {}", pathCopy, e);
-                }
-                log.info("downloaded file: {}", pathCopy);
-              });
+      ClusterFileStore.pullFileFromNode(coreContainer, fileStore, pathCopy, 
getFrom);
       return response;
     }
 
     FileStore.FileType type = fileStore.getType(path, false);
-    if (type == FileStore.FileType.NOFILE) {
-      final var fileMissingResponse =
-          instantiateJerseyResponse(FileStoreDirectoryListingResponse.class);
-      fileMissingResponse.files = Collections.singletonMap(path, null);
-      return fileMissingResponse;
-    }
-    if (type == FileStore.FileType.DIRECTORY) {
-      final var directoryListingResponse =
-          instantiateJerseyResponse(FileStoreDirectoryListingResponse.class);
-      final var directoryContents =
-          fileStore.list(path, null).stream()
-              .map(details -> convertToResponse(details))
-              .collect(Collectors.toList());
-      directoryListingResponse.files = Collections.singletonMap(path, 
directoryContents);
-      return directoryListingResponse;
+    if (type == FileStore.FileType.NOFILE
+        || type == FileStore.FileType.DIRECTORY
+        || (type == FileStore.FileType.FILE && Boolean.TRUE.equals(meta))) {
+      return ClusterFileStore.getMetadata(type, path, fileStore);
     }
-    if (Boolean.TRUE.equals(meta)) {
-      if (type == FileStore.FileType.FILE) {
-        int idx = path.lastIndexOf('/');
-        String fileName = path.substring(idx + 1);
-        String parentPath = path.substring(0, path.lastIndexOf('/'));
-        List<FileStore.FileDetails> l = fileStore.list(parentPath, s -> 
s.equals(fileName));
 
-        final var fileMetaResponse =
-            instantiateJerseyResponse(FileStoreDirectoryListingResponse.class);
-        fileMetaResponse.files =
-            Collections.singletonMap(path, l.isEmpty() ? null : 
convertToResponse(l.get(0)));
-        return fileMetaResponse;
-      }
-    } else { // User wants to get the "raw" file
-      // TODO Should we be trying to json-ify otherwise "raw" files in this 
way?  It seems like a
-      // pretty sketchy idea, esp. for code with very little test coverage.  
Consider removing
-      if ("json".equals(req.getParams().get(CommonParams.WT))) {
-        final var jsonResponse = 
instantiateJerseyResponse(FileStoreJsonFileResponse.class);
-        try {
-          fileStore.get(
-              pathCopy,
-              it -> {
-                try {
-                  InputStream inputStream = it.getInputStream();
-                  if (inputStream != null) {
-                    jsonResponse.response = new 
String(inputStream.readAllBytes(), UTF_8);
-                  }
-                } catch (IOException e) {
-                  throw new SolrException(
-                      SolrException.ErrorCode.SERVER_ERROR, "Error reading 
file " + pathCopy);
+    // User wants to get the "raw" file
+    // TODO Should we be trying to json-ify otherwise "raw" files in this way? 
 It seems like a

Review Comment:
   There's no discussion of it either in the PR or the JIRA that introduced it 
(unless I'm missing something?).  My guess is that it was an attempt at 
convenience?
   
   In any case; I think this entire file can now go.  See my comment 
[above](https://github.com/apache/solr/pull/3047/files#r1929771967) for more 
details.



-- 
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: issues-unsubscr...@solr.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to