From 8d81c8d3d085dccdda0b9cb64c351d961fc9837a Mon Sep 17 00:00:00 2001 From: Jonathan Narvaez Sosa Date: Tue, 18 Feb 2025 18:05:52 -0600 Subject: [PATCH 01/10] Move constants from interfaces to final classes --- .../jmetadata/ApplicationConstants.java | 111 ++++++++++-------- src/main/java/com/josdem/jmetadata/Auth.java | 9 +- 2 files changed, 69 insertions(+), 51 deletions(-) diff --git a/src/main/java/com/josdem/jmetadata/ApplicationConstants.java b/src/main/java/com/josdem/jmetadata/ApplicationConstants.java index 4df70e2f..a341ef53 100644 --- a/src/main/java/com/josdem/jmetadata/ApplicationConstants.java +++ b/src/main/java/com/josdem/jmetadata/ApplicationConstants.java @@ -16,52 +16,67 @@ package com.josdem.jmetadata; -public interface ApplicationConstants { - String LOGIN_FAIL = "Login fail"; - String LOGGED_AS = "Logged as : "; - String DONE = "Done"; - String OPEN_ERROR = "Error on importing Music"; - String WORKING = "Working"; - int ARTIST_COLUMN = 0; - int TITLE_COLUMN = 1; - int ALBUM_COLUMN = 2; - int GENRE_COLUMN = 3; - int YEAR_COLUMN = 4; - int TRACK_NUMBER_COLUMN = 5; - int TOTAL_TRACKS_NUMBER_COLUMN = 6; - int CD_NUMBER_COLUMN = 7; - int TOTAL_CDS_NUMBER_COLUMN = 8; - int STATUS_COLUMN = 9; - int WIDTH = 1024; - int HEIGHT = 600; - String COVER_ART_FROM_FILE = "Cover Art from File"; - String COVER_ART_FROM_LASTFM = "Cover Art from Lastfm"; - String COVER_ART_FROM_MUSIC_BRAINZ = "Cover Art from MusicBrainz"; - String COVER_ART_FROM_DRAG_AND_DROP = "Cover Art from Drag & Drop"; - String COVER_ART_DEFAULT = "Covert Art not found"; - String APPLY = "Apply"; - String EXPORT = "Export"; - String NEW = "New"; - String READY = "Ready"; - String APPLICATION_NAME = "JMetadata"; - String IMAGE_EXT = "PNG"; - String FILE_EXT = "txt"; - String PREFIX = "JMetadata_"; - String GETTING_ALBUM = "Getting Album from Musicbrainz"; - String GETTING_LAST_FM = "Getting Last.fm Metadata"; - String COMPLETE_DEFAULT_VALUES = "Completing default values"; - String GETTING_FORMATTER = "Formatting metadata"; - String WRITING_METADATA = "Writing Metadata"; - String USERNAME_LABEL = "username:"; - String PASSWORD_LABEL = "password:"; - int THREE_HUNDRED = 300; - String CORRUPTED_METADATA_LABEL = " has a corrupted coverArt"; - String METADATA_FROM_FILE_LABEL = " title and artist metadata were extracted from file name"; - String AND_ANOTHER = " and another "; - String DIRECTORY_EMPTY = "I could not find any mp3 or mp4 audio file in the directory"; - String DIRECTORY_NOT_FOUND = "I could not find that directory to scan: "; - String FILE_NOT_FOUND = "I could not find this file: "; - String TOO_MUCH_FILES_LOADED = "Too much files loaded, maximum allowed: "; - String MEDIA_TYPE = "application/json"; - String USER_AGENT = "JMetadata/1.1.0 (contact@josdem.io)"; +public final class ApplicationConstants { + + private ApplicationConstants() {} + + public static final String LOGIN_FAIL = "Login fail"; + public static final String LOGGED_AS = "Logged as : "; + public static final String DONE = "Done"; + public static final String OPEN_ERROR = "Error on importing Music"; + public static final String WORKING = "Working"; + + public static final int ARTIST_COLUMN = 0; + public static final int TITLE_COLUMN = 1; + public static final int ALBUM_COLUMN = 2; + public static final int GENRE_COLUMN = 3; + public static final int YEAR_COLUMN = 4; + public static final int TRACK_NUMBER_COLUMN = 5; + public static final int TOTAL_TRACKS_NUMBER_COLUMN = 6; + public static final int CD_NUMBER_COLUMN = 7; + public static final int TOTAL_CDS_NUMBER_COLUMN = 8; + public static final int STATUS_COLUMN = 9; + + public static final int WIDTH = 1024; + public static final int HEIGHT = 600; + + public static final String COVER_ART_FROM_FILE = "Cover Art from File"; + public static final String COVER_ART_FROM_LASTFM = "Cover Art from Lastfm"; + public static final String COVER_ART_FROM_MUSIC_BRAINZ = "Cover Art from MusicBrainz"; + public static final String COVER_ART_FROM_DRAG_AND_DROP = "Cover Art from Drag & Drop"; + public static final String COVER_ART_DEFAULT = "Cover Art not found"; + + public static final String APPLY = "Apply"; + public static final String EXPORT = "Export"; + public static final String NEW = "New"; + public static final String READY = "Ready"; + + public static final String APPLICATION_NAME = "JMetadata"; + public static final String IMAGE_EXT = "PNG"; + public static final String FILE_EXT = "txt"; + public static final String PREFIX = "JMetadata_"; + + public static final String GETTING_ALBUM = "Getting Album from Musicbrainz"; + public static final String GETTING_LAST_FM = "Getting Last.fm Metadata"; + public static final String COMPLETE_DEFAULT_VALUES = "Completing default values"; + public static final String GETTING_FORMATTER = "Formatting metadata"; + public static final String WRITING_METADATA = "Writing Metadata"; + + public static final String USERNAME_LABEL = "username:"; + public static final String PASSWORD_LABEL = "password:"; + + public static final int THREE_HUNDRED = 300; + + public static final String CORRUPTED_METADATA_LABEL = " has a corrupted coverArt"; + public static final String METADATA_FROM_FILE_LABEL = + " title and artist metadata were extracted from file name"; + public static final String AND_ANOTHER = " and another "; + public static final String DIRECTORY_EMPTY = + "I could not find any mp3 or mp4 audio file in the directory"; + public static final String DIRECTORY_NOT_FOUND = "I could not find that directory to scan: "; + public static final String FILE_NOT_FOUND = "I could not find this file: "; + public static final String TOO_MUCH_FILES_LOADED = "Too much files loaded, maximum allowed: "; + + public static final String MEDIA_TYPE = "application/json"; + public static final String USER_AGENT = "JMetadata/1.1.0 (contact@josdem.io)"; } diff --git a/src/main/java/com/josdem/jmetadata/Auth.java b/src/main/java/com/josdem/jmetadata/Auth.java index 760fc12d..aa8dbf4e 100644 --- a/src/main/java/com/josdem/jmetadata/Auth.java +++ b/src/main/java/com/josdem/jmetadata/Auth.java @@ -16,7 +16,10 @@ package com.josdem.jmetadata; -public interface Auth { - static final String KEY = "LASTFM_API_KEY"; - static final String SECRET = "LASTFM_API_SECRET"; +public final class Auth { + + private Auth() {} + + public static final String KEY = "LASTFM_API_KEY"; + public static final String SECRET = "LASTFM_API_SECRET"; } From 310ff5d615ce950b907262606afaacc8fdedc71a Mon Sep 17 00:00:00 2001 From: Jonathan Narvaez Sosa Date: Tue, 18 Feb 2025 23:51:14 -0600 Subject: [PATCH 02/10] Rename enum constants to match naming convention --- .../josdem/jmetadata/action/ActionResult.java | 22 ++++++++--------- .../controller/CompleteController.java | 18 +++++++------- .../controller/DefaultController.java | 4 ++-- .../controller/ExporterController.java | 2 +- .../controller/FormatterController.java | 2 +- .../controller/ScrobblerController.java | 2 +- .../com/josdem/jmetadata/gui/MainWindow.java | 24 +++++++++---------- .../jmetadata/helper/ExporterHelper.java | 2 +- .../jmetadata/helper/ScrobblerHelper.java | 12 +++++----- .../impl/LastFMCompleteServiceImpl.java | 4 ++-- .../service/impl/LastfmServiceImpl.java | 8 +++---- .../MusicBrainzCompleteServiceAdapter.java | 2 +- .../controller/CompleteControllerTest.java | 8 +++---- .../controller/DefaultControllerTest.java | 4 ++-- .../controller/ExporterControllerTest.java | 6 ++--- .../controller/FormatterControllerTest.java | 6 ++--- .../controller/ScrobblerControllerTest.java | 12 +++++----- .../jmetadata/helper/ScrobblerHelperTest.java | 8 +++---- .../service/LastFMCompleteServiceTest.java | 4 ++-- .../jmetadata/service/LastfmServiceTest.java | 18 +++++++------- 20 files changed, 84 insertions(+), 84 deletions(-) diff --git a/src/main/java/com/josdem/jmetadata/action/ActionResult.java b/src/main/java/com/josdem/jmetadata/action/ActionResult.java index 0cb1bff4..70f7d805 100644 --- a/src/main/java/com/josdem/jmetadata/action/ActionResult.java +++ b/src/main/java/com/josdem/jmetadata/action/ActionResult.java @@ -17,15 +17,15 @@ package com.josdem.jmetadata.action; public enum ActionResult { - Ready, - New, - Error, - Not_Scrobbleable, - Sessionless, - NotFound, - Updated, - NotLogged, - Sent, - Exported, - Complete + READY, + NEW, + ERROR, + NOT_SCROBBLEABLE, + SESSIONLESS, + NOT_FOUND, + UPDATED, + NOT_LOGGED, + SENT, + EXPORTED, + COMPLETE } diff --git a/src/main/java/com/josdem/jmetadata/controller/CompleteController.java b/src/main/java/com/josdem/jmetadata/controller/CompleteController.java index 2762d13d..759d8dcd 100644 --- a/src/main/java/com/josdem/jmetadata/controller/CompleteController.java +++ b/src/main/java/com/josdem/jmetadata/controller/CompleteController.java @@ -66,7 +66,7 @@ void setup() { @RequestMethod(Actions.COMPLETE_ALBUM_METADATA) public synchronized ActionResult completeAlbumMetadata(List metadataList) { if (!musicBrainzCompleteServiceAdapter.canComplete(metadataList)) { - return ActionResult.Ready; + return ActionResult.READY; } try { if (ApplicationState.cache.get(metadataList.getFirst().getAlbum()) == null) { @@ -84,7 +84,7 @@ public synchronized ActionResult completeAlbumMetadata(List metadataLi ApplicationState.cache.put(albumName, musicBrainzResponse); log.info("MusicBrainz Response: {}", musicBrainzResponse); if (musicBrainzResponse.getReleases().isEmpty()) { - return ActionResult.Ready; + return ActionResult.READY; } Album album = musicBrainzService.getAlbumByName(albumName); log.info("MusicBrainz Album: {}", album); @@ -102,23 +102,23 @@ public synchronized ActionResult completeAlbumMetadata(List metadataLi } else { log.error("Error getting releases: {}", result.errorBody()); } - return ActionResult.New; + return ActionResult.NEW; } } catch (IOException ex) { log.error(ex.getMessage(), ex); - return ActionResult.Error; + return ActionResult.ERROR; } - return ActionResult.Complete; + return ActionResult.COMPLETE; } @RequestMethod(Actions.COMPLETE_LAST_FM_METADATA) public ActionResult completeLastFmMetadata(List metadataList) { log.info("trying to complete using LastFM service"); if (!lastFMCompleteServiceAdapter.canComplete(metadataList)) { - return ActionResult.Ready; + return ActionResult.READY; } metadataList.forEach(lastfmService::completeLastFM); - return ActionResult.New; + return ActionResult.NEW; } @RequestMethod(Actions.WRITE_METADATA) @@ -144,10 +144,10 @@ public synchronized ActionResult completeAlbum(Metadata metadata) { metadataWriter.writeCoverArt(coverArtNew.getImage()); metadata.setCoverArt(coverArtNew.getImage()); } - return ActionResult.Updated; + return ActionResult.UPDATED; } catch (MetadataException mde) { log.error(mde.getMessage(), mde); - return ActionResult.Error; + return ActionResult.ERROR; } } } diff --git a/src/main/java/com/josdem/jmetadata/controller/DefaultController.java b/src/main/java/com/josdem/jmetadata/controller/DefaultController.java index 8f754d63..4442a875 100644 --- a/src/main/java/com/josdem/jmetadata/controller/DefaultController.java +++ b/src/main/java/com/josdem/jmetadata/controller/DefaultController.java @@ -45,8 +45,8 @@ public synchronized ActionResult complete(List metadatas) MetadataException { if (defaultService.isCompletable(metadatas)) { defaultService.complete(metadatas); - return ActionResult.New; + return ActionResult.NEW; } - return ActionResult.Ready; + return ActionResult.READY; } } diff --git a/src/main/java/com/josdem/jmetadata/controller/ExporterController.java b/src/main/java/com/josdem/jmetadata/controller/ExporterController.java index 0c1d874e..b44d711b 100644 --- a/src/main/java/com/josdem/jmetadata/controller/ExporterController.java +++ b/src/main/java/com/josdem/jmetadata/controller/ExporterController.java @@ -44,7 +44,7 @@ public ActionResult sendMetadata(ExportPackage exportPackage) return exporterHelper.export(exportPackage); } catch (IOException ioe) { log.error(ioe.getMessage(), ioe); - return ActionResult.Error; + return ActionResult.ERROR; } } } diff --git a/src/main/java/com/josdem/jmetadata/controller/FormatterController.java b/src/main/java/com/josdem/jmetadata/controller/FormatterController.java index eb5a2550..b4d76c9e 100644 --- a/src/main/java/com/josdem/jmetadata/controller/FormatterController.java +++ b/src/main/java/com/josdem/jmetadata/controller/FormatterController.java @@ -35,6 +35,6 @@ public synchronized ActionResult format(Metadata metadata) { formatterService.isAnalyzable(metadata); Boolean formatted = formatterService.wasFormatted(metadata); Boolean capitalized = formatterService.wasCamelized(metadata); - return formatted || capitalized ? ActionResult.New : ActionResult.Ready; + return formatted || capitalized ? ActionResult.NEW : ActionResult.READY; } } diff --git a/src/main/java/com/josdem/jmetadata/controller/ScrobblerController.java b/src/main/java/com/josdem/jmetadata/controller/ScrobblerController.java index b5f532d1..91bfba1b 100644 --- a/src/main/java/com/josdem/jmetadata/controller/ScrobblerController.java +++ b/src/main/java/com/josdem/jmetadata/controller/ScrobblerController.java @@ -48,7 +48,7 @@ public ActionResult sendMetadata(Metadata metadata) { return scrobblerHelper.send(metadata); } catch (IOException | InterruptedException ioe) { log.error(ioe.getMessage(), ioe); - return ActionResult.Error; + return ActionResult.ERROR; } } } diff --git a/src/main/java/com/josdem/jmetadata/gui/MainWindow.java b/src/main/java/com/josdem/jmetadata/gui/MainWindow.java index 4019549d..2ece99d6 100644 --- a/src/main/java/com/josdem/jmetadata/gui/MainWindow.java +++ b/src/main/java/com/josdem/jmetadata/gui/MainWindow.java @@ -228,7 +228,7 @@ public void observe(ObservValue t) { metadataWithAlbum.add(metadata); getDescriptionTable() .getModel() - .setValueAt(ActionResult.New, selectedRow, ApplicationConstants.STATUS_COLUMN); + .setValueAt(ActionResult.NEW, selectedRow, ApplicationConstants.STATUS_COLUMN); getApplyButton().setEnabled(!working); log.info("setting applyButton to : " + !working); } @@ -428,7 +428,7 @@ void onReadyToApplyMetadata(MetadataAlbumValues metadataValues) { metadataWithAlbum.add(metadata); getDescriptionTable() .getModel() - .setValueAt(ActionResult.New, i, ApplicationConstants.STATUS_COLUMN); + .setValueAt(ActionResult.NEW, i, ApplicationConstants.STATUS_COLUMN); if (i == selectedRow) { updateImage(i); } @@ -629,7 +629,7 @@ private JLabel getLoginLabel() { public JButton getCompleteMetadataButton() { if (completeMetadataButton == null) { - completeMetadataButton = new JButton(ActionResult.Complete.toString()); + completeMetadataButton = new JButton(ActionResult.COMPLETE.toString()); completeMetadataButton.setName(COMPLETE_BUTTON_NAME); completeMetadataButton.setEnabled(false); completeMetadataButton.setBounds(COMPLETE_BUTTON_BOUNDS); @@ -793,7 +793,7 @@ public void tableChanged(TableModelEvent e) { getDescriptionTable() .getModel() - .setValueAt(ActionResult.New, row, ApplicationConstants.STATUS_COLUMN); + .setValueAt(ActionResult.NEW, row, ApplicationConstants.STATUS_COLUMN); controlEngineConfigurator .getControlEngine() @@ -952,7 +952,7 @@ public void onResponse(ActionResult response) { "response in getting information from MusicBrainz track{}: {}", metadata.getTitle(), response); - if (response.equals(ActionResult.New)) { + if (response.equals(ActionResult.NEW)) { metadataWithAlbum.add(metadata); getDescriptionTable() .getModel() @@ -1013,7 +1013,7 @@ public void onResponse(ActionResult response) { + metadata.getTitle() + ": " + response); - if (response.equals(ActionResult.New)) { + if (response.equals(ActionResult.NEW)) { metadataWithAlbum.add(metadata); getDescriptionTable() .getModel() @@ -1036,7 +1036,7 @@ public void onResponse(ActionResult response) { } else if (!getDescriptionTable() .getModel() .getValueAt(i, ApplicationConstants.STATUS_COLUMN) - .equals(ActionResult.New)) { + .equals(ActionResult.NEW)) { getDescriptionTable() .getModel() .setValueAt( @@ -1071,7 +1071,7 @@ public void onResponse(ActionResult response) { + metadata.getTitle() + ": " + response); - if (response.equals(ActionResult.New)) { + if (response.equals(ActionResult.NEW)) { for (Metadata metadata : metadataList) { int i = metadataList.indexOf(metadata); updateStatus(counter++, metadataList.size()); @@ -1098,7 +1098,7 @@ public void onResponse(ActionResult response) { } else if (!getDescriptionTable() .getModel() .getValueAt(i, ApplicationConstants.STATUS_COLUMN) - .equals(ActionResult.New)) { + .equals(ActionResult.NEW)) { getDescriptionTable() .getModel() .setValueAt( @@ -1130,7 +1130,7 @@ public void onResponse(ActionResult response) { + metadata.getTitle() + ": " + response); - if (response.equals(ActionResult.New)) { + if (response.equals(ActionResult.NEW)) { metadataWithAlbum.add(metadata); getDescriptionTable() .getModel() @@ -1165,7 +1165,7 @@ public void onResponse(ActionResult response) { getDescriptionTable() .getModel() .setValueAt( - ActionResult.New, + ActionResult.NEW, i, ApplicationConstants.STATUS_COLUMN); if (metadata.getNewCoverArt() != null @@ -1175,7 +1175,7 @@ public void onResponse(ActionResult response) { } else if (!getDescriptionTable() .getModel() .getValueAt(i, ApplicationConstants.STATUS_COLUMN) - .equals(ActionResult.New)) { + .equals(ActionResult.NEW)) { getDescriptionTable() .getModel() .setValueAt( diff --git a/src/main/java/com/josdem/jmetadata/helper/ExporterHelper.java b/src/main/java/com/josdem/jmetadata/helper/ExporterHelper.java index cf3b8784..4fe22a0a 100644 --- a/src/main/java/com/josdem/jmetadata/helper/ExporterHelper.java +++ b/src/main/java/com/josdem/jmetadata/helper/ExporterHelper.java @@ -49,6 +49,6 @@ public ActionResult export(ExportPackage exportPackage) exportPackage.setMetadataList(metadatas); imageExporter.export(exportPackage); metadataExporter.export(exportPackage); - return ActionResult.Exported; + return ActionResult.EXPORTED; } } diff --git a/src/main/java/com/josdem/jmetadata/helper/ScrobblerHelper.java b/src/main/java/com/josdem/jmetadata/helper/ScrobblerHelper.java index 0b9dcf9a..abe5e69b 100644 --- a/src/main/java/com/josdem/jmetadata/helper/ScrobblerHelper.java +++ b/src/main/java/com/josdem/jmetadata/helper/ScrobblerHelper.java @@ -56,7 +56,7 @@ public class ScrobblerHelper { private ActionResult scrobbling(Metadata metadata) throws IOException, InterruptedException { User currentUser = controlEngine.get(Model.CURRENT_USER); if (StringUtils.isEmpty(currentUser.getUsername())) { - return ActionResult.NotLogged; + return ActionResult.NOT_LOGGED; } if (currentUser.getSession() != null) { @@ -70,11 +70,11 @@ private ActionResult scrobbling(Metadata metadata) throws IOException, Interrupt return future.get(); } catch (ExecutionException eex) { log.error(eex.getMessage(), eex); - return ActionResult.Error; + return ActionResult.ERROR; } } else { log.error("There isn't a valid session"); - return ActionResult.Sessionless; + return ActionResult.SESSIONLESS; } } @@ -89,7 +89,7 @@ public ActionResult send(Metadata metadata) throws IOException, InterruptedExcep metadataMap.put(metadata, startTime); return scrobbling(metadata); } - return ActionResult.Not_Scrobbleable; + return ActionResult.NOT_SCROBBLEABLE; } public void setControlEngine(ControlEngine controlEngine) { @@ -119,10 +119,10 @@ public ActionResult call() throws Exception { + " - " + metadata.getTitle() + " scrobbling to Last.fm was Successful"); - return ActionResult.Sent; + return ActionResult.SENT; } else { log.error("Submitting track " + metadata.getTitle() + " to Last.fm failed: " + result); - return ActionResult.Error; + return ActionResult.ERROR; } } } diff --git a/src/main/java/com/josdem/jmetadata/service/impl/LastFMCompleteServiceImpl.java b/src/main/java/com/josdem/jmetadata/service/impl/LastFMCompleteServiceImpl.java index 119fa3ea..e8f3233d 100644 --- a/src/main/java/com/josdem/jmetadata/service/impl/LastFMCompleteServiceImpl.java +++ b/src/main/java/com/josdem/jmetadata/service/impl/LastFMCompleteServiceImpl.java @@ -128,7 +128,7 @@ public ActionResult isSomethingNew(LastfmAlbum lastfmAlbum, Metadata metadata) { if (lastfmAlbum.getImageIcon() == null && StringUtils.isEmpty(lastfmAlbum.getYear()) && StringUtils.isEmpty(lastfmAlbum.getGenre())) { - return ActionResult.Ready; + return ActionResult.READY; } if (lastfmAlbum.getImageIcon() != null) { CoverArt coverArt = new CoverArt(lastfmAlbum.getImageIcon(), CoverArtType.LAST_FM); @@ -140,6 +140,6 @@ public ActionResult isSomethingNew(LastfmAlbum lastfmAlbum, Metadata metadata) { if (StringUtils.isEmpty(metadata.getGenre())) { metadata.setGenre(lastfmAlbum.getGenre()); } - return ActionResult.New; + return ActionResult.NEW; } } diff --git a/src/main/java/com/josdem/jmetadata/service/impl/LastfmServiceImpl.java b/src/main/java/com/josdem/jmetadata/service/impl/LastfmServiceImpl.java index 6a31e0a4..5f271254 100644 --- a/src/main/java/com/josdem/jmetadata/service/impl/LastfmServiceImpl.java +++ b/src/main/java/com/josdem/jmetadata/service/impl/LastfmServiceImpl.java @@ -40,17 +40,17 @@ public synchronized ActionResult completeLastFM(Metadata metadata) { LastfmAlbum lastfmAlbum = completeService.getLastFM(metadata); return completeService.isSomethingNew(lastfmAlbum, metadata); } else { - return ActionResult.Ready; + return ActionResult.READY; } } catch (MalformedURLException mfe) { log.error("MalformedURLException: {}", mfe.getMessage(), mfe); - return ActionResult.Error; + return ActionResult.ERROR; } catch (IOException ioe) { log.error("IOException: {}", ioe.getMessage(), ioe); - return ActionResult.Error; + return ActionResult.ERROR; } catch (IllegalStateException ise) { log.error("Missing environment variables for LastFM API Key or Secret: {}", ise.getMessage()); - return ActionResult.Error; + return ActionResult.ERROR; } } } diff --git a/src/main/java/com/josdem/jmetadata/service/impl/MusicBrainzCompleteServiceAdapter.java b/src/main/java/com/josdem/jmetadata/service/impl/MusicBrainzCompleteServiceAdapter.java index b2ef1d3b..3a6c549f 100644 --- a/src/main/java/com/josdem/jmetadata/service/impl/MusicBrainzCompleteServiceAdapter.java +++ b/src/main/java/com/josdem/jmetadata/service/impl/MusicBrainzCompleteServiceAdapter.java @@ -60,6 +60,6 @@ public ActionResult isSomethingNew(AlbumInfo albumInfo, Metadata metadata) { metadata.setYear(albumInfo.getYear()); updated = true; } - return updated ? ActionResult.New : ActionResult.Ready; + return updated ? ActionResult.NEW : ActionResult.READY; } } diff --git a/src/test/java/com/josdem/jmetadata/controller/CompleteControllerTest.java b/src/test/java/com/josdem/jmetadata/controller/CompleteControllerTest.java index 8610ac3d..cc687f68 100644 --- a/src/test/java/com/josdem/jmetadata/controller/CompleteControllerTest.java +++ b/src/test/java/com/josdem/jmetadata/controller/CompleteControllerTest.java @@ -77,7 +77,7 @@ public void setup() throws Exception { when(metadata.getTotalCds()).thenReturn(totalCds); when(metadata.getYear()).thenReturn(year); when(metadata.getGenre()).thenReturn(genre); - when(coverArtService.completeLastFM(metadata)).thenReturn(ActionResult.Ready); + when(coverArtService.completeLastFM(metadata)).thenReturn(ActionResult.READY); controller = new CompleteController( metadataWriter, @@ -103,7 +103,7 @@ public void shouldCompleteAlbumInMetadata() throws Exception { verify(metadataWriter).writeTotalCds(totalCds); verify(metadataWriter).writeYear(year); verify(metadataWriter).writeGenre(genre); - assertEquals(ActionResult.Updated, result); + assertEquals(ActionResult.UPDATED, result); } @Test @@ -116,7 +116,7 @@ public void shouldRemoveCoverArt() throws Exception { verify(metadataWriter).removeCoverArt(); verify(metadataWriter).writeCoverArt(imageIcon); verify(metadata).setCoverArt(imageIcon); - assertEquals(ActionResult.Updated, result); + assertEquals(ActionResult.UPDATED, result); } @Test @@ -126,7 +126,7 @@ public void shouldNotUpdateMetadata() throws Exception { ActionResult result = controller.completeAlbum(metadata); - assertEquals(ActionResult.Error, result); + assertEquals(ActionResult.ERROR, result); } @Test diff --git a/src/test/java/com/josdem/jmetadata/controller/DefaultControllerTest.java b/src/test/java/com/josdem/jmetadata/controller/DefaultControllerTest.java index 5d48836e..5371a172 100644 --- a/src/test/java/com/josdem/jmetadata/controller/DefaultControllerTest.java +++ b/src/test/java/com/josdem/jmetadata/controller/DefaultControllerTest.java @@ -57,7 +57,7 @@ void shouldCompleteTotalTracks(TestInfo testInfo) throws Exception { var result = defaultController.complete(metadatas); verify(defaultService).complete(metadatas); - assertEquals(ActionResult.New, result); + assertEquals(ActionResult.NEW, result); } @Test @@ -68,6 +68,6 @@ void shouldNotCompleteTotalTracks(TestInfo testInfo) throws Exception { var result = defaultController.complete(metadatas); - assertEquals(ActionResult.Ready, result); + assertEquals(ActionResult.READY, result); } } diff --git a/src/test/java/com/josdem/jmetadata/controller/ExporterControllerTest.java b/src/test/java/com/josdem/jmetadata/controller/ExporterControllerTest.java index e0a6d6e7..c06a38e7 100644 --- a/src/test/java/com/josdem/jmetadata/controller/ExporterControllerTest.java +++ b/src/test/java/com/josdem/jmetadata/controller/ExporterControllerTest.java @@ -50,12 +50,12 @@ void setup() { @DisplayName("sending metadata") void shouldSendMetadata(TestInfo testInfo) throws Exception { log.info(testInfo.getDisplayName()); - when(exporterHelper.export(exportPackage)).thenReturn(ActionResult.Exported); + when(exporterHelper.export(exportPackage)).thenReturn(ActionResult.EXPORTED); var result = exporterController.sendMetadata(exportPackage); verify(exporterHelper).export(exportPackage); - assertEquals(ActionResult.Exported, result); + assertEquals(ActionResult.EXPORTED, result); } @Test @@ -66,6 +66,6 @@ void shouldReportErrorInSendingMetadata(TestInfo testInfo) throws Exception { var result = exporterController.sendMetadata(exportPackage); - assertEquals(ActionResult.Error, result); + assertEquals(ActionResult.ERROR, result); } } diff --git a/src/test/java/com/josdem/jmetadata/controller/FormatterControllerTest.java b/src/test/java/com/josdem/jmetadata/controller/FormatterControllerTest.java index 3f6bb5ec..0fb06263 100644 --- a/src/test/java/com/josdem/jmetadata/controller/FormatterControllerTest.java +++ b/src/test/java/com/josdem/jmetadata/controller/FormatterControllerTest.java @@ -45,7 +45,7 @@ public void shouldFormatWhenBadFormat() throws Exception { when(formatterService.wasFormatted(metadata)).thenReturn(true); when(formatterService.wasCamelized(metadata)).thenReturn(false); ActionResult result = formatterController.format(metadata); - assertEquals(ActionResult.New, result); + assertEquals(ActionResult.NEW, result); } @Test @@ -53,7 +53,7 @@ public void shouldFormatWhenNotCamelized() throws Exception { when(formatterService.wasFormatted(metadata)).thenReturn(false); when(formatterService.wasCamelized(metadata)).thenReturn(true); ActionResult result = formatterController.format(metadata); - assertEquals(ActionResult.New, result); + assertEquals(ActionResult.NEW, result); } @Test @@ -61,6 +61,6 @@ public void shouldReturnComplete() throws Exception { when(formatterService.wasFormatted(metadata)).thenReturn(false); when(formatterService.wasCamelized(metadata)).thenReturn(false); ActionResult result = formatterController.format(metadata); - assertEquals(ActionResult.Ready, result); + assertEquals(ActionResult.READY, result); } } diff --git a/src/test/java/com/josdem/jmetadata/controller/ScrobblerControllerTest.java b/src/test/java/com/josdem/jmetadata/controller/ScrobblerControllerTest.java index 031e8a8f..fa25a57b 100644 --- a/src/test/java/com/josdem/jmetadata/controller/ScrobblerControllerTest.java +++ b/src/test/java/com/josdem/jmetadata/controller/ScrobblerControllerTest.java @@ -55,24 +55,24 @@ void setup() { @DisplayName("sending metadata") void shouldSendMetadata(TestInfo testInfo) throws Exception { log.info(testInfo.getDisplayName()); - when(scrobblerHelper.send(metadata)).thenReturn(ActionResult.New); + when(scrobblerHelper.send(metadata)).thenReturn(ActionResult.NEW); var result = controller.sendMetadata(metadata); verify(scrobblerHelper).send(metadata); - assertEquals(ActionResult.New, result); + assertEquals(ActionResult.NEW, result); } @Test @DisplayName("detecting error in scrobbling") void shouldDetectWhenErrorInScrobbling(TestInfo testInfo) throws Exception { log.info(testInfo.getDisplayName()); - when(scrobblerHelper.send(metadata)).thenReturn(ActionResult.Error); + when(scrobblerHelper.send(metadata)).thenReturn(ActionResult.ERROR); var result = controller.sendMetadata(metadata); verify(scrobblerHelper).send(metadata); - assertEquals(ActionResult.Error, result); + assertEquals(ActionResult.ERROR, result); } @Test @@ -91,7 +91,7 @@ void shouldCatchIOException(TestInfo testInfo) throws Exception { var result = controller.sendMetadata(metadata); - assertEquals(ActionResult.Error, result); + assertEquals(ActionResult.ERROR, result); } @Test @@ -102,6 +102,6 @@ void shouldCatchInterruptedException(TestInfo testInfo) throws Exception { var result = controller.sendMetadata(metadata); - assertEquals(ActionResult.Error, result); + assertEquals(ActionResult.ERROR, result); } } diff --git a/src/test/java/com/josdem/jmetadata/helper/ScrobblerHelperTest.java b/src/test/java/com/josdem/jmetadata/helper/ScrobblerHelperTest.java index 4c9d52a0..3ae9b982 100644 --- a/src/test/java/com/josdem/jmetadata/helper/ScrobblerHelperTest.java +++ b/src/test/java/com/josdem/jmetadata/helper/ScrobblerHelperTest.java @@ -85,7 +85,7 @@ public void shouldNotAddAScrobblingIfNoArtist() throws Exception { private void notSendToScrobblingMapAssertion() { verify(metadataMap, never()).size(); verify(metadataMap, never()).put(isA(Metadata.class), isA(Long.class)); - assertEquals(ActionResult.Not_Scrobbleable, result); + assertEquals(ActionResult.NOT_SCROBBLEABLE, result); } @Test @@ -112,7 +112,7 @@ public void shouldFailWhenSubmitScrobbler() throws Exception { currentUser.getSession())) .thenReturn(result); - assertEquals(ActionResult.Sessionless, helperScrobbler.send(metadata)); + assertEquals(ActionResult.SESSIONLESS, helperScrobbler.send(metadata)); } private void setMetadataTrackExpectations() { @@ -136,7 +136,7 @@ public void shouldSendAnScrobbler() throws Exception { currentUser.getSession())) .thenReturn(result); - assertEquals(ActionResult.Sent, helperScrobbler.send(metadata)); + assertEquals(ActionResult.SENT, helperScrobbler.send(metadata)); } private void setExpectations() { @@ -150,6 +150,6 @@ public void shouldReturnIfNoLogin() throws Exception { setMetadataTrackExpectations(); when(currentUser.getUsername()).thenReturn(StringUtils.EMPTY); - assertEquals(ActionResult.NotLogged, helperScrobbler.send(metadata)); + assertEquals(ActionResult.NOT_LOGGED, helperScrobbler.send(metadata)); } } diff --git a/src/test/java/com/josdem/jmetadata/service/LastFMCompleteServiceTest.java b/src/test/java/com/josdem/jmetadata/service/LastFMCompleteServiceTest.java index ab7b278b..1e796f73 100644 --- a/src/test/java/com/josdem/jmetadata/service/LastFMCompleteServiceTest.java +++ b/src/test/java/com/josdem/jmetadata/service/LastFMCompleteServiceTest.java @@ -186,7 +186,7 @@ private void setImageExpectations() throws IOException { public void shouldDetectWhenNothingChanged(TestInfo testInfo) { log.info(testInfo.getDisplayName()); var actionResult = completeService.isSomethingNew(lastfmAlbum, metadata); - assertEquals(ActionResult.Ready, actionResult); + assertEquals(ActionResult.READY, actionResult); } @Test @@ -196,7 +196,7 @@ void shouldDetectLastfmHasNewValues(TestInfo testInfo) { when(lastfmAlbum.getYear()).thenReturn(year); var actionResult = completeService.isSomethingNew(lastfmAlbum, metadata); - assertEquals(ActionResult.New, actionResult); + assertEquals(ActionResult.NEW, actionResult); } @Test diff --git a/src/test/java/com/josdem/jmetadata/service/LastfmServiceTest.java b/src/test/java/com/josdem/jmetadata/service/LastfmServiceTest.java index 3f0cfeec..93ada57a 100644 --- a/src/test/java/com/josdem/jmetadata/service/LastfmServiceTest.java +++ b/src/test/java/com/josdem/jmetadata/service/LastfmServiceTest.java @@ -54,12 +54,12 @@ void setup() throws Exception { void shouldCompleteMetadataFromLastfm() throws Exception { setCompleteHelperExpectations(); when(lastfmAlbum.getImageIcon()).thenReturn(imageIcon); - when(completeService.isSomethingNew(lastfmAlbum, metadata)).thenReturn(ActionResult.New); + when(completeService.isSomethingNew(lastfmAlbum, metadata)).thenReturn(ActionResult.NEW); ActionResult result = lastfmService.completeLastFM(metadata); verify(completeService).isSomethingNew(lastfmAlbum, metadata); - assertEquals(ActionResult.New, result); + assertEquals(ActionResult.NEW, result); } @Test @@ -67,7 +67,7 @@ void shouldNotCompleteLastfmCoverArtMetadataDueToMetadataComplete() throws Excep when(completeService.canLastFMHelpToComplete(metadata)).thenReturn(false); ActionResult result = lastfmService.completeLastFM(metadata); - assertEquals(ActionResult.Ready, result); + assertEquals(ActionResult.READY, result); } @Test @@ -83,20 +83,20 @@ void shouldNotCompleteGenreIfAlredyHasOne() throws Exception { @Test void shouldReturnMetadataCompleteIfLastfmHasNotNewValues() throws Exception { setCompleteHelperExpectations(); - when(completeService.isSomethingNew(lastfmAlbum, metadata)).thenReturn(ActionResult.Ready); + when(completeService.isSomethingNew(lastfmAlbum, metadata)).thenReturn(ActionResult.READY); ActionResult result = lastfmService.completeLastFM(metadata); - assertEquals(ActionResult.Ready, result); + assertEquals(ActionResult.READY, result); } @Test void shouldReturnSomethingnewValueIfNoBadFormatAndCapitalized() throws Exception { setCompleteHelperExpectations(); - when(completeService.isSomethingNew(lastfmAlbum, metadata)).thenReturn(ActionResult.New); + when(completeService.isSomethingNew(lastfmAlbum, metadata)).thenReturn(ActionResult.NEW); ActionResult result = lastfmService.completeLastFM(metadata); - assertEquals(ActionResult.New, result); + assertEquals(ActionResult.NEW, result); } void setCompleteHelperExpectations() throws MalformedURLException, IOException { @@ -110,7 +110,7 @@ void shouldCatchMalformedURLException() throws Exception { ActionResult result = lastfmService.completeLastFM(metadata); - assertEquals(ActionResult.Error, result); + assertEquals(ActionResult.ERROR, result); } @Test @@ -119,6 +119,6 @@ void shouldCatchIOException() throws Exception { ActionResult result = lastfmService.completeLastFM(metadata); - assertEquals(ActionResult.Error, result); + assertEquals(ActionResult.ERROR, result); } } From 7c403a1182694f40773e96406c000a9698cbc96b Mon Sep 17 00:00:00 2001 From: Jonathan Narvaez Sosa Date: Tue, 18 Feb 2025 23:56:37 -0600 Subject: [PATCH 03/10] Move constants from interface Actions to final class --- .../com/josdem/jmetadata/action/Actions.java | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/src/main/java/com/josdem/jmetadata/action/Actions.java b/src/main/java/com/josdem/jmetadata/action/Actions.java index b231bb46..25a07b1a 100644 --- a/src/main/java/com/josdem/jmetadata/action/Actions.java +++ b/src/main/java/com/josdem/jmetadata/action/Actions.java @@ -27,39 +27,39 @@ import org.asmatron.messengine.action.RequestAction; import org.asmatron.messengine.action.ValueAction; -/** - * @author josdem (joseluis.delacruz@gmail.com) - * @understands A class who has Publisher Subscriber label descriptors - */ -public interface Actions { - String LOGIN_ID = "login"; - ActionId> LOGIN = cm(LOGIN_ID); - - String GET_METADATA = "getMetadata"; - ActionId METADATA = cm(GET_METADATA); - - String SEND_METADATA = "sendMetadata"; - ActionId> SEND = cm(SEND_METADATA); - - String COMPLETE_ALBUM_METADATA = "completeAlbumMetadata"; - ActionId, ActionResult>> COMPLETE_MUSICBRAINZ = +public final class Actions { + + private Actions() {} + + public static final String LOGIN_ID = "login"; + public static final ActionId> LOGIN = cm(LOGIN_ID); + + public static final String GET_METADATA = "getMetadata"; + public static final ActionId METADATA = cm(GET_METADATA); + + public static final String SEND_METADATA = "sendMetadata"; + public static final ActionId> SEND = cm(SEND_METADATA); + + public static final String COMPLETE_ALBUM_METADATA = "completeAlbumMetadata"; + public static final ActionId, ActionResult>> COMPLETE_MUSICBRAINZ = cm(COMPLETE_ALBUM_METADATA); - String COMPLETE_LAST_FM_METADATA = "completeLastfmMetadata"; - ActionId, ActionResult>> COMPLETE_LAST_FM = + public static final String COMPLETE_LAST_FM_METADATA = "completeLastfmMetadata"; + public static final ActionId, ActionResult>> COMPLETE_LAST_FM = cm(COMPLETE_LAST_FM_METADATA); - String WRITE_METADATA = "writeMetadata"; - ActionId> WRITE = cm(WRITE_METADATA); + public static final String WRITE_METADATA = "writeMetadata"; + public static final ActionId> WRITE = cm(WRITE_METADATA); - String EXPORT_METADATA = "exportMetadata"; - ActionId> EXPORT = cm(EXPORT_METADATA); + public static final String EXPORT_METADATA = "exportMetadata"; + public static final ActionId> EXPORT = + cm(EXPORT_METADATA); - String COMPLETE_FORMATTER_METADATA = "completeFormatterMetadata"; - ActionId> COMPLETE_FORMATTER = + public static final String COMPLETE_FORMATTER_METADATA = "completeFormatterMetadata"; + public static final ActionId> COMPLETE_FORMATTER = cm(COMPLETE_FORMATTER_METADATA); - String COMPLETE_DEFAULT_METADATA = "completeDefaultMetadata"; - ActionId, ActionResult>> COMPLETE_DEFAULT = + public static final String COMPLETE_DEFAULT_METADATA = "completeDefaultMetadata"; + public static final ActionId, ActionResult>> COMPLETE_DEFAULT = cm(COMPLETE_DEFAULT_METADATA); } From 13b72936d04f1f7353aef23a5b13e6383a4fe705 Mon Sep 17 00:00:00 2001 From: Jonathan Narvaez Sosa Date: Sat, 22 Feb 2025 14:22:17 -0600 Subject: [PATCH 04/10] Remove unnecessary thrown exceptions from DefaultController --- .../jmetadata/controller/DefaultController.java | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/main/java/com/josdem/jmetadata/controller/DefaultController.java b/src/main/java/com/josdem/jmetadata/controller/DefaultController.java index 4442a875..9da14b6b 100644 --- a/src/main/java/com/josdem/jmetadata/controller/DefaultController.java +++ b/src/main/java/com/josdem/jmetadata/controller/DefaultController.java @@ -18,16 +18,11 @@ import com.josdem.jmetadata.action.ActionResult; import com.josdem.jmetadata.action.Actions; -import com.josdem.jmetadata.exception.MetadataException; import com.josdem.jmetadata.model.Metadata; import com.josdem.jmetadata.service.DefaultService; -import java.io.IOException; import java.util.List; import lombok.RequiredArgsConstructor; import org.asmatron.messengine.annotations.RequestMethod; -import org.jaudiotagger.audio.exceptions.CannotReadException; -import org.jaudiotagger.audio.exceptions.ReadOnlyFileException; -import org.jaudiotagger.tag.TagException; import org.springframework.stereotype.Controller; @Controller @@ -37,12 +32,7 @@ public class DefaultController { private final DefaultService defaultService; @RequestMethod(Actions.COMPLETE_DEFAULT_METADATA) - public synchronized ActionResult complete(List metadatas) - throws IOException, - CannotReadException, - TagException, - ReadOnlyFileException, - MetadataException { + public synchronized ActionResult complete(List metadatas) { if (defaultService.isCompletable(metadatas)) { defaultService.complete(metadatas); return ActionResult.NEW; From e4c9973e307eb8b27d0d5b6bc8dd210d03cf0761 Mon Sep 17 00:00:00 2001 From: Jonathan Narvaez Sosa Date: Sat, 22 Feb 2025 15:03:27 -0600 Subject: [PATCH 05/10] Fix InterruptedException handling and reduce cognitive complexity --- .../controller/MetadataController.java | 68 ++++++++++--------- .../controller/ScrobblerController.java | 6 +- 2 files changed, 41 insertions(+), 33 deletions(-) diff --git a/src/main/java/com/josdem/jmetadata/controller/MetadataController.java b/src/main/java/com/josdem/jmetadata/controller/MetadataController.java index e9b4db48..079192c4 100644 --- a/src/main/java/com/josdem/jmetadata/controller/MetadataController.java +++ b/src/main/java/com/josdem/jmetadata/controller/MetadataController.java @@ -63,49 +63,53 @@ public void getMetadata() { if (root.exists()) { configurator .getControlEngine() - .fireEvent(Events.DIRECTORY_SELECTED, new ValueEvent(root.getAbsolutePath())); - try { - metadataList = metadataService.extractMetadata(root); - if (metadataList.isEmpty()) { - configurator.getControlEngine().fireEvent(Events.DIRECTORY_EMPTY); - } else { - Collections.sort(metadataList); - sendLoadedEvent(metadataList); - } - } catch (IOException - | TagException - | ReadOnlyFileException - | InvalidAudioFrameException - | InvalidId3VersionException - | InterruptedException - | CannotReadException - | MetadataException e) { - handleException(e); - } catch (IllegalArgumentException e) { - sendLoadedEvent(metadataList); - handleException(e); - } catch (TooMuchFilesException e) { - log.error(e.getMessage(), e); - String maxFilesAllowed = properties.getProperty("max.files.allowed"); - configurator - .getControlEngine() - .fireEvent(Events.MUCH_FILES_LOADED, new ValueEvent(maxFilesAllowed)); - } + .fireEvent(Events.DIRECTORY_SELECTED, new ValueEvent<>(root.getAbsolutePath())); + processMetadata(root); } else { configurator .getControlEngine() - .fireEvent(Events.DIRECTORY_NOT_EXIST, new ValueEvent(root.toString())); + .fireEvent(Events.DIRECTORY_NOT_EXIST, new ValueEvent<>(root.toString())); } } else { configurator.getControlEngine().fireEvent(Events.DIRECTORY_SELECTED_CANCEL); } } + private void processMetadata(File root) { + try { + metadataList = metadataService.extractMetadata(root); + if (metadataList.isEmpty()) { + configurator.getControlEngine().fireEvent(Events.DIRECTORY_EMPTY); + } else { + Collections.sort(metadataList); + sendLoadedEvent(metadataList); + } + } catch (IOException + | TagException + | ReadOnlyFileException + | InvalidAudioFrameException + | InvalidId3VersionException + | CannotReadException + | MetadataException e) { + handleException(e); + } catch (IllegalArgumentException e) { + sendLoadedEvent(metadataList); + handleException(e); + } catch (TooMuchFilesException e) { + log.error(e.getMessage(), e); + String maxFilesAllowed = properties.getProperty("max.files.allowed"); + configurator + .getControlEngine() + .fireEvent(Events.MUCH_FILES_LOADED, new ValueEvent<>(maxFilesAllowed)); + } catch (InterruptedException e) { + log.warn("Thread was interrupted", e); + Thread.currentThread().interrupt(); + } + } + private void sendLoadedEvent(List metadataList) { configurator.getControlEngine().set(Model.METADATA, metadataList, null); - configurator - .getControlEngine() - .fireEvent(Events.LOAD, new ValueEvent>(metadataList)); + configurator.getControlEngine().fireEvent(Events.LOAD, new ValueEvent<>(metadataList)); configurator.getControlEngine().fireEvent(Events.LOADED); } diff --git a/src/main/java/com/josdem/jmetadata/controller/ScrobblerController.java b/src/main/java/com/josdem/jmetadata/controller/ScrobblerController.java index 91bfba1b..468207dc 100644 --- a/src/main/java/com/josdem/jmetadata/controller/ScrobblerController.java +++ b/src/main/java/com/josdem/jmetadata/controller/ScrobblerController.java @@ -46,9 +46,13 @@ public ActionResult sendMetadata(Metadata metadata) { try { log.info("Sending scrobbling for: {}", metadata.getTitle()); return scrobblerHelper.send(metadata); - } catch (IOException | InterruptedException ioe) { + } catch (IOException ioe) { log.error(ioe.getMessage(), ioe); return ActionResult.ERROR; + } catch (InterruptedException e) { + log.warn("Thread was interrupted", e); + Thread.currentThread().interrupt(); + return ActionResult.ERROR; } } } From 49cb8efeb42fcfd4c5b803ce54cf3c7e8821c625 Mon Sep 17 00:00:00 2001 From: Jonathan Narvaez Sosa Date: Sat, 22 Feb 2025 15:11:55 -0600 Subject: [PATCH 06/10] Refactor: Rename shadowing variable 'listeners' to 'componentListeners' --- .../jmetadata/dnd/DnDListenerCollection.java | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/josdem/jmetadata/dnd/DnDListenerCollection.java b/src/main/java/com/josdem/jmetadata/dnd/DnDListenerCollection.java index 9f663cfb..246e8c63 100644 --- a/src/main/java/com/josdem/jmetadata/dnd/DnDListenerCollection.java +++ b/src/main/java/com/josdem/jmetadata/dnd/DnDListenerCollection.java @@ -25,19 +25,19 @@ @Slf4j public class DnDListenerCollection { - private Map> listeners = new HashMap>(); + private Map> listeners = new HashMap<>(); public void put(Component component, T listener) { - List listeners = null; + List componentListeners = null; synchronized (this.listeners) { - listeners = this.listeners.get(component); - if (listeners == null) { - listeners = new ArrayList(); - this.listeners.put(component, listeners); + componentListeners = this.listeners.get(component); + if (componentListeners == null) { + componentListeners = new ArrayList<>(); + this.listeners.put(component, componentListeners); } } - synchronized (listeners) { - for (T t : listeners) { + synchronized (componentListeners) { + for (T t : componentListeners) { if (t.getClass().equals(listener.getClass()) && t.equals(listener)) { log.warn( "DUPLICATE HANDLER FOR COMPONENT: " @@ -47,7 +47,7 @@ public void put(Component component, T listener) { return; } } - listeners.add(listener); + componentListeners.add(listener); } } @@ -56,23 +56,23 @@ public void remove(Component component) { } public void remove(Component component, T listener) { - List listeners = this.listeners.get(component); - if (listeners != null) { - listeners.remove(listener); - if (listeners.isEmpty()) { + List componentListeners = this.listeners.get(component); + if (componentListeners != null) { + componentListeners.remove(listener); + if (componentListeners.isEmpty()) { this.listeners.remove(component); } } } public DnDListenerEntries getInmediateEntries(Class clazz, Component component) { - DnDListenerEntries entries = new DnDListenerEntries(); + DnDListenerEntries entries = new DnDListenerEntries<>(); if (component == null) { return entries; } - List listeners = this.listeners.get(component); - if (listeners != null) { - for (T t : listeners) { + List componentListeners = this.listeners.get(component); + if (componentListeners != null) { + for (T t : componentListeners) { entries.put(clazz, component, t); } } @@ -84,7 +84,7 @@ public DnDListenerEntries getInmediateEntries(Class clazz, Component compo } public DnDListenerEntries getUpwardEntries(Class clazz, Component c) { - DnDListenerEntries dragOverListeners = new DnDListenerEntries(); + DnDListenerEntries dragOverListeners = new DnDListenerEntries<>(); addListenersToMotionMap(clazz, c, dragOverListeners); return dragOverListeners; } @@ -93,9 +93,9 @@ private void addListenersToMotionMap(Class clazz, Component c, DnDListenerEnt if (c == null) { return; } - List list = listeners.get(c); - if (list != null) { - for (T t : list) { + List componentListeners = listeners.get(c); + if (componentListeners != null) { + for (T t : componentListeners) { result.put(clazz, c, t); } } From e48273597c4eb59261f55d22a0eb81c63a296fc5 Mon Sep 17 00:00:00 2001 From: Jonathan Narvaez Sosa Date: Sat, 22 Feb 2025 15:18:03 -0600 Subject: [PATCH 07/10] Refactor: Throw UnsupportedOperationException in unimplemented methods --- .../jmetadata/dnd/DragAndDropActionEmpty.java | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/josdem/jmetadata/dnd/DragAndDropActionEmpty.java b/src/main/java/com/josdem/jmetadata/dnd/DragAndDropActionEmpty.java index 8f0d4ab7..03f70a0b 100644 --- a/src/main/java/com/josdem/jmetadata/dnd/DragAndDropActionEmpty.java +++ b/src/main/java/com/josdem/jmetadata/dnd/DragAndDropActionEmpty.java @@ -30,10 +30,14 @@ public boolean isDone() { } @Override - public void dragExit() {} + public void dragExit() { + throw new UnsupportedOperationException("dragExit() is not implemented"); + } @Override - public void setLocation(Point location) {} + public void setLocation(Point location) { + throw new UnsupportedOperationException("setLocation() is not implemented"); + } @Override public ObserverCollection> onComponentChangedListener() { @@ -41,10 +45,14 @@ public ObserverCollection> onComponentChangedListener() { } @Override - public void setDropListeners(DnDListenerEntries dropListeners) {} + public void setDropListeners(DnDListenerEntries dropListeners) { + throw new UnsupportedOperationException("setDropListeners() is not implemented"); + } @Override - public void setDragListeners(DnDListenerEntries dragListeners) {} + public void setDragListeners(DnDListenerEntries dragListeners) { + throw new UnsupportedOperationException("setDragListeners() is not implemented"); + } @Override public boolean validate(Point point) { @@ -57,11 +65,13 @@ public boolean isDragObjectSet() { } @Override - public void setDragObject(DraggedObject draggedObject) {} + public void setDragObject(DraggedObject draggedObject) { + throw new UnsupportedOperationException("setDragObject() is not implemented"); + } @Override public boolean drop(Point location) { - return false; + throw new UnsupportedOperationException("drop() is not implemented"); } @Override From 091cdaeb60065fa8f001d89ea2be5dc75a7773a1 Mon Sep 17 00:00:00 2001 From: Jonathan Narvaez Sosa Date: Sat, 22 Feb 2025 15:36:19 -0600 Subject: [PATCH 08/10] Refactor: Rename local variables to avoid shadowing class fields --- .../jmetadata/dnd/DragAndDropActionImpl.java | 73 ++++++++++--------- 1 file changed, 37 insertions(+), 36 deletions(-) diff --git a/src/main/java/com/josdem/jmetadata/dnd/DragAndDropActionImpl.java b/src/main/java/com/josdem/jmetadata/dnd/DragAndDropActionImpl.java index c5b1b127..17cd36c1 100644 --- a/src/main/java/com/josdem/jmetadata/dnd/DragAndDropActionImpl.java +++ b/src/main/java/com/josdem/jmetadata/dnd/DragAndDropActionImpl.java @@ -49,19 +49,19 @@ public boolean validate(final Point point) { } Boolean result = false; if (isDragObjectSet()) { - final DraggedObject draggedObject = this.draggedObject; - final DnDListenerEntries dropListeners = this.dropListeners; + final DraggedObject currentDraggedObject = this.draggedObject; + final DnDListenerEntries currentDropListeners = this.dropListeners; result = - dropListeners.forEach( - DragAndDropIterators.validateDrop(draggedObject, point, currentFrame)); + currentDropListeners.forEach( + DragAndDropIterators.validateDrop(currentDraggedObject, point, currentFrame)); result = DragAndDropIterators.createBoolean(result); } else { result = true; } if (this.validationResult != result.booleanValue()) { - final DnDListenerEntries dragListeners = this.dragListeners; + final DnDListenerEntries currentDragListeners = this.dragListeners; this.validationResult = result.booleanValue(); - dragListeners.forEach(DragAndDropIterators.dragAllowedChanged(result.booleanValue())); + currentDragListeners.forEach(DragAndDropIterators.dragAllowedChanged(result.booleanValue())); } return validationResult; } @@ -73,30 +73,31 @@ public void dragExit() { @Override public boolean drop(final Point point) { - final DraggedObject draggedObject = this.draggedObject; - final DnDListenerEntries dropListeners = this.dropListeners; - final DnDListenerEntries dragListeners = this.dragListeners; - final Container currentFrame = this.currentFrame; + final DraggedObject currentDraggedObject = this.draggedObject; + final DnDListenerEntries currentDropListeners = this.dropListeners; + final DnDListenerEntries currentDragListeners = this.dragListeners; + final Container currentFrameInstance = this.currentFrame; final boolean isDone = isDone(); final boolean isDragObjectSet = isDragObjectSet(); stop(true); boolean success = false; if (!(isDone || !isDragObjectSet)) { Boolean result = - dropListeners.forEach(DragAndDropIterators.doDrop(point, draggedObject, currentFrame)); + currentDropListeners.forEach( + DragAndDropIterators.doDrop(point, currentDraggedObject, currentFrameInstance)); success = DragAndDropIterators.createBoolean(result); } - dragListeners.forEach(DragAndDropIterators.dropOcurred(success)); + currentDragListeners.forEach(DragAndDropIterators.dropOcurred(success)); return success; } - public void setLocation(Point location) { + public void setLocation(Point newLocation) { if (isDone()) { return; } - if (!this.location.equals(location)) { - this.location = location; - Component newComponent = currentFrame.findComponentAt(location); + if (!this.location.equals(newLocation)) { + this.location = newLocation; + Component newComponent = currentFrame.findComponentAt(newLocation); if (newComponent != null && !newComponent.equals(currentComponent)) { currentComponent = newComponent; currentComponentChanged.fire(new ObservValue(newComponent)); @@ -105,39 +106,39 @@ public void setLocation(Point location) { } @Override - public void setDragObject(DraggedObject draggedObject) { - if (this.draggedObject != draggedObject) { - this.draggedObject = draggedObject; + public void setDragObject(DraggedObject newDraggedObject) { + if (this.draggedObject != newDraggedObject) { + this.draggedObject = newDraggedObject; currentComponentChanged.fire(new ObservValue(currentComponent)); } } - public void setDropListeners(DnDListenerEntries dropListeners) { - if (dropListeners == null || isDone()) { - dropListeners = DnDListenerEntries.empty(); + public void setDropListeners(DnDListenerEntries newDropListeners) { + if (newDropListeners == null || isDone()) { + newDropListeners = DnDListenerEntries.empty(); } - this.dropListeners = dropListeners; + this.dropListeners = newDropListeners; validate(location); } - public void setDragListeners(DnDListenerEntries dragListeners) { - setDragListeners(dragListeners, false); + public void setDragListeners(DnDListenerEntries newDragListeners) { + setDragListeners(newDragListeners, false); } private void setDragListeners( - DnDListenerEntries dragListeners, boolean isForDrop) { - if (dragListeners == null || isDone()) { - dragListeners = DnDListenerEntries.empty(); + DnDListenerEntries newDragListeners, boolean isForDrop) { + if (newDragListeners == null || isDone()) { + newDragListeners = DnDListenerEntries.empty(); } - Point location = this.location; + Point currentLocation = this.location; DnDListenerEntries lastListeners = this.dragListeners; - lastListeners.forEachExclude(DragAndDropIterators.dragExit(isForDrop), dragListeners); + lastListeners.forEachExclude(DragAndDropIterators.dragExit(isForDrop), newDragListeners); DraggedObject draggedObj = draggedObject; boolean valRes = validationResult; Container frame = currentFrame; - dragListeners.forEachExclude( - DragAndDropIterators.dragEnter(location, draggedObj, valRes, frame), lastListeners); - this.dragListeners = dragListeners; + newDragListeners.forEachExclude( + DragAndDropIterators.dragEnter(currentLocation, draggedObj, valRes, frame), lastListeners); + this.dragListeners = newDragListeners; } @Override @@ -181,8 +182,8 @@ protected void updateDragOver() { if (isDone()) { return; } - final Point point = location; - final DnDListenerEntries dragListeners = this.dragListeners; - dragListeners.forEach(DragAndDropIterators.updateLocation(point, currentFrame)); + final Point currentPoint = location; + final DnDListenerEntries currentDragListeners = this.dragListeners; + currentDragListeners.forEach(DragAndDropIterators.updateLocation(currentPoint, currentFrame)); } } From 151c723f6dcd5ff56eac04773a952263dc81bc36 Mon Sep 17 00:00:00 2001 From: Jonathan Narvaez Sosa Date: Sat, 22 Feb 2025 15:46:59 -0600 Subject: [PATCH 09/10] Refactor: Add private constructor to utility class and remove commented-out code --- .../jmetadata/dnd/DragAndDropIterators.java | 67 +++++++------------ 1 file changed, 26 insertions(+), 41 deletions(-) diff --git a/src/main/java/com/josdem/jmetadata/dnd/DragAndDropIterators.java b/src/main/java/com/josdem/jmetadata/dnd/DragAndDropIterators.java index 56681852..5c62ee00 100644 --- a/src/main/java/com/josdem/jmetadata/dnd/DragAndDropIterators.java +++ b/src/main/java/com/josdem/jmetadata/dnd/DragAndDropIterators.java @@ -58,6 +58,32 @@ public static DoInListeners dragAllowedChanged(boolean v return new DragAllowedChangedIterator(validationResult); } + private static final class DoDropIterator implements DoInListeners { + private final Point point; + private final DraggedObject draggedObject; + private final Container currentFrame; + + private DoDropIterator(Point point, DraggedObject draggedObject, Container currentFrame) { + this.point = point; + this.draggedObject = draggedObject; + this.currentFrame = currentFrame; + } + + @Override + public Boolean doIn(DropListener listener, Component component, Boolean lastResult) { + log("doDrop", component, listener); + Point location = convertToComponentRelativePoint(point, currentFrame, component); + boolean validateDrop = listener.validateDrop(draggedObject, location); + if (validateDrop) { + listener.doDrop(draggedObject, location); + } + return createBoolean(validateDrop, lastResult); + } + + @Override + public void done() {} + } + private static final class DragEnterIterator extends DoInListenersAdapter { private final Point point; @@ -137,47 +163,6 @@ public Void doIn(DragOverListener listener, Component component, Void lastResult } } - private static final class DoDropIterator implements DoInListeners { - private final Point point; - private final DraggedObject draggedObject; - private final Container currentFrame; - - // private ExecutorService executorService; - - private DoDropIterator(Point point, DraggedObject draggedObject, Container currentFrame) { - this.point = point; - this.draggedObject = draggedObject; - this.currentFrame = currentFrame; - // executorService = Executors.newSingleThreadExecutor(); - } - - @Override - public Boolean doIn(DropListener listener, Component component, Boolean lastResult) { - log("doDrop", component, listener); - Point location = convertToComponentRelativePoint(point, currentFrame, component); - boolean validateDrop = listener.validateDrop(draggedObject, location); - if (validateDrop) { - doDrop(listener, draggedObject, location); - } - return createBoolean(validateDrop, lastResult); - } - - private void doDrop( - final DropListener listener, final DraggedObject draggedObject, final Point location) { - // executorService.execute(new Runnable() { - // @Override - // public void run() { - listener.doDrop(draggedObject, location); - // } - // }); - } - - @Override - public void done() { - // executorService.shutdown(); - } - } - private static final class ValidateDropIterator extends DoInListenersAdapter { private final DraggedObject draggedObject; From 525ea47b8e88dad64797fc5ecb9ba1763f0d5813 Mon Sep 17 00:00:00 2001 From: Jonathan Narvaez Sosa Date: Mon, 24 Feb 2025 00:34:30 -0600 Subject: [PATCH 10/10] Refactor: Apply pattern matching for instanceof and return empty array instead of null --- .../josdem/jmetadata/dnd/MainFrameDragOverListener.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/josdem/jmetadata/dnd/MainFrameDragOverListener.java b/src/main/java/com/josdem/jmetadata/dnd/MainFrameDragOverListener.java index 32113af0..479901f5 100644 --- a/src/main/java/com/josdem/jmetadata/dnd/MainFrameDragOverListener.java +++ b/src/main/java/com/josdem/jmetadata/dnd/MainFrameDragOverListener.java @@ -33,8 +33,8 @@ public MainFrameDragOverListener(Window frame) { @Override public void dragEnter(DraggedObject draggedObject) { - if (frame instanceof Frame) { - dialog = new DragTooltipDialog((Frame) frame); + if (frame instanceof Frame windowFrame) { + dialog = new DragTooltipDialog(windowFrame); } else { dialog = new DragTooltipDialog(null); } @@ -79,7 +79,7 @@ public void dropOcurred(boolean success) { @Override public void updateLocation(Point location) { if (dialog != null) { - Point p = (Point) location.clone(); + Point p = new Point(location); p.x = p.x + 5; SwingUtilities.convertPointToScreen(p, frame); dialog.setLocation(p); @@ -95,6 +95,6 @@ public void dragAllowedChanged(boolean allowed) { @Override public Class[] handledTypes() { - return null; + return new Class[0]; } }