hierynomus / smbj

Server Message Block (SMB2, SMB3) implementation in Java
Other
707 stars 180 forks source link

Added List files #754

Open rokkakasu opened 1 year ago

rokkakasu commented 1 year ago

Added List files method to list all the files under the directory recursively using single session.

hierynomus commented 1 year ago

Hi @rokkakasu. Thank you for your effort. However, in this PR's current state, I won't merge it. Things like these do not belong in the core classes of the library. The core classes expose the base functionality. If I would merge this it would hide a huge performance penalty. These kind of things belong in classes built on top of the library. This could be added to for instance SmbFiles.

Also the code quality is well below standards of this repository. There are a lot of indentation errors.

rokkakasu commented 1 year ago

Hi @hierynomus Kindly reuse the part of the code I have provided so that it will be useful for many library users. in present implementation to list files in each subfolders a separate NTLM auth is needed, with this methods that can be avoided. Kindly use the Code in any form. and feel free to reject the PR.

Thanks, @rokkakasu

laeubi commented 1 year ago

@rokkakasu I think using a Stream would have many benefits, and is used in the NIO API as well, this will allow to iterate but still not eager fetch everything.

rokkakasu commented 1 year ago

Sure.

On Fri, 21 Apr, 2023, 12:58 pm Christoph Läubrich, @.***> wrote:

@.**** commented on this pull request.

In src/main/java/com/hierynomus/smbj/utils/SmbFileAttributeUtils.java https://github.com/hierynomus/smbj/pull/754#discussion_r1173409803:

  • *
  • *
    • 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.
  • */ +package com.hierynomus.smbj.utils;
  • +/**

    • This utility class provides utilities for file attributes.
  • *
  • */ +public class SmbFileAttributeUtils {

Could you maybe submit this as a separate PR to SmbFiles this is really useful and I missing such a good way to check if its a file or directory!

— Reply to this email directly, view it on GitHub https://github.com/hierynomus/smbj/pull/754#pullrequestreview-1395215183, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIJCEGXJ476X3Y6KUPMBXY3XCIZJFANCNFSM6AAAAAAWAC7M2Q . You are receiving this because you were mentioned.Message ID: @.***>

rokkakasu commented 1 year ago

Hi @hierynomus/smbj @.***>

May i move this method from diskshare to Smbfiles class .

Will you approve my PR.

This utility can through all sub folders without creating new sessions for each sub directories.

Thanks @rokkakasu.

On Sun, 19 Mar, 2023, 8:13 pm Jeroen van Erp, @.***> wrote:

Hi @rokkakasu https://github.com/rokkakasu. Thank you for your effort. However, in this PR's current state, I won't merge it. Things like these do not belong in the core classes of the library. The core classes expose the base functionality. If I would merge this it would hide a huge performance penalty. These kind of things belong in classes built on top of the library. This could be added to for instance SmbFiles.

Also the code quality is well below standards of this repository. There are a lot of indentation errors.

— Reply to this email directly, view it on GitHub https://github.com/hierynomus/smbj/pull/754#issuecomment-1475280029, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIJCEGTHIRHHNGTQDNUPFO3W44LQRANCNFSM6AAAAAAWAC7M2Q . You are receiving this because you were mentioned.Message ID: @.***>

hierynomus commented 1 year ago

Let me re-iterate. In the current implementation this will not be approved nor merged into the master branch. Submit a good PR, containing code, tests and proper formatting. Also not duplicating any constants already defined in the library, then I'll review and think about merging.

hierynomus commented 1 year ago

As said initiallly, this code does not belong in the core classes of the library, but instead in a utilities class, such as SmbFiles.