feat: file sync #321
feat: file sync #321
Conversation
| @@ -0,0 +1,154 @@ | |||
| package org.zephyrsoft.sdb2.remote; | |||
There was a problem hiding this comment.
The file header is missing, containing license information.
| ImageIcon imageIcon = new ImageIcon(URI.create(imageUrl).toURL()); | ||
| URI imageUri = URI.create(imageUrl); | ||
| if (imageUri.getScheme().equals("sdb")){ | ||
| imageUri = Paths.get(FileAndDirectoryLocations.getDBBlobDir(), imageUrl.replace("sdb://", "")).toUri(); |
There was a problem hiding this comment.
This is kind of duplicated by the code in PresenterWindow.java around line 239. Maybe a common method handling this replacement could be added? Maybe even more high-level in logic flow, like after loading the songs database?
BTW: It should also be checked if the newly referenced file exists - if not, this will fail when trying to display the image.
| try { | ||
| Files.createDirectories(Paths.get(FileAndDirectoryLocations.getDBBlobDir())); | ||
| } catch (IOException e) { | ||
| e.printStackTrace(); |
There was a problem hiding this comment.
it would be better to log the exception, stdout is not watched
| try { | ||
| Files.write(Paths.get(FileAndDirectoryLocations.getDBBlobDir(), (String) args[0]), data); | ||
| } catch (IOException e) { | ||
| e.printStackTrace(); |
There was a problem hiding this comment.
it would be better to log the exception, stdout is not watched
| } | ||
|
|
||
| private Collection<String> getMissingFiles(Song song){ | ||
| HashSet<String> missingFiles = new HashSet<>(); |
There was a problem hiding this comment.
Why does this create a set if it adds at most one entry?
| try { | ||
| Files.createDirectories(Paths.get(FileAndDirectoryLocations.getDBBlobDir())); | ||
| } catch (IOException e) { | ||
| e.printStackTrace(); |
There was a problem hiding this comment.
it would be better to log the exception, stdout is not watched
| try { | ||
| Files.copy(path, dbPath, StandardCopyOption.COPY_ATTRIBUTES); | ||
| } catch (IOException e) { | ||
| e.printStackTrace(); |
There was a problem hiding this comment.
it would be better to log the exception, stdout is not watched
| byte[] content = Files.readAllBytes(dbPath); | ||
| remoteController.getFilesRequestSet().set(content, fileName); | ||
| } catch (IOException e) { | ||
| e.printStackTrace(); |
There was a problem hiding this comment.
it would be better to log the exception, stdout is not watched
| @@ -99,7 +100,7 @@ public void publish(String topic, byte[] payload, int qos, boolean retained) { | |||
| LOG.debug("Publishing message: {}", topic); | |||
| try { | |||
| client.publish(topic, payload, qos, retained); | |||
| LOG.debug("Payload: " + new String(payload, StandardCharsets.UTF_8)); | |||
| LOG.debug("Payload: " + new String(Arrays.copyOfRange(payload, 0, 50), StandardCharsets.UTF_8)); | |||
There was a problem hiding this comment.
What if a file is shorter than 50 bytes? This should not fail just because of logging - or if it does fail, it should give the correct cause (if an exception occurs here, the logged message would be "could not publish message" which is not true).
|
One more point: As you stated, there's no cleanup yet. Local files should be removed after 21 days if they are unused at that point (this policy is also in effect for |
This is a simple implementation of the db file sync api. If a playlist change contains images with a local url the files will be copied to a local blob directory and the provider will be set to sdb://. Then the file is uploaded using the api. If all file uploads succeed, the playlist change will be published. If a remote change happens, files not existing locally will be downloaded and then the playlist change is forwarded to the gui.
One note: