Modify

Opened 7 years ago

Last modified 3 years ago

#15182 new enhancement

Standalone JOSM validator

Reported by: Don-vip Owned by: team
Priority: major Milestone: Longterm
Component: Core validator Version:
Keywords: modularization Cc:

Description (last modified by Don-vip)

During Java 9 compatibility effort (#11924) I wondered if modularization could be applied to JOSM, which currently is a big spaghetti-based monolithic jar.

A clean modular architecture would be a huge task (don't know if it would really be possible at all, as we have a lot of interwoven dependencies) but I'd like to see if we can achieve a more affordable intermediate state, where we could build a standalone version of JOSM validator that could be run as follows:

java -jar josm-validator.jar data.osm -o warnings.xml

This jar having no dependency on AWT/Swing/JavaFX. This way it could be run for example by a Java 9 server VM which does not include the java.desktop module.

It would not change anything on JOSM editor except probably some API changes, but as little as possible.

#14704 is a prerequisite.

Attachments (5)

validator-standalone.patch (9.7 KB ) - added by Don-vip 7 years ago.
15182.patch (26.3 KB ) - added by taylor.smock 3 years ago.
Add validate command on JOSM command line. Output is line-by-line delimited geojson, for use in MapRoulette (AFAIK, MapRoulette is the only major "random problem" platform).
15182.2.patch (62.3 KB ) - added by taylor.smock 3 years ago.
Add tests
15182.3.patch (65.9 KB ) - added by taylor.smock 3 years ago.
Fix test issue (from test pollution), fix progress monitor issue
15182.sys_exit.patch (5.3 KB ) - added by taylor.smock 3 years ago.
Replace Sys.exit with Lifecycle.exitJosm, also change some instances of System.err.println to Logging.error.

Download all attachments as: .zip

Change History (148)

comment:1 by Don-vip, 7 years ago

In 12620/josm:

see #15182 - deprecate all Main logging methods and introduce suitable replacements in Logging for most of them

comment:2 by Don-vip, 7 years ago

In 12621/josm:

see #15182 - do not print stacktrace by default for debug/trace levels, as before

comment:3 by Don-vip, 7 years ago

In 12624/josm:

see #15182 - remove unused imports

comment:4 by Don-vip, 7 years ago

In 12627/josm:

see #15182 - remove unneeded imports to Main

comment:5 by Don-vip, 7 years ago

In 12628/josm:

see #15182 - build.xml - define constants for values repeated multiple times (java version, jar filename)

comment:6 by Don-vip, 7 years ago

Description: modified (diff)

comment:7 by Don-vip, 7 years ago

In 12629/josm:

see #15182 - move GUI initialization stuff from Main (abstract) to gui.MainApplication

comment:8 by stoecker, 7 years ago

Hmm, wouldn't it be more "satisfactory" to first make JOSM callable as standalone validator at all before you make it fully encapsulated? Even with many useless internal dependencies that would be a useful application.

comment:9 by Don-vip, 7 years ago

sure!

comment:10 by Don-vip, 7 years ago

#14704 is a prerequisite :)

comment:11 by Don-vip, 7 years ago

In 12630/josm:

see #15182 - deprecate Main.map and Main.isDisplayingMapView(). Replacements: gui.MainApplication.getMap() / gui.MainApplication.isDisplayingMapView()

comment:12 by Don-vip, 7 years ago

In 12631/josm:

see #15182 - remove last calls to Main.map

comment:13 by Don-vip, 7 years ago

In 12632/josm:

see #15182 - fix unit tests

comment:14 by Don-vip, 7 years ago

In 12633/josm:

see #15182 - move GUI program arguments management from Main to gui.MainApplication

comment:15 by Don-vip, 7 years ago

In 12634/josm:

see #15182 - deprecate Main.worker, replace it by gui.MainApplication.worker + code refactoring to make sure only editor packages use it

comment:16 by Don-vip, 7 years ago

In 12635/josm:

see #15182 - checkstyle

comment:17 by Don-vip, 7 years ago

In 12636/josm:

see #15182 - deprecate Main.getLayerManager(). Replacement: gui.MainApplication.getLayerManager()

comment:18 by Don-vip, 7 years ago

In 12637/josm:

see #15182 - deprecate Main.toolbar. Replacement: gui.MainApplication.getToolbar()

comment:19 by Don-vip, 7 years ago

In 12638/josm:

see #15182 - remove Main.currentProgressMonitor. Replacement: PleaseWaitProgressMonitor.getCurrent()

comment:20 by Don-vip, 7 years ago

In 12639/josm:

see #15182 - deprecate shortcut handling and mapframe listener methods in Main. Replacement: same methods in gui.MainApplication

comment:21 by Don-vip, 7 years ago

Description: modified (diff)

comment:22 by Don-vip, 7 years ago

In 12641/josm:

see #15182 - deprecate Main.main.undoRedo. Replacement: gui.MainApplication.undoRedo

comment:23 by Don-vip, 7 years ago

In 12642/josm:

see #15182 - deprecate Main.main.panel. Replacement: gui.MainApplication.getMainPanel()

comment:24 by Don-vip, 7 years ago

In 12643/josm:

see #15182 - deprecate Main.main.menu. Replacement: gui.MainApplication.getMenu()

comment:25 by stoecker, 7 years ago

Hmm, I don't think undoRedo is really a GUI element. I can think of possible projects using it without a gui, as it is the central comnand handling instance.

comment:26 by Don-vip, 7 years ago

I can make it a singleton so it can be called outside of MainApplication.

comment:27 by Don-vip, 7 years ago

In 12644/josm:

see #15182 - remove dependence of I18n on GUI

comment:28 by Don-vip, 7 years ago

In 12645/josm:

see #15182 - remove last non-deprecated dependence of Main on GUI

comment:29 by Don-vip, 7 years ago

In 12647/josm:

see #15182 - remove unused method

comment:30 by Don-vip, 7 years ago

In 12648/josm:

see #15182 - refactor build.xml

comment:31 by Don-vip, 7 years ago

In 12649/josm:

see #15182 - code refactoring to avoid dependence on GUI packages from Preferences

comment:32 by Don-vip, 7 years ago

In 12650/josm:

see #15182 - convert MapCSS performance test to Java + update import

comment:33 by Don-vip, 7 years ago

In 12651/josm:

see #15182 - code refactoring to avoid dependence on GUI packages from MapPaintStyles

comment:34 by Don-vip, 7 years ago

In 12656/josm:

see #15182 - move SearchCompiler from actions.search to data.osm.search

comment:35 by Don-vip, 7 years ago

In 12659/josm:

see #15182 - extract SearchMode and SearchSetting from actions.search.SearchAction to data.osm.search

comment:36 by Don-vip, 7 years ago

In 12662/josm:

see #15182 - remove dependence on GUI from data.osm.search.SearchCompiler

comment:37 by Don-vip, 7 years ago

In 12663/josm:

see #15182 - move NameFormatter* from gui to data.osm

comment:38 by Don-vip, 7 years ago

In 12664/josm:

see #15182 - fix javadoc warning

comment:39 by Don-vip, 7 years ago

In 12665/josm:

see #15182 - fix #15186 - fix #15192 - fix initialization of Main.worker (regression)

comment:40 by Don-vip, 7 years ago

In 12666/josm:

see #15182 - fix #15193 - fix SearchCompiler

comment:41 by Don-vip, 7 years ago

In 12669/josm:

see #15182 - remove dependence on JMapViewer for package data.coor (only useful for imagery)

comment:42 by Don-vip, 7 years ago

In 12670/josm:

see #15182 - remove dependence on GUI for tools.PlatformHook

comment:43 by Don-vip, 7 years ago

In 12671/josm:

see #15182 - move file importers/exporters from io package to gui.io.importexport package, as they rely heavily on GUI and are mainly used from Open/Save actions

comment:44 by Don-vip, 7 years ago

In 12672/josm:

see #15182 - move ConflictCollection from OsmDataLayer to DataSet. Simplifies some code where a data set is enough, and a layer is not needed

comment:45 by Don-vip, 7 years ago

In 12673/josm:

see #15182 - move CyclicUploadDependencyException from actions.upload to data.osm (used in data.APIDataSet)

comment:46 by Don-vip, 7 years ago

In 12674/josm:

see #15182 - move PROP_SYSTEM_OF_MEASUREMENT from gui.preferences.ProjectionPreferences to data.SystemOfMeasurment

comment:47 by Don-vip, 7 years ago

In 12675/josm:

see #15182 - move the Swing-based ProgressMonitor implementations from gui.progress to gui.progress.swing. Progress monitor concept is used in very large parts of JOSM, a console-based implementation could be added later

comment:48 by Don-vip, 7 years ago

In 12676/josm:

see #15182 - PMD

comment:49 by bastiK, 7 years ago

Exciting to see all those changes towards a more modular structure! How are we doing with respect to the upcoming release, is everything considered stable?

comment:50 by Don-vip, 7 years ago

I think so. It requires some plugins to be updated (I'm on it) but it's often just moving classes around and updating the imports. I have finished with deprecating stuff in Main, I have now a lot of code that compiles without the GUI :)

comment:51 by Don-vip, 7 years ago

In 12678/josm:

see #15182 - move WindowGeometry from tools to gui.util

comment:52 by Don-vip, 7 years ago

In 12679/josm:

see #15182 - make actions.downloadtasks.Download*Task depend on io.OsmServerLocationReader, not the opposite

comment:53 by Don-vip, 7 years ago

In 12681/josm:

see #15182 - removed useless import

comment:54 by Don-vip, 7 years ago

In 12682/josm:

see #15182 - move GuiSizesHelper from gui.util to tools (only called from there)

comment:55 by Don-vip, 7 years ago

In 12683/josm:

see #15182 - transfer GUI dependencies from tools.TextTagParser to gui.datatransfer.importers.TextTagPaster

comment:56 by Don-vip, 7 years ago

In 12686/josm:

see #15182 - move OAuthAccessTokenHolder from gui.preferences.server to data.oauth

comment:57 by Don-vip, 7 years ago

In 12687/josm:

see #15182 - move UploadStrategySpecification and required enums from gui.io to io

comment:58 by Don-vip, 7 years ago

In 12688/josm:

see #15182 - refactor PurgeAction/PurgeCommand to avoid unneeded dependence on action

comment:59 by Don-vip, 7 years ago

In 12689/josm:

see #15182 - refactor MergeNodesAction to avoid unneeded GUI dependence in DuplicateNode test

comment:60 by Don-vip, 7 years ago

In 12690/josm:

see #15182 - build.xml fixes, required for standalone validator jar (problem unseen in complete compilation)

comment:61 by Don-vip, 7 years ago

In 12691/josm:

see #15182 - introduce Main.getEditDataSet to avoid unneeded GUI dependence in validator tests and tagging presets

comment:62 by Don-vip, 7 years ago

In 12695/josm:

see #15182 - refactor PlatformHookOsx so that all GUI actions are performed in MainApplication and only macOS-specific stuff remains in platform hook

by Don-vip, 7 years ago

Attachment: validator-standalone.patch added

comment:63 by Don-vip, 7 years ago

With current build.xml + this task:

    <!--
      ** build standalone validator jar.
    -->
    <target name="standalone-validator-jar" depends="clean,javacc,compile-cots,create-revision,check-schemas">
        <delete dir="validator-src"/>
        <delete dir="validator-build"/>
        <mkdir dir="validator-src"/>
        <copy todir="validator-src" includeemptydirs="false">
            <fileset dir="${src.dir}">
                <include name="org/openstreetmap/josm/*.java"/>
                <include name="org/openstreetmap/josm/command/*.java"/>
                <include name="org/openstreetmap/josm/data/**"/>
                <exclude name="org/openstreetmap/josm/data/imagery/**"/>
                <include name="org/openstreetmap/josm/gui/mappaint/**"/>
                <include name="org/openstreetmap/josm/gui/progress/**"/>
                <exclude name="org/openstreetmap/josm/gui/progress/swing/**"/>
                <include name="org/openstreetmap/josm/gui/tagging/presets/**"/>
                <include name="org/openstreetmap/josm/io/**"/>
                <exclude name="org/openstreetmap/josm/io/imagery/**"/>
                <exclude name="org/openstreetmap/josm/io/remotecontrol/**"/>
                <exclude name="org/openstreetmap/josm/io/session/**"/>
                <include name="org/openstreetmap/josm/tools/**"/>
            </fileset>
        </copy>
        <patch patchfile="patches/validator-standalone.patch" strip="1" dir="validator-src" failonerror="true"/>
        <mkdir dir="validator-build"/>
        <javac compiler="${javac.compiler}" sourcepath="" srcdir="validator-src" classpath="${build.dir}"
            destdir="validator-build" target="${java.lang.version}" source="${java.lang.version}" debug="on" includeantruntime="false" createMissingPackageInfoClass="false" encoding="UTF-8">
            <compilerclasspath>
                <pathelement location="${error_prone_ant.jar}"/>
            </compilerclasspath>
            <compilerarg value="-Xlint:cast"/>
            <compilerarg value="-Xlint:deprecation"/>
            <compilerarg value="-Xlint:dep-ann"/>
            <compilerarg value="-Xlint:divzero"/>
            <compilerarg value="-Xlint:empty"/>
            <compilerarg value="-Xlint:finally"/>
            <compilerarg value="-Xlint:overrides"/>
            <!--<compilerarg value="-Xlint:rawtypes"/>-->
            <compilerarg value="-Xlint:static"/>
            <compilerarg value="-Xlint:try"/>
            <compilerarg value="-Xlint:unchecked"/>
            <!-- Undocumented argument to ignore "Sun internal proprietary API" warning, see http://stackoverflow.com/a/13862308/2257172 -->
            <compilerarg value="-XDignore.symbol.file"/>
            <compilerarg value="-Xep:ReferenceEquality:OFF" compiler="com.google.errorprone.ErrorProneAntCompilerAdapter"/>
            <compilerarg value="-Xep:ImmutableEnumChecker:OFF" compiler="com.google.errorprone.ErrorProneAntCompilerAdapter"/>
            <compilerarg value="-Xep:FutureReturnValueIgnored:OFF" compiler="com.google.errorprone.ErrorProneAntCompilerAdapter"/>
            <compilerarg value="-Xep:FloatingPointLiteralPrecision:OFF" compiler="com.google.errorprone.ErrorProneAntCompilerAdapter"/>
            <compilerarg value="-Xep:ShortCircuitBoolean:OFF" compiler="com.google.errorprone.ErrorProneAntCompilerAdapter"/>
            <compilerarg value="-Xep:LiteralClassName:OFF" compiler="com.google.errorprone.ErrorProneAntCompilerAdapter"/>
            <compilerarg line="-Xmaxerrs 1000"/>
        </javac>
        <copy todir="validator-build" failonerror="no" includeemptydirs="no">
            <fileset dir="resources"/>
        </copy>
    </target>

r12545 (current tested): 626 errors
r12695 (current trunk): 344 errors

The three main problems ahead are:

  • code relying on layers
  • mappaint code. we need the MapCSS code for MapCSS validator test. But we don't need all the renderers, etc.
  • tagging presets, very GUI-oriented right now. We need them also for validator tests, but not the GUI components

From all errors here are the packages missing the most, by order of occurrence:

      1  package org.openstreetmap.josm.actions.SplitWayAction does not exist
      1  package org.openstreetmap.josm.gui.dialogs.relation does not exist
      1  package org.openstreetmap.josm.gui.draw.MapViewPath does not exist
      1  package org.openstreetmap.josm.gui.help does not exist
      1  package org.openstreetmap.josm.gui.layer.markerlayer does not exist
      1  package org.openstreetmap.josm.gui.layer.OsmDataLayer does not exist
      1  package org.openstreetmap.josm.gui.oauth does not exist
      1  package org.openstreetmap.josm.gui.preferences.display does not exist
      1  package org.openstreetmap.josm.gui.preferences does not exist
      1  package org.openstreetmap.josm.gui.preferences.server.ProxyPreferencesPanel does not exist
      2  package org.openstreetmap.gui.jmapviewer does not exist
      2  package org.openstreetmap.josm.actions.JoinAreasAction does not exist
      2  package org.openstreetmap.josm.gui.bugreport does not exist
      2  package org.openstreetmap.josm.gui.dialogs does not exist
      2  package org.openstreetmap.josm.gui.dialogs.relation.sort does not exist
      2  package org.openstreetmap.josm.gui.io does not exist
      2  package org.openstreetmap.josm.plugins does not exist
      4  package org.openstreetmap.josm.gui.preferences.projection does not exist
      4  package org.openstreetmap.josm.gui.preferences.server does not exist
      6  package org.openstreetmap.josm.gui.layer.MainLayerManager does not exist
      8  package org.openstreetmap.josm.actions does not exist
      8  package org.openstreetmap.josm.gui.MapViewState does not exist
      8  package org.openstreetmap.josm.gui.tagging.ac does not exist
     12  package org.openstreetmap.josm.gui.layer.LayerManager does not exist
     12  package org.openstreetmap.josm.gui.util does not exist
     15  package org.openstreetmap.josm.gui.widgets does not exist
     16  package org.openstreetmap.josm.gui.draw does not exist
     22  package org.openstreetmap.josm.gui.layer does not exist

and here are the files with the most unresolved imports:

      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\command\AddCommand.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\command\AddPrimitivesCommand.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\command\ChangeCommand.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\command\PurgeCommand.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\data\cache\JCSCachedTileLoaderJob.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\data\cache\JCSCacheManager.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\data\osm\ChangesetCache.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\data\osm\DataSet.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\data\osm\visitor\paint\MapRendererFactory.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\data\osm\visitor\paint\OffsetIterator.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\data\PreferencesUtils.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\data\validation\tests\DuplicateNode.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\data\validation\tests\TagChecker.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\gui\mappaint\ElemStyles.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\gui\mappaint\mapcss\ExpressionFactory.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\gui\mappaint\styleelement\AreaIconElement.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\gui\mappaint\styleelement\MapImage.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\gui\mappaint\styleelement\Symbol.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\gui\tagging\presets\items\Link.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\gui\tagging\presets\TaggingPresetSearchAction.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\gui\tagging\presets\TaggingPresetSearchPrimitiveDialog.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\io\MultiFetchOverpassObjectReader.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\io\MultiFetchServerObjectReader.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\io\OsmConnection.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\io\OsmWriter.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\tools\bugreport\BugReport.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\tools\bugreport\BugReportQueue.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\tools\Geometry.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\tools\ImageOverlay.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\tools\ImageProvider.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\tools\OsmUrlToBounds.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\tools\PlatformHookWindows.java
      1 \SVN\josm\core\validator-src\org\openstreetmap\josm\tools\Shortcut.java
      2 \SVN\josm\core\validator-src\org\openstreetmap\josm\data\osm\event\DatasetEventManager.java
      2 \SVN\josm\core\validator-src\org\openstreetmap\josm\data\osm\FilterModel.java
      2 \SVN\josm\core\validator-src\org\openstreetmap\josm\data\osm\visitor\paint\AbstractMapRenderer.java
      2 \SVN\josm\core\validator-src\org\openstreetmap\josm\data\osm\visitor\paint\ArrowPaintHelper.java
      2 \SVN\josm\core\validator-src\org\openstreetmap\josm\data\projection\Projections.java
      2 \SVN\josm\core\validator-src\org\openstreetmap\josm\data\validation\OsmValidator.java
      2 \SVN\josm\core\validator-src\org\openstreetmap\josm\data\validation\PaintVisitor.java
      2 \SVN\josm\core\validator-src\org\openstreetmap\josm\data\validation\tests\PublicTransportRouteTest.java
      2 \SVN\josm\core\validator-src\org\openstreetmap\josm\gui\mappaint\styleelement\NodeElement.java
      2 \SVN\josm\core\validator-src\org\openstreetmap\josm\gui\mappaint\styleelement\placement\CompletelyInsideAreaStrategy.java
      2 \SVN\josm\core\validator-src\org\openstreetmap\josm\gui\mappaint\styleelement\placement\PartiallyInsideAreaStrategy.java
      2 \SVN\josm\core\validator-src\org\openstreetmap\josm\gui\mappaint\styleelement\placement\PositionForAreaStrategy.java
      2 \SVN\josm\core\validator-src\org\openstreetmap\josm\gui\tagging\presets\items\Check.java
      2 \SVN\josm\core\validator-src\org\openstreetmap\josm\gui\tagging\presets\TaggingPresetSelector.java
      2 \SVN\josm\core\validator-src\org\openstreetmap\josm\io\auth\AbstractCredentialsAgent.java
      2 \SVN\josm\core\validator-src\org\openstreetmap\josm\io\auth\JosmPreferencesCredentialAgent.java
      2 \SVN\josm\core\validator-src\org\openstreetmap\josm\io\DefaultProxySelector.java
      2 \SVN\josm\core\validator-src\org\openstreetmap\josm\io\GeoJSONWriter.java
      2 \SVN\josm\core\validator-src\org\openstreetmap\josm\io\MessageNotifier.java
      2 \SVN\josm\core\validator-src\org\openstreetmap\josm\io\OsmApi.java
      3 \SVN\josm\core\validator-src\org\openstreetmap\josm\command\Command.java
      3 \SVN\josm\core\validator-src\org\openstreetmap\josm\data\osm\event\SelectionEventManager.java
      3 \SVN\josm\core\validator-src\org\openstreetmap\josm\data\osm\visitor\paint\StyledMapRenderer.java
      3 \SVN\josm\core\validator-src\org\openstreetmap\josm\data\osm\visitor\paint\WireframeMapRenderer.java
      3 \SVN\josm\core\validator-src\org\openstreetmap\josm\gui\tagging\presets\TaggingPresetItem.java
      3 \SVN\josm\core\validator-src\org\openstreetmap\josm\tools\bugreport\BugReportSender.java
      3 \SVN\josm\core\validator-src\org\openstreetmap\josm\tools\RightAndLefthandTraffic.java
      4 \SVN\josm\core\validator-src\org\openstreetmap\josm\data\osm\history\HistoryDataSet.java
      4 \SVN\josm\core\validator-src\org\openstreetmap\josm\gui\mappaint\styleelement\placement\OnLineStrategy.java
      4 \SVN\josm\core\validator-src\org\openstreetmap\josm\gui\tagging\presets\items\Combo.java
      4 \SVN\josm\core\validator-src\org\openstreetmap\josm\gui\tagging\presets\items\Text.java
      5 \SVN\josm\core\validator-src\org\openstreetmap\josm\command\DeleteCommand.java
      5 \SVN\josm\core\validator-src\org\openstreetmap\josm\data\osm\visitor\paint\relations\MultipolygonCache.java
      6 \SVN\josm\core\validator-src\org\openstreetmap\josm\gui\tagging\presets\TaggingPreset.java
      7 \SVN\josm\core\validator-src\org\openstreetmap\josm\data\UndoRedoHandler.java
      7 \SVN\josm\core\validator-src\org\openstreetmap\josm\gui\mappaint\MapPaintMenu.java

comment:64 by Don-vip, 7 years ago

In 12696/josm:

see #15182 - remove deprecation of Main.undoRedo until I figure how to deal with this class

in reply to:  description comment:65 by bastiK, 7 years ago

Replying to Don-vip:

During Java 9 compatibility effort (#11924) I wondered if modularization could be applied to JOSM, which currently is a big spaghetti-based monolithic jar.

A clean modular architecture would be a huge task (don't know if it would really be possible at all, as we have a lot of interwoven dependencies) but I'd like to see if we can achieve a more affordable intermediate state, where we could build a standalone version of JOSM validator that could be run as follows:

java -jar josm-validator.jar data.osm -o warnings.xml

This jar having no dependency on AWT/Swing/JavaFX. This way it could be run for example by a Java 9 server VM which does not include the java.desktop module.

I have similar long-term ideas, namely a stanalone MapCSS renderer and a projection library. Both should run in headless mode as well and come with command line arguments for basic use. After you've done the groundwork, it should be a piece of cake! ;)

comment:66 by Don-vip, 7 years ago

Well maybe I will need your help for the MapCSS part :)

comment:67 by michael2402, 7 years ago

When doing this, wouldn't it be wise to externalize that code completely?

It will make it much easier to re-use and normal contributors will be less likely to break things since the "normal" build will fail as well if the dependencies are not met.

I'd suggest the following for now:

  • josm
    • DataSet (+ all data set related classes)
    • MapCSS
      • DataSet
      • MapPainting (Projection, MapView point conversion, MapViewPaintable)
    • Imagery
      • MapPainting
    • Commands (that modify the data set)
      • DataSet

Everything depends on data set for now.

For the preferences, I'd suggest to use a separate module for the storage classes. We can then use those preferences in the MapCSS/MapPainting/... modules and keep the user interface code for them in josm main for now.

comment:68 by Don-vip, 7 years ago

Milestone: 17.09

comment:69 by Don-vip, 7 years ago

This doesn't work. Validator depends on MapCSS, but I don't want it to depend on MapView, which extends JComponent and imports a lot of awt/swing classes. I haven't looked exactly yet how to split the MapCSS parsing from the MapView/paint code.

The first step is to cut Swing out of our core data classes. That's I am trying to achieve. We'll see next how/if we can modularize further.

comment:70 by bastiK, 7 years ago

We can start with very informal modules, i.e. move some of the .java files into different directories. In terms of the build process, there is almost no change at all, we simply tell javac to find the source files in multiple directories.

Then, gradually, the dependency structure can be carved out more clearly, eventually to be turned into proper Java 9 modules.

Here is a possible directory structure:

modules
    |- org.openstreetmap.josm.app
        |- src
        |- build.xml
    |- org.openstreetmap.josm.validator
        |- src
            |- main
                |- java
                    |- module-info.java
                    |- org/openstreetmap/josm/data/validation/*.java
                |- resources
                    |- ignoretags.cfg
                    |- deprecated.mapcss
                     ...
            |- test
                |- java
                |- resources
        |- build.xml
    |- org.openstreetmap.josm.mapcss
    |- org.openstreetmap.josm.projection
    |- org.openstreetmap.jmapviewer
     ...
build.xml
README

The directory names (org.openstreetmap.josm.validator, ...) are the names of later the Java 9 modules as per the module naming convention.

Switching to the standard Maven directory layout for each module (like in the draft) would be neither a big advantage nor disadvantage. On the plus side, it may be easier for new developers who are familiar with the Maven layout.

One question that remains, is if all this is worthwhile and achievable or a hopeless endeavor. ;)

comment:71 by Don-vip, 7 years ago

That's about what I had in mind for the long term :)
It is worthwile. Once we have clean modules we can publish them to Maven Central. I'd like to see JOSM core classes become the standard OSM Java library :) There would be no need for other people to create their own (osm-common or osmapi for example)

comment:72 by Don-vip, 7 years ago

In 12718/josm:

see #13036 - see #15229 - see #15182 - make Commands depends only on a DataSet, not a Layer. This removes a lot of GUI dependencies

comment:73 by Don-vip, 7 years ago

In 12734/josm:

see #15182 - see #15229 - remove GUI dependence in NoteData

comment:74 by Don-vip, 7 years ago

In 12742/josm:

see #15229 - see #15182 - remove useless dependence of Geometry on GUI now that MultipolygonCache does not need anymore a MapView

comment:75 by Don-vip, 7 years ago

In 12743/josm:

see #15229 - see #15182 - deprecate gui.JosmUserIdentityManager - replaced by data.UserIdentityManager

comment:76 by Don-vip, 7 years ago

In 12744/josm:

see #15229 - see #15182 - remove GUI dependence of PlatformHookWindows - move workaround to MainApplication

comment:77 by Don-vip, 7 years ago

In 12747/josm:

see #15229 - see #15182 - fix unit tests

comment:78 by Don-vip, 7 years ago

In 12748/josm:

see #15229 - see #15182 - deprecate GuiHelper.getMenuShortcutKeyMaskEx() - replaced by PlatformHook.getMenuShortcutKeyMaskEx()

comment:79 by Don-vip, 7 years ago

In 12749/josm:

see #15229 - see #15182 - see #13036 - remove GUI stuff from Command and DeleteCommand

comment:80 by Don-vip, 7 years ago

In 12750/josm:

see #15229 - see #15182 - fix unit tests

comment:81 by Don-vip, 7 years ago

r12750 is down to 282 errors. List of problematic files:

org/openstreetmap/josm/command/DeleteCommand.java
org/openstreetmap/josm/data/cache/JCSCachedTileLoaderJob.java
org/openstreetmap/josm/data/cache/JCSCacheManager.java
org/openstreetmap/josm/data/osm/ChangesetCache.java
org/openstreetmap/josm/data/osm/DataSet.java
org/openstreetmap/josm/data/osm/event/DatasetEventManager.java
org/openstreetmap/josm/data/osm/event/SelectionEventManager.java
org/openstreetmap/josm/data/osm/FilterModel.java
org/openstreetmap/josm/data/osm/history/HistoryDataSet.java
org/openstreetmap/josm/data/osm/visitor/paint/AbstractMapRenderer.java
org/openstreetmap/josm/data/osm/visitor/paint/ArrowPaintHelper.java
org/openstreetmap/josm/data/osm/visitor/paint/MapRendererFactory.java
org/openstreetmap/josm/data/osm/visitor/paint/OffsetIterator.java
org/openstreetmap/josm/data/osm/visitor/paint/relations/MultipolygonCache.java
org/openstreetmap/josm/data/osm/visitor/paint/StyledMapRenderer.java
org/openstreetmap/josm/data/osm/visitor/paint/WireframeMapRenderer.java
org/openstreetmap/josm/data/PreferencesUtils.java
org/openstreetmap/josm/data/projection/Projections.java
org/openstreetmap/josm/data/validation/OsmValidator.java
org/openstreetmap/josm/data/validation/PaintVisitor.java
org/openstreetmap/josm/data/validation/tests/DuplicateNode.java
org/openstreetmap/josm/data/validation/tests/TagChecker.java
org/openstreetmap/josm/gui/mappaint/ElemStyles.java
org/openstreetmap/josm/gui/mappaint/mapcss/ExpressionFactory.java
org/openstreetmap/josm/gui/mappaint/MapPaintMenu.java
org/openstreetmap/josm/gui/mappaint/styleelement/AreaIconElement.java
org/openstreetmap/josm/gui/mappaint/styleelement/MapImage.java
org/openstreetmap/josm/gui/mappaint/styleelement/NodeElement.java
org/openstreetmap/josm/gui/mappaint/styleelement/placement/CompletelyInsideAreaStrategy.java
org/openstreetmap/josm/gui/mappaint/styleelement/placement/OnLineStrategy.java
org/openstreetmap/josm/gui/mappaint/styleelement/placement/PartiallyInsideAreaStrategy.java
org/openstreetmap/josm/gui/mappaint/styleelement/placement/PositionForAreaStrategy.java
org/openstreetmap/josm/gui/mappaint/styleelement/Symbol.java
org/openstreetmap/josm/gui/tagging/presets/items/Check.java
org/openstreetmap/josm/gui/tagging/presets/items/Combo.java
org/openstreetmap/josm/gui/tagging/presets/items/Link.java
org/openstreetmap/josm/gui/tagging/presets/items/Text.java
org/openstreetmap/josm/gui/tagging/presets/TaggingPresetItem.java
org/openstreetmap/josm/gui/tagging/presets/TaggingPreset.java
org/openstreetmap/josm/gui/tagging/presets/TaggingPresetSearchAction.java
org/openstreetmap/josm/gui/tagging/presets/TaggingPresetSearchPrimitiveDialog.java
org/openstreetmap/josm/gui/tagging/presets/TaggingPresetSelector.java
org/openstreetmap/josm/io/auth/AbstractCredentialsAgent.java
org/openstreetmap/josm/io/auth/JosmPreferencesCredentialAgent.java
org/openstreetmap/josm/io/DefaultProxySelector.java
org/openstreetmap/josm/io/GeoJSONWriter.java
org/openstreetmap/josm/io/MessageNotifier.java
org/openstreetmap/josm/io/MultiFetchOverpassObjectReader.java
org/openstreetmap/josm/io/MultiFetchServerObjectReader.java
org/openstreetmap/josm/io/OsmApi.java
org/openstreetmap/josm/io/OsmConnection.java
org/openstreetmap/josm/io/OsmWriter.java
org/openstreetmap/josm/tools/bugreport/BugReport.java
org/openstreetmap/josm/tools/bugreport/BugReportQueue.java
org/openstreetmap/josm/tools/bugreport/BugReportSender.java
org/openstreetmap/josm/tools/ImageOverlay.java
org/openstreetmap/josm/tools/ImageProvider.java
org/openstreetmap/josm/tools/OsmUrlToBounds.java
org/openstreetmap/josm/tools/RightAndLefthandTraffic.java

comment:82 by Don-vip, 7 years ago

In 12751/josm:

see #15229 - see #15182 - see #13036 - update unit tests and groovy script

comment:83 by Don-vip, 7 years ago

In 12752/josm:

see #15229 - see #15182 - see #13036 - fix groovy script

comment:84 by Don-vip, 7 years ago

In 12754/josm:

see #15229 - see #15182 - see #13036 - update unit tests

comment:85 by Don-vip, 7 years ago

In 12755/josm:

see #15229 - see #15182 - make Main to not depend on JCS

comment:86 by Don-vip, 7 years ago

In 12756/josm:

see #15229 - see #15182 - move RotationAngle from gui.util to tools

comment:87 by Don-vip, 7 years ago

In 12757/josm:

see #15229 - see #15182 - see #13036 - update fixture for performance tests

comment:88 by Don-vip, 7 years ago

In 12758/josm:

see #15229 - see #15182 - deprecate DataSet.getAutoCompletionManager() - replacement: AutoCompletionManager.of(DataSet)

comment:89 by Don-vip, 7 years ago

In 12760/josm:

see #15229 - see #15182 - see #13036 - remove more GUI stuff from DeleteCommand

comment:90 by Don-vip, 7 years ago

In 12761/josm:

see #15229 - see #15182 - see #13036 - fix stupid mistake

comment:91 by Don-vip, 7 years ago

In 12763/josm:

see #15229 - see #15182 - see #13036 - remove last GUI stuff from DeleteCommand

comment:92 by Don-vip, 7 years ago

In 12764/josm:

see #15229 - see #15182 - fix unit test

comment:93 by Don-vip, 7 years ago

In 12766/josm:

see #15229 - see #15182 - remove GUI stuff from MessageNotifier

comment:94 by Don-vip, 7 years ago

In 12767/josm:

see #15229 - see #15182 - do not longer fire ChangesetCacheEvent in EDT - now up to listeners to run their code in it if required

comment:95 by Don-vip, 7 years ago

In 12782/josm:

see #15229 - see #15182 - move ImageProcessor from gui.layer to tools

comment:96 by bastiK, 7 years ago

In 12786/josm:

see #15182 - remove GUI references from Projections class

comment:97 by Don-vip, 7 years ago

In 12790/josm:

see #15229 - see #15182 - remove GUI references from BugReportSender

comment:98 by Don-vip, 7 years ago

In 12791/josm:

see #15229 - see #15182 - javadoc fixes

comment:99 by bastiK, 7 years ago

In 12792/josm:

closes #15273, see #15229, see #15182 - add command line interface module for projections

  • run josm project --help to see the options
  • extracts parser from LatLon and CustomProjection into LatLonParser

comment:100 by bastiK, 7 years ago

In 12793/josm:

see #15273, see #15229, see #15182 - add missing class

comment:101 by Don-vip, 7 years ago

In 12796/josm:

see #15229 - see #15182 - remove GUI references from OsmUrlToBounds

comment:102 by Don-vip, 7 years ago

In 12799/josm:

see #15229 - see #15182 - see #14794 - move Multi* GUI classes from tools to gui.util

comment:103 by Don-vip, 7 years ago

In 12800/josm:

see #15229 - see #15182 - deprecate OsmWriter.writeLayer(OsmDataLayer) - replacement: OsmWriter.write(DataSet)

comment:104 by Don-vip, 7 years ago

In 12803/josm:

see #15229 - see #15182 - remove GUI references from OsmConnection

comment:105 by Don-vip, 7 years ago

In 12804/josm:

see #15229 - see #15182 - remove GUI references from OsmApi

comment:106 by Don-vip, 7 years ago

In 12805/josm:

see #15229 - see #15182 - remove GUI references from DefaultProxySelector

comment:107 by Don-vip, 7 years ago

In 12806/josm:

see #15229 - see #15182 - remove GUI references from GeoJSONWriter

comment:108 by Don-vip, 7 years ago

In 12807/josm:

see #15229 - see #15182 - see #14794 - checkstyle, unit tests

comment:109 by Don-vip, 7 years ago

In 12816/josm:

see #15229 - see #15182 - remove GUI references from I/O subsystem

comment:110 by Don-vip, 7 years ago

In 12821/josm:

see #15229 - see #15182 - remove GUI references from I/O subsystem

comment:111 by Don-vip, 7 years ago

In 12823/josm:

see #15229 - see #15182 - move PaintVisitor from data.validation to gui.layer.validation

comment:112 by Don-vip, 7 years ago

In 12825/josm:

see #15229 - see #15182 - make FileWatcher generic so it has no more dependence on MapCSS

comment:113 by Don-vip, 7 years ago

In 12826/josm:

see #15229 - see #15182 - remove GUI references from PreferencesUtils

comment:114 by Don-vip, 7 years ago

In 12828/josm:

see #15229 - see #15182 - see #13036 - convert SplitWayAction to SplitWayCommand to remove dependence of DeleteCommand on actions package

comment:115 by Don-vip, 7 years ago

In 12834/josm:

see #15229 - see #15182 - move TaggingPresetSearchAction to actions package

comment:116 by Don-vip, 7 years ago

In 12835/josm:

see #15229 - see #15182 - remove GUI references from TaggingPresetItem

comment:117 by Don-vip, 7 years ago

In 12859/josm:

see #15229 - see #15182 - deprecate non-GUI AutoCompletion* classes from gui.tagging.ac. Offer better replacements in new data.tagging.ac package

comment:118 by Don-vip, 7 years ago

In 12869/josm:

see #15229 - see #15182 - SonarQube - squid:S2444 - make static fields volatile

comment:119 by Don-vip, 7 years ago

In 12870/josm:

see #15229 - see #15182 - remove dependence of ImageProvider on PluginHandler

comment:120 by Don-vip, 7 years ago

Milestone: 17.0917.10

comment:121 by simon04, 7 years ago

In 12886/josm:

see #15229 - see #15182 - fix typo

comment:122 by Don-vip, 7 years ago

Milestone: 17.10

it will take some time...

comment:123 by Don-vip, 5 years ago

Owner: changed from Don-vip to team
Status: assignednew

in reply to:  8 comment:124 by mkoniecz, 4 years ago

Replying to stoecker:

Hmm, wouldn't it be more "satisfactory" to first make JOSM callable as standalone validator at all before you make it fully encapsulated? Even with many useless internal dependencies that would be a useful application.

Is it something that is possible? Even via some hacky methods like simulating button clicks via scripted mouse movements? I know that it is not mentioned in this thread, but maybe it got possible via unrelated improvements?

I researched further, and appears that https://wiki.openstreetmap.org/wiki/Osmose#APIs may be an alternative...


explanation of my usecase:

I am processing OSM data to generate SVG image that is then burned onto plywood with laser cutter.

I have a data validation - so far validation was very specific to the program (though some reports like detecting highway=residential connected to road network only with highway=service may make sense in general).

But to avoid mysterious bugs or missing geometries I want to include also geometry detector - waterway=riverbank on unclosed lines and similar.

Is using JOSM validator for such geometry validator a feasible task? Or would it be better to look at Osmose? (writing my own geometry validator, what I briefly considered, is an obviously bad idea).


This specific issue was triggered by https://www.openstreetmap.org/way/752113735 that caused weird crashes in the middle of data processing, so I want to detect such data issues and exit early from my program with a clear error.

Last edited 4 years ago by mkoniecz (previous) (diff)

by taylor.smock, 3 years ago

Attachment: 15182.patch added

Add validate command on JOSM command line. Output is line-by-line delimited geojson, for use in MapRoulette (AFAIK, MapRoulette is the only major "random problem" platform).

in reply to:  8 comment:125 by taylor.smock, 3 years ago

Replying to stoecker:

Hmm, wouldn't it be more "satisfactory" to first make JOSM callable as standalone validator at all before you make it fully encapsulated? Even with many useless internal dependencies that would be a useful application.

Have a patch. :)

Notes on the patch:

  • No tests (yet) Done in attachment:15182.2.patch
  • No way to enable/disable specific tests (yet) Done in attachment:15182.2.patch (via command-line config options)
  • Returned GeoJSON is compatible with RFC8142 (RS/LF characters)
  • Works in Ubuntu docker image with only openjdk-11-jre-headless installed on top
  • Writes to stdout about any tasks that could not be generated (I need to figure out what is going on with them -- probably due to not writing non-MP relations to json)
  • GeoJson writer now has options:
    • Right-hand-rule for polygons (MR uses a stricter geojson format spec, which requires polygons/multipolygons to follow the right hand rule). This needs some testing.
    • Write OSM data to properties. This follows the Overpass Turbo specification (prefix metadata with @, escape @ with another @)
  • GuiHelper#assertCallFromEDT checks if the environment is headless as well before throwing.
Last edited 3 years ago by taylor.smock (previous) (diff)

comment:126 by stoecker, 3 years ago

Summary: Standalone JOSM validator[PATCH] Standalone JOSM validator

comment:127 by taylor.smock, 3 years ago

Summary: [PATCH] Standalone JOSM validator[WIP PATCH] Standalone JOSM validator

@stoecker: Just a heads up, I don't consider the patch ready to apply yet. I uploaded it just in case I got hit by a bus or something. It works, but there are some rough edges. I've got some changes on my work machine which I probably should have uploaded Friday which fixes some of the issues (specifically, it adds the ability to change configuration settings). And since the CLI name was generic (validate), I also added mapcss validation (validator mapcss is included in that).

But I should have added the [WIP PATCH] prefix.

by taylor.smock, 3 years ago

Attachment: 15182.2.patch added

Add tests

comment:128 by taylor.smock, 3 years ago

Summary: [WIP PATCH] Standalone JOSM validator[PATCH] Standalone JOSM validator

comment:129 by taylor.smock, 3 years ago

Summary: [PATCH] Standalone JOSM validator[WIP PATCH] Standalone JOSM validator

Looks like I made a mistake with the progress monitor somewhere.

comment:130 by taylor.smock, 3 years ago

Summary: [WIP PATCH] Standalone JOSM validator[PATCH] Standalone JOSM validator

by taylor.smock, 3 years ago

Attachment: 15182.3.patch added

Fix test issue (from test pollution), fix progress monitor issue

comment:131 by taylor.smock, 3 years ago

For test results, see https://gitlab.com/smocktaylor/josm/-/merge_requests/15 (ignore the test-java-integration job -- it times out after an hour on gitlab.com).

comment:132 by taylor.smock, 3 years ago

2 week ping

comment:133 by Don-vip, 3 years ago

In 18365/josm:

see #15182 - make JOSM callable as standalone validator (patch by taylor.smock)

comment:134 by Don-vip, 3 years ago

Milestone: Longterm

comment:135 by Don-vip, 3 years ago

Awesome, thank you Taylor! Don't hesitate to ping me when I don't respond. I lack time but I'm still around :)

in reply to:  135 comment:136 by taylor.smock, 3 years ago

Replying to Don-vip:

Awesome, thank you Taylor! Don't hesitate to ping me when I don't respond. I lack time but I'm still around :)

I tend to do two week pings -- see wiki:/DevelopersGuide/PatchGuide, "[if] you don't get any reaction for two weeks, it may be a good idea to bring the topic up again and ask for a status update".

I get that all the core maintainers are volunteers, so I don't panic if no one responds to a patch for a week or two. Unless we are getting inundated with bug reports that said patch fixes.

comment:137 by taylor.smock, 3 years ago

Summary: [PATCH] Standalone JOSM validatorStandalone JOSM validator

I've gone ahead and dropped [PATCH] from the summary, as the current patch has been applied.

comment:138 by Don-vip, 3 years ago

Thanks. there's a coverity warning to fix:

New defect(s) Reported-by: Coverity Scan
Showing 1 of 1 defect(s)


** CID 1469108:  SpotBugs: Bad practice  (FB.DM_EXIT)
/src/org/openstreetmap/josm/data/validation/ValidatorCLI.java: 387 in org.openstreetmap.josm.data.validation.ValidatorCLI.handleOption(org.openstreetmap.josm.data.validation.ValidatorCLI$Option)()


________________________________________________________________________________________________________
*** CID 1469108:  SpotBugs: Bad practice  (FB.DM_EXIT)
/src/org/openstreetmap/josm/data/validation/ValidatorCLI.java: 387 in org.openstreetmap.josm.data.validation.ValidatorCLI.handleOption(org.openstreetmap.josm.data.validation.ValidatorCLI$Option)()
381         }
382     
383         private void handleOption(final Option option) {
384             switch (option) {
385             case HELP:
386                 showHelp();
>>>     CID 1469108:  SpotBugs: Bad practice  (FB.DM_EXIT)
>>>     org.openstreetmap.josm.data.validation.ValidatorCLI.handleOption(ValidatorCLI$Option) invokes System.exit(...), which shuts down the entire virtual machine.
387                 System.exit(0);
388                 break;
389             case DEBUG:
390                 this.logLevel = Logging.LEVEL_DEBUG;
391                 break;
392             case TRACE:

in reply to:  138 comment:139 by taylor.smock, 3 years ago

by taylor.smock, 3 years ago

Attachment: 15182.sys_exit.patch added

Replace Sys.exit with Lifecycle.exitJosm, also change some instances of System.err.println to Logging.error.

comment:140 by taylor.smock, 3 years ago

With attachment:15182.sys_exit.patch, I don't know if bastiK had a reason for using System.err over Logging for exceptions.

comment:141 by stoecker, 3 years ago

Question is if Logging is already initialized at that stage. Also if it's possible to configure the software in a way that you don't see errors on call (which would be bad for commandline args parsing).

in reply to:  141 comment:142 by taylor.smock, 3 years ago

Replying to stoecker:

Question is if Logging is already initialized at that stage. Also if it's possible to configure the software in a way that you don't see errors on call (which would be bad for commandline args parsing).

That might be it -- logging can be configured using system properties.

comment:143 by taylor.smock, 3 years ago

In 18366/josm:

Replace usages of System.exit with Lifecycle.exitJosm

This fixes some coverity scan issues (spotbugs) with respect to exiting the VM.

See #15182: Standalone JOSM validator

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The owner will remain team.
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from team to the specified user.
Next status will be 'needinfo'. The owner will be changed from team to Don-vip.
as duplicate The resolution will be set to duplicate. Next status will be 'closed'. The specified ticket will be cross-referenced with this ticket.
The owner will be changed from team to anonymous. Next status will be 'assigned'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.