Browse files

Made AbstractPage and its descendants reasonably thread-safe. (Hmm...)

  • Loading branch information...
1 parent 1dd5ff9 commit 417ebc867ff5e57df633471ed0eddc7105d30f9b @Acerbic committed Sep 23, 2011
View
2 .classpath
@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<classpath>
<classpathentry kind="src" path="src"/>
- <classpathentry including="dloader/*.java" kind="src" path="test"/>
+ <classpathentry including="**/*.java" kind="src" path="test"/>
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-1.7"/>
<classpathentry exported="true" kind="lib" path="lib/entagged-audioformats-0.15.jar" sourcepath="D:/Gleb/JavaLibrary/entagged-audioformats-source-0.15"/>
<classpathentry exported="true" kind="lib" path="lib/jaxen-1.1.3.jar">
View
4 .project
@@ -16,7 +16,7 @@
</natures>
<filteredResources>
<filter>
- <id>1316336340453</id>
+ <id>1316782643042</id>
<name></name>
<type>6</type>
<matcher>
@@ -25,7 +25,7 @@
</matcher>
</filter>
<filter>
- <id>1316336340468</id>
+ <id>1316782643057</id>
<name></name>
<type>6</type>
<matcher>
View
2 src/dloader/PageProcessor.java
@@ -173,7 +173,7 @@ else if (!job.saveTo.equals(saveTo)) {
* @return new PageParser descendant fitting for the page
* @throws IllegalArgumentException - when baseURL is bad or null
*/
- static final
+ static final public
AbstractPage detectPage(String baseURL) throws IllegalArgumentException {
URL u;
try {
View
47 src/dloader/page/AbstractPage.java
@@ -56,7 +56,7 @@ public ProblemsReadingDocumentException(Throwable e) {
/**
* title of this item (as stored into cache)
*/
- private String title;
+ private volatile String title;
/**
* url of a page referencing this item
@@ -67,33 +67,25 @@ public ProblemsReadingDocumentException(Throwable e) {
* array of a children items to this one (can be of size zero, can be null).
* Certain elements of this array can be null as well (if child parsing failed)
*/
- private AbstractPage[] childPages;
+ private volatile AbstractPage[] childPages;
/**
* reference to a parent item (may be null)
*/
- private AbstractPage parent;
+ private volatile AbstractPage parent;
/**
* Inherited by all descendants and instances, providing unified logging.
*/
protected static Logger logger = Logger.getLogger(Logger.GLOBAL_LOGGER_NAME);;
- public synchronized final AbstractPage[] getChildPages() {
- return childPages;
- }
-
- public synchronized final void setChildPages(AbstractPage[] childPages) {
- this.childPages = childPages;
- }
-
- public synchronized final AbstractPage getParent() {
- return parent;
- }
+// these three are not synchronized because they only read values that declared volatile
+ public final AbstractPage[] getChildPages() { return childPages; }
+ public final AbstractPage getParent() { return parent; }
+ public String getTitle() {return title;}
+
+ public synchronized void setTitle(String title) {this.title = title;}
- public synchronized final void setParent(AbstractPage parent) {
- this.parent = parent;
- }
/**
* Constructs from web address
@@ -116,24 +108,15 @@ public AbstractPage(URL url) throws IllegalArgumentException {
this.url = url;
}
- public synchronized
- String getTitle() {
- return title;
- }
-
- public synchronized
- void setTitle(String title) {
- this.title = title;
- }
-
/**
* Convert a string to a proper file name (NOT path, only filename).
* (don't check for existing file collisions, only validness of a name)
* @param name - string to convert
* @return proper file name
* @throws IOException if file name is not valid
*/
- public static String getFSSafeName(String name) throws IOException {
+ public static final
+ String getFSSafeName(String name) throws IOException {
for (char c : ":/\\*?\"<>|\t\n\r".toCharArray())
name = name.replace(String.valueOf(c), "");
name = name.trim(); // only trailing spaces are forbidden
@@ -360,9 +343,8 @@ protected final URL resolveLink(String link) throws MalformedURLException {
void saveToCache (org.jdom.Document doc) {
assert (doc != null);
- // absolutely required fields
- if (getTitle()==null || url==null)
- return;
+ // absolutely required field
+ if (getTitle()==null) return;
//1. Compose this one and childrefs
Element e = getSpecificDataXML();
if (e==null) return; // element is corrupt and should not be cached
@@ -396,7 +378,7 @@ void saveToCache (org.jdom.Document doc) {
*/
private
Element scanXMLForThisElement(org.jdom.Document doc) {
- assert (doc != null); assert (url != null);
+ assert (doc != null);
String searchXPath = String.format("//%s[@url='%s']",getClass().getSimpleName(),url.toString());
List<?> result = queryXPathList(searchXPath, doc);
@@ -414,6 +396,7 @@ Element scanXMLForThisElement(org.jdom.Document doc) {
* Checks if call to saveResult can be skipped (especially if it is a long operation)
* If the effect of saveResult is a minor thing (like simple directory creation) this function is
* allowed to make call to saveResult and return true;
+ * @param saveTo - path to where this item will be saved in consequent call to saveResult
* @return true if call to saveResult can be skipped (will yield no effect), \t false if it must be performed
*/
abstract public boolean isSavingNotRequired(String saveTo);
View
36 src/dloader/page/Album.java
@@ -24,7 +24,7 @@
/**
* link to the album cover
*/
- public URL coverUrl;
+ public volatile URL coverUrl; // hope volatile is enough
/**
* arbitrary additional info
*/
@@ -39,18 +39,31 @@
public Album(String s) throws IllegalArgumentException {super(s);}
+ /**
+ * Builds path to save cover image to disk
+ * @param saveTo - saving path for parenting item
+ * @return path to album cover image
+ * @throws IOException
+ */
+ public String getCoverSavePath(String saveTo) throws IOException {
+ Path p = Paths.get(getChildrenSaveTo(saveTo), "cover.jpg");
+ return p.toString();
+ }
+
@Override
- public String saveResult(String saveTo) throws IOException {
+ public synchronized
+ String saveResult(String saveTo) throws IOException {
Path p = Paths.get(saveTo, getFSSafeName(getTitle()));
Files.createDirectories(p);
- if (WebDownloader.fetchWebFile(coverUrl, p.resolve("cover.jpg").toString()) != 0)
+ if (WebDownloader.fetchWebFile(coverUrl, getCoverSavePath(saveTo)) != 0)
return "cover image downloaded";
else return null;
}
@Override
- protected void readCacheSelf(Element e) throws ProblemsReadingDocumentException{
+ protected
+ void readCacheSelf(Element e) throws ProblemsReadingDocumentException{
try {
coverUrl = resolveLink(e.getAttributeValue("coverUrl"));
} catch (MalformedURLException e1) {
@@ -68,7 +81,8 @@ protected Element getSpecificDataXML() {
}
@Override
- protected AbstractPage parseChild(Element element) throws ProblemsReadingDocumentException {
+ protected
+ AbstractPage parseChild(Element element) throws ProblemsReadingDocumentException {
try {
trackCounter++; // that includes counting for failed parsing
URL u = resolveLink(element.getAttributeValue("href"));
@@ -84,7 +98,8 @@ protected AbstractPage parseChild(Element element) throws ProblemsReadingDocumen
}
@Override
- protected void parseSelf(Document doc) throws ProblemsReadingDocumentException {
+ protected
+ void parseSelf(Document doc) throws ProblemsReadingDocumentException {
@SuppressWarnings("unchecked")
List<Element> imgList = (List<Element>) queryXPathList("//pre:div[@id='tralbumArt']/pre:img", doc);
if (imgList.size() > 0) {
@@ -118,12 +133,13 @@ public String getChildrenSaveTo(String saveTo) throws IOException {
}
@Override
- public boolean isSavingNotRequired(String saveTo) {
+ public synchronized
+ boolean isSavingNotRequired(String saveTo) {
Path p;
try {
- p = Paths.get(saveTo, getFSSafeName(getTitle()), "cover.jpg");
- if (Files.isRegularFile(p) && Files.size(p) > 0)
- return true;
+ p = Paths.get(getChildrenSaveTo(saveTo));
+ if (Files.isRegularFile(p) && Files.size(p) > 0)
+ return true;
} catch (IOException e) {
logger.log(Level.WARNING,null,e);
}
View
3 src/dloader/page/Discography.java
@@ -54,7 +54,8 @@ protected void parseSelf(Document doc) throws ProblemsReadingDocumentException
}
@Override
- public String saveResult(String saveTo) throws IOException {
+ public synchronized
+ String saveResult(String saveTo) throws IOException {
Path p = Paths.get(saveTo, getFSSafeName(getTitle()));
Files.createDirectories(p);
return null;
View
21 src/dloader/page/Track.java
@@ -35,8 +35,7 @@
* Set of custom properties read from page, saved to cache and
* resulting audio file metadata tags
*/
- Properties defaultProperties;
- Properties properties;
+ volatile Properties properties;
@Override
public String getTitle() {
@@ -81,16 +80,15 @@ public String setProperty(String name, String value) {
dataPatterns.put("comment", Pattern.compile(".*trackinfo:.*\"has_info\":\"([^\"]*)\".*", Pattern.DOTALL));
}
- public Track(String s) throws IllegalArgumentException {super(s);}
- public Track(URL url) throws IllegalArgumentException {super(url);}
-
{
- defaultProperties = new Properties();
- properties = new Properties(defaultProperties);
+ properties = new Properties();
}
+ public Track(String s) throws IllegalArgumentException {super(s);}
+ public Track(URL url) throws IllegalArgumentException {super(url);}
@Override
- public String saveResult(String saveTo) throws IOException {
+ public synchronized
+ String saveResult(String saveTo) throws IOException {
Files.createDirectories(Paths.get(saveTo));
Path p = Paths.get(saveTo, getFSSafeName(getTitle()) + ".mp3");
boolean wasDownloaded =
@@ -112,6 +110,7 @@ public String saveResult(String saveTo) throws IOException {
* @param fileTag - the tag to be examined and mimicked
* @return mapping {Track property name -> audio tag field id}
*/
+ private
Map<String,String> getTextFieldIds(Tag fileTag) {
Map<String,String> tagToCustomFrame = new HashMap<String,String>();
@@ -142,7 +141,7 @@ public String saveResult(String saveTo) throws IOException {
* @return true if actual write operation happened
* @throws IOException - file read/write problems
*/
- public boolean tagAudioFile(String file) throws IOException {
+ boolean tagAudioFile(String file) throws IOException {
try {
AudioFile theFile = AudioFileIO.read(Paths.get(file).toFile());
entagged.audioformats.Tag fileTag = theFile.getTag();
@@ -280,7 +279,9 @@ public String getChildrenSaveTo(String saveTo) {
@Override
public boolean isSavingNotRequired(String saveTo) {
- // this is commented out because even if file exists it might be needing some tagging
+ // this is commented out because even if file exists it might be
+ // needing some tagging
+ // TODO: implement proper check for tags present (honor the "ForceRetag" flag)
// try {
// Path p = Paths.get(saveTo, getFSSafeName(getTitle()) + ".mp3");
// if (Files.isRegularFile(p) && Files.size(p) > 0)
View
3 test/dloader/AllTests.java
@@ -3,10 +3,11 @@
import org.junit.runner.RunWith;
import org.junit.runners.Suite;
import org.junit.runners.Suite.SuiteClasses;
+import dloader.page.*;
@RunWith(Suite.class)
@SuiteClasses({ AbstractPageTest.class, PageProcessorTest.class,
- TrackTest.class, XMLCacheTest.class })
+ dloader.page.TrackTest.class, XMLCacheTest.class })
public class AllTests {
}
View
3 test/dloader/AbstractPageTest.java → test/dloader/page/AbstractPageTest.java
@@ -1,4 +1,4 @@
-package dloader;
+package dloader.page;
import static org.junit.Assert.*;
@@ -14,6 +14,7 @@
import dloader.page.AbstractPage;
import dloader.page.AbstractPage.ProblemsReadingDocumentException;
+import dloader.*;
public class AbstractPageTest {
View
3 test/dloader/TrackTest.java → test/dloader/page/TrackTest.java
@@ -1,4 +1,4 @@
-package dloader;
+package dloader.page;
import static org.junit.Assert.*;
@@ -14,6 +14,7 @@
import org.junit.Test;
import dloader.page.Track;
+import dloader.*;
import entagged.audioformats.AudioFile;
import entagged.audioformats.AudioFileIO;

0 comments on commit 417ebc8

Please sign in to comment.