From ad1aa0640718525ec8786d7b70c1c2603ce3f2a6 Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Fri, 30 Apr 2021 23:17:31 +0200 Subject: [PATCH] Cleanup: extracted SAX2 import from AbstractFileTrackImporter. --- .../io/file/importer/ExportImportTest.java | 10 ++-- .../io/file/importer/LegacyImportTest.java | 6 +- .../importer/AbstractFileTrackImporter.java | 36 +++--------- .../file/importer/GpxFileTrackImporter.java | 6 ++ .../io/file/importer/ImportService.java | 4 +- .../file/importer/KmlFileTrackImporter.java | 6 ++ .../io/file/importer/KmzTrackImporter.java | 5 +- .../io/file/importer/XMLImporter.java | 56 +++++++++++++++++++ 8 files changed, 88 insertions(+), 41 deletions(-) create mode 100644 src/main/java/de/dennisguse/opentracks/io/file/importer/XMLImporter.java diff --git a/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/ExportImportTest.java b/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/ExportImportTest.java index 3703b74d9..a6abcbe63 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/ExportImportTest.java +++ b/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/ExportImportTest.java @@ -154,7 +154,7 @@ public class ExportImportTest { // 2. import InputStream inputStream = new ByteArrayInputStream(outputStream.toByteArray()); - AbstractFileTrackImporter trackImporter = new KmlFileTrackImporter(context); + XMLImporter trackImporter = new XMLImporter(new KmlFileTrackImporter(context)); importTrackId = trackImporter.importFile(inputStream).get(0); // then @@ -195,7 +195,7 @@ public class ExportImportTest { // 2. import InputStream inputStream = new ByteArrayInputStream(outputStream.toByteArray()); - AbstractFileTrackImporter trackImporter = new KmlFileTrackImporter(context); + XMLImporter trackImporter = new XMLImporter(new KmlFileTrackImporter(context)); importTrackId = trackImporter.importFile(inputStream).get(0); // then @@ -237,7 +237,7 @@ public class ExportImportTest { // 2. import InputStream inputStream = new ByteArrayInputStream(outputStream.toByteArray()); - AbstractFileTrackImporter trackImporter = new KmlFileTrackImporter(context); + XMLImporter trackImporter = new XMLImporter(new KmlFileTrackImporter(context)); importTrackId = trackImporter.importFile(inputStream).get(0); // then @@ -263,7 +263,7 @@ public class ExportImportTest { // 2. import InputStream inputStream = new ByteArrayInputStream(outputStream.toByteArray()); - AbstractFileTrackImporter trackImporter = new GpxFileTrackImporter(context, contentProviderUtils); + XMLImporter trackImporter = new XMLImporter(new GpxFileTrackImporter(context, contentProviderUtils)); importTrackId = trackImporter.importFile(inputStream).get(0); // then @@ -313,7 +313,7 @@ public class ExportImportTest { // 2. import InputStream inputStream = new ByteArrayInputStream(outputStream.toByteArray()); - AbstractFileTrackImporter trackImporter = new GpxFileTrackImporter(context, contentProviderUtils); + XMLImporter trackImporter = new XMLImporter(new GpxFileTrackImporter(context, contentProviderUtils)); importTrackId = trackImporter.importFile(inputStream).get(0); // then diff --git a/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/LegacyImportTest.java b/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/LegacyImportTest.java index 95471be3e..572eee9c5 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/LegacyImportTest.java +++ b/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/LegacyImportTest.java @@ -50,7 +50,7 @@ public class LegacyImportTest { @Test public void kml_with_statistics_marker() { // given - KmlFileTrackImporter trackImporter = new KmlFileTrackImporter(context); + XMLImporter trackImporter = new XMLImporter(new KmlFileTrackImporter(context)); InputStream inputStream = InstrumentationRegistry.getInstrumentation().getContext().getResources().openRawResource(de.dennisguse.opentracks.debug.test.R.raw.legacy_kml_statistics_marker); // when @@ -92,7 +92,7 @@ public class LegacyImportTest { @Test(expected = ImportParserException.class) public void kml_without_locations() { // given - KmlFileTrackImporter trackImporter = new KmlFileTrackImporter(context); + XMLImporter trackImporter = new XMLImporter(new KmlFileTrackImporter(context)); InputStream inputStream = InstrumentationRegistry.getInstrumentation().getContext().getResources().openRawResource(de.dennisguse.opentracks.debug.test.R.raw.legacy_kml_empty); // when @@ -106,7 +106,7 @@ public class LegacyImportTest { @Test public void gpx_with_pause_resume() { // given - GpxFileTrackImporter trackImporter = new GpxFileTrackImporter(context); + XMLImporter trackImporter = new XMLImporter(new GpxFileTrackImporter(context)); InputStream inputStream = InstrumentationRegistry.getInstrumentation().getContext().getResources().openRawResource(de.dennisguse.opentracks.debug.test.R.raw.legacy_gpx_pause_resume); // when diff --git a/src/main/java/de/dennisguse/opentracks/io/file/importer/AbstractFileTrackImporter.java b/src/main/java/de/dennisguse/opentracks/io/file/importer/AbstractFileTrackImporter.java index b5854987d..440c6453c 100644 --- a/src/main/java/de/dennisguse/opentracks/io/file/importer/AbstractFileTrackImporter.java +++ b/src/main/java/de/dennisguse/opentracks/io/file/importer/AbstractFileTrackImporter.java @@ -18,7 +18,6 @@ package de.dennisguse.opentracks.io.file.importer; import android.content.Context; import android.content.SharedPreferences; -import android.database.sqlite.SQLiteConstraintException; import android.net.Uri; import android.util.Log; @@ -29,8 +28,6 @@ import org.xml.sax.SAXException; import org.xml.sax.helpers.DefaultHandler; import java.io.File; -import java.io.IOException; -import java.io.InputStream; import java.time.Duration; import java.time.Instant; import java.util.ArrayList; @@ -38,9 +35,6 @@ import java.util.List; import java.util.Locale; import java.util.UUID; -import javax.xml.parsers.ParserConfigurationException; -import javax.xml.parsers.SAXParserFactory; - import de.dennisguse.opentracks.R; import de.dennisguse.opentracks.content.data.Distance; import de.dennisguse.opentracks.content.data.Marker; @@ -62,7 +56,7 @@ import de.dennisguse.opentracks.util.TrackIconUtils; * * @author Jimmy Shih */ -abstract class AbstractFileTrackImporter extends DefaultHandler implements TrackImporter { +abstract class AbstractFileTrackImporter extends DefaultHandler implements XMLImporter.TrackParser { private static final String TAG = AbstractFileTrackImporter.class.getSimpleName(); @@ -127,24 +121,6 @@ abstract class AbstractFileTrackImporter extends DefaultHandler implements Track } } - @Override - @NonNull - public List importFile(InputStream inputStream) { - try { - SAXParserFactory.newInstance().newSAXParser().parse(inputStream, this); - return trackIds; - } catch (IOException | SAXException | ParserConfigurationException | ParsingException e) { - Log.e(TAG, "Unable to import file", e); - if (trackIds.size() > 0) { - cleanImport(); - } - throw new ImportParserException(e); - } catch (SQLiteConstraintException e) { - Log.e(TAG, "Unable to import file", e); - throw new ImportAlreadyExistsException(e); - } - } - protected void onFileEnd() { // Add markers to the last imported track int size = trackIds.size(); @@ -484,10 +460,12 @@ abstract class AbstractFileTrackImporter extends DefaultHandler implements Track } } - /** - * Cleans up import. - */ - private void cleanImport() { + @Override + public List getImportTrackIds() { + return trackIds; + } + + public void cleanImport() { contentProviderUtils.deleteTracks(context, trackIds); } diff --git a/src/main/java/de/dennisguse/opentracks/io/file/importer/GpxFileTrackImporter.java b/src/main/java/de/dennisguse/opentracks/io/file/importer/GpxFileTrackImporter.java index 1877384d3..22c042981 100644 --- a/src/main/java/de/dennisguse/opentracks/io/file/importer/GpxFileTrackImporter.java +++ b/src/main/java/de/dennisguse/opentracks/io/file/importer/GpxFileTrackImporter.java @@ -22,6 +22,7 @@ import androidx.annotation.VisibleForTesting; import org.xml.sax.Attributes; import org.xml.sax.SAXException; +import org.xml.sax.helpers.DefaultHandler; import java.util.Locale; @@ -83,6 +84,11 @@ public class GpxFileTrackImporter extends AbstractFileTrackImporter { super(context, contentProviderUtils); } + @Override + public DefaultHandler getHandler() { + return this; + } + @Override public void startElement(String uri, String localName, String tag, Attributes attributes) throws SAXException { switch (tag) { diff --git a/src/main/java/de/dennisguse/opentracks/io/file/importer/ImportService.java b/src/main/java/de/dennisguse/opentracks/io/file/importer/ImportService.java index 8bac87638..394cce2df 100644 --- a/src/main/java/de/dennisguse/opentracks/io/file/importer/ImportService.java +++ b/src/main/java/de/dennisguse/opentracks/io/file/importer/ImportService.java @@ -51,9 +51,9 @@ public class ImportService extends JobIntentService { String fileExtension = FileUtils.getExtension(file); if (TrackFileFormat.GPX.getExtension().equals(fileExtension)) { - trackImporter = new GpxFileTrackImporter(this); + trackImporter = new XMLImporter(new GpxFileTrackImporter(this)); } else if (TrackFileFormat.KML_WITH_TRACKDETAIL_AND_SENSORDATA.getExtension().equals(fileExtension)) { - trackImporter = new KmlFileTrackImporter(this); + trackImporter = new XMLImporter(new KmlFileTrackImporter(this)); } else if (TrackFileFormat.KMZ_WITH_TRACKDETAIL_AND_SENSORDATA_AND_PICTURES.getExtension().equals(fileExtension)) { trackImporter = new KmzTrackImporter(this, file.getUri()); } else { diff --git a/src/main/java/de/dennisguse/opentracks/io/file/importer/KmlFileTrackImporter.java b/src/main/java/de/dennisguse/opentracks/io/file/importer/KmlFileTrackImporter.java index 559d81396..527df20a6 100644 --- a/src/main/java/de/dennisguse/opentracks/io/file/importer/KmlFileTrackImporter.java +++ b/src/main/java/de/dennisguse/opentracks/io/file/importer/KmlFileTrackImporter.java @@ -23,6 +23,7 @@ import androidx.annotation.VisibleForTesting; import org.xml.sax.Attributes; import org.xml.sax.SAXException; +import org.xml.sax.helpers.DefaultHandler; import java.util.ArrayList; @@ -83,6 +84,11 @@ public class KmlFileTrackImporter extends AbstractFileTrackImporter { super(context, contentProviderUtils); } + @Override + public DefaultHandler getHandler() { + return this; + } + @Override public void startElement(String uri, String localName, String tag, Attributes attributes) throws SAXException { switch (tag) { diff --git a/src/main/java/de/dennisguse/opentracks/io/file/importer/KmzTrackImporter.java b/src/main/java/de/dennisguse/opentracks/io/file/importer/KmzTrackImporter.java index badca4dcd..f03f40544 100644 --- a/src/main/java/de/dennisguse/opentracks/io/file/importer/KmzTrackImporter.java +++ b/src/main/java/de/dennisguse/opentracks/io/file/importer/KmzTrackImporter.java @@ -71,7 +71,7 @@ public class KmzTrackImporter implements TrackImporter { List importFile(InputStream inputStream) { List trackIds = findAndParseKmlFile(inputStream); - ArrayList trackIdsWithImages = new ArrayList<>(); + List trackIdsWithImages = new ArrayList<>(); for (Track.Id trackId : trackIds) { if (copyKmzImages(trackId)) { @@ -228,7 +228,7 @@ public class KmzTrackImporter implements TrackImporter { } private List parseKml(ZipInputStream zipInputStream) { - KmlFileTrackImporter kmlFileTrackImporter = new KmlFileTrackImporter(context); + XMLImporter kmlFileTrackImporter = new XMLImporter(new KmlFileTrackImporter(context)); try (ByteArrayInputStream byteArrayInputStream = new ByteArrayInputStream(getKml(zipInputStream))) { return kmlFileTrackImporter.importFile(byteArrayInputStream); @@ -244,6 +244,7 @@ public class KmzTrackImporter implements TrackImporter { * * @param zipInputStream the zip input stream */ + //TODO We should be able to process the stream; we are wasting memory here. private byte[] getKml(ZipInputStream zipInputStream) throws IOException { try (ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream()) { byte[] buffer = new byte[BUFFER_SIZE]; diff --git a/src/main/java/de/dennisguse/opentracks/io/file/importer/XMLImporter.java b/src/main/java/de/dennisguse/opentracks/io/file/importer/XMLImporter.java new file mode 100644 index 000000000..f36b12fff --- /dev/null +++ b/src/main/java/de/dennisguse/opentracks/io/file/importer/XMLImporter.java @@ -0,0 +1,56 @@ +package de.dennisguse.opentracks.io.file.importer; + +import android.database.sqlite.SQLiteConstraintException; +import android.util.Log; + +import androidx.annotation.NonNull; + +import org.xml.sax.SAXException; +import org.xml.sax.helpers.DefaultHandler; + +import java.io.IOException; +import java.io.InputStream; +import java.util.List; + +import javax.xml.parsers.ParserConfigurationException; +import javax.xml.parsers.SAXParserFactory; + +import de.dennisguse.opentracks.content.data.Track; + +public class XMLImporter implements TrackImporter { + + private static final String TAG = XMLImporter.class.getSimpleName(); + + private final TrackParser parser; + + public XMLImporter(TrackParser parser) { + this.parser = parser; + } + + @Override + @NonNull + public List importFile(InputStream inputStream) throws ImportParserException, ImportAlreadyExistsException { + try { + SAXParserFactory.newInstance().newSAXParser().parse(inputStream, parser.getHandler()); + List trackIds = parser.getImportTrackIds(); + return trackIds; + } catch (IOException | SAXException | ParserConfigurationException | AbstractFileTrackImporter.ParsingException e) { + Log.e(TAG, "Unable to import file", e); + if (parser.getImportTrackIds().size() > 0) { + parser.cleanImport(); + } + throw new ImportParserException(e); + } catch (SQLiteConstraintException e) { + Log.e(TAG, "Unable to import file", e); + throw new ImportAlreadyExistsException(e); + } + } + + interface TrackParser { + DefaultHandler getHandler(); + + List getImportTrackIds(); + + void cleanImport(); + } +}