BD auth and directory creation added#26
BD auth and directory creation added#26Rainboom7 wants to merge 2 commits intotypeundefined:masterfrom
Conversation
typeundefined
left a comment
typeundefined
left a comment
There was a problem hiding this comment.
- Очень много "мусорных" правок, связанных с форматированием. Это только увеличивает дифф и ухудшает читаемость.
- Бизнес-логика не должна присутствовать в контроллере
- На мой взгляд, jpa здесь не очень оправдан, это как из пушки по воробьям. Но как бы и так можно, да)
| DirectoryContents dirContents; | ||
| if (path == null) { | ||
| dirContents = explorerService.getRootContents(); | ||
| System.out.println( path ); |
There was a problem hiding this comment.
Для логирования мы пользуемся логгером
| if (path == null) { | ||
| dirContents = explorerService.getRootContents(); | ||
| System.out.println( path ); | ||
| if ( path == null || "/".equals( path ) ) { |
There was a problem hiding this comment.
/, ./., //// будут резолвиться одинаково в один и тот же каталог. Эта проверка ненадежна
| data.put("directory", dirContents); | ||
| return new ModelAndView("filesView", data); | ||
| dirContents = explorerService.getContentsFiltered( path, new NamePartialMatcher( search ) ); | ||
| Map<String, Object> data = new HashMap<>( ); |
There was a problem hiding this comment.
А зачем появился этот дифф? Значимых правок нет, форматирование странноватое и не соответствует форматированию остального файла
| } | ||
|
|
||
| @RequestMapping(value = "/login", method = RequestMethod.GET) | ||
| public String loginPage(Model model) { |
There was a problem hiding this comment.
Лучше вынести в отдельный контроллер
|
|
||
|
|
||
| @RequestMapping(path = "/searchAll", method = RequestMethod.GET) | ||
| public ModelAndView searchAll(@RequestParam(name = "search") String search, |
There was a problem hiding this comment.
Для search тоже лучше завести отдельный контроллер, кстати
| @RequestMapping(value = "/createdir", method = RequestMethod.POST) | ||
| public String createDir( | ||
| @RequestParam("directory") String directory, | ||
| @RequestParam(name = "path", required = false) Optional<String> oPath, |
There was a problem hiding this comment.
Можно облегчить себе жизнь и заменить Optional<String> на Optional<Path>
| throws IOException { | ||
| String directoryPath; | ||
| String path = oPath.orElse( "" ); | ||
| if ( path == null ) directoryPath = explorerService.getRootPath( ).resolve( directory ).toString( ); |
There was a problem hiding this comment.
path здесь никогда не будет null
| String path = oPath.orElse( "" ); | ||
| if ( path == null ) directoryPath = explorerService.getRootPath( ).resolve( directory ).toString( ); | ||
| else directoryPath = explorerService.getAbsolutePath( Paths.get( path ) ).resolve( directory ).toString( ); | ||
| new File( directoryPath ).mkdir( ); |
There was a problem hiding this comment.
Это должно быть в FileSystemProvider. Контроллер не должен иметь бизнес-логики
|
|
||
| return (AppUser) query.getSingleResult(); | ||
| } catch (NoResultException e) { | ||
| return null; |
|
|
||
| public class WebUtils { | ||
|
|
||
| public static String toString(User user) { |
There was a problem hiding this comment.
А зачем здесь этот метод, тем более статический?
| import ru.amm.fileexplorer.server.config.entity.UserRole; | ||
|
|
||
| @Repository | ||
| @Transactional |
There was a problem hiding this comment.
Кстати, транзакция на уровне Repository это почти всегда плохая идея
2 participants
Second commit is just some ctrl+alt+l i forgot to press in first