Skip to content

Use Location in the I/O API#395

Merged
ctrueden merged 6 commits into
masterfrom
use-location
Aug 13, 2020
Merged

Use Location in the I/O API#395
ctrueden merged 6 commits into
masterfrom
use-location

Conversation

@frauzufall

@frauzufall frauzufall commented Aug 12, 2020

Copy link
Copy Markdown
Member

Following this gitter discussion I cherry-picked this commit and re-added the String methods to hopefully keep backwards compatibility.

I also added a test, but it's a bit dull. I forgot how to include a test SciJavaPlugin without having to define a public class, can someone remind me?

Happy for feedback!

@ctrueden @frauzufall
Use Location, not String, in the I/O API
b7285c5 
This is much more elegant. It is also a very breaking change.

Co-authored-by: Deborah Schmidt <mail@frauzufall.de>
@frauzufall

frauzufall commented Aug 12, 2020

Copy link
Copy Markdown
Member Author

Here is what revapi has to say when comparing this branch to 2.83.3:

[ERROR] Failed to execute goal org.revapi:revapi-maven-plugin:0.11.4:check (default-cli) on project scijava-common: The following API problems caused the build to fail:
[ERROR] java.class.noLongerImplementsInterface: class org.scijava.io.AbstractIOPlugin<D extends java.lang.Object>: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.class.noLongerImplementsInterface: class org.scijava.io.AbstractIOPlugin<D extends java.lang.Object>: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.class.noLongerImplementsInterface: class org.scijava.io.AbstractIOPlugin<D extends java.lang.Object>: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.class.noLongerImplementsInterface: class org.scijava.io.DefaultIOService: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.class.noLongerImplementsInterface: class org.scijava.io.DefaultIOService: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.method.returnTypeTypeParametersChanged: method java.lang.Class<org.scijava.io.location.Location> org.scijava.io.IOPlugin<D>::getType(): The return type changed from 'java.lang.Class<java.lang.String>' to 'java.lang.Class<org.scijava.io.location.Location>'.
[ERROR] java.class.noLongerImplementsInterface: interface org.scijava.io.IOPlugin<D extends java.lang.Object>: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.method.returnTypeTypeParametersChanged: method java.lang.Class<org.scijava.io.location.Location> org.scijava.io.IOService::getType(): The return type changed from 'java.lang.Class<java.lang.String>' to 'java.lang.Class<org.scijava.io.location.Location>'.
[ERROR] java.method.addedToInterface: method java.lang.Object org.scijava.io.IOService::open(org.scijava.io.location.Location) throws java.io.IOException: Method was added to an interface.
[ERROR] java.method.addedToInterface: method void org.scijava.io.IOService::save(java.lang.Object, org.scijava.io.location.Location) throws java.io.IOException: Method was added to an interface.
[ERROR] java.class.noLongerImplementsInterface: interface org.scijava.io.IOService: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.method.parameterTypeChanged: parameter void org.scijava.io.event.DataOpenedEvent::<init>(===org.scijava.io.location.Location===, java.lang.Object): The type of the parameter changed from 'java.lang.String' to 'org.scijava.io.location.Location'.
[ERROR] java.method.removed: method java.lang.String org.scijava.io.event.DataOpenedEvent::getSource(): Method was removed.
[ERROR] java.method.parameterTypeChanged: parameter void org.scijava.io.event.DataSavedEvent::<init>(===org.scijava.io.location.Location===, java.lang.Object): The type of the parameter changed from 'java.lang.String' to 'org.scijava.io.location.Location'.
[ERROR] java.method.removed: method java.lang.String org.scijava.io.event.DataSavedEvent::getDestination(): Method was removed.
[ERROR] java.method.parameterTypeChanged: parameter void org.scijava.io.event.IOEvent::<init>(===org.scijava.io.location.Location===, java.lang.Object): The type of the parameter changed from 'java.lang.String' to 'org.scijava.io.location.Location'.
[ERROR] java.method.removed: method java.lang.String org.scijava.io.event.IOEvent::getDescriptor(): Method was removed.
[ERROR] java.class.noLongerImplementsInterface: class org.scijava.script.io.ScriptIOPlugin: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.class.noLongerImplementsInterface: class org.scijava.script.io.ScriptIOPlugin: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.class.noLongerImplementsInterface: class org.scijava.script.io.ScriptIOPlugin: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.class.noLongerImplementsInterface: class org.scijava.text.io.TextIOPlugin: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.class.noLongerImplementsInterface: class org.scijava.text.io.TextIOPlugin: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.class.noLongerImplementsInterface: class org.scijava.text.io.TextIOPlugin: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.

@imagejan

imagejan commented Aug 12, 2020

Copy link
Copy Markdown
Member

@frauzufall wrote:

I forgot how to include a test SciJavaPlugin without having to define a public class, can someone remind me?

Do you mean this?

https://github.com/scijava/scijava-table/blob/57e5472be079a7d65a89b2a9565fc94a227b8d05/src/test/java/org/scijava/table/io/TableIOServiceTest.java#L58-L60

@frauzufall

frauzufall commented Aug 12, 2020 •
edited
Loading

Copy link
Copy Markdown
Member Author

EDIT I made a separate PR #396 for the TypedIOService stuff.

@frauzufall frauzufall mentioned this pull request Aug 12, 2020
Merged
@frauzufall
Re-add reprecated methods
7d4a580 
* they were removed in favor of using Location instead of String for IO
handling
* they are re-added to restore backwards compatibility
@frauzufall
IOService: add default body to open(Location) and save(Object, Location)
d255cdd 
- this restores backwards-compatibility
@frauzufall

frauzufall commented Aug 12, 2020

Copy link
Copy Markdown
Member Author

I fixed the most important API changes, this is the new status of revapi:

[ERROR] java.class.noLongerImplementsInterface: class org.scijava.io.AbstractIOPlugin<D extends java.lang.Object>: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.class.noLongerImplementsInterface: class org.scijava.io.AbstractIOPlugin<D extends java.lang.Object>: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.class.noLongerImplementsInterface: class org.scijava.io.AbstractIOPlugin<D extends java.lang.Object>: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.class.noLongerImplementsInterface: class org.scijava.io.DefaultIOService: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.class.noLongerImplementsInterface: class org.scijava.io.DefaultIOService: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.method.returnTypeTypeParametersChanged: method java.lang.Class<org.scijava.io.location.Location> org.scijava.io.IOPlugin<D>::getType(): The return type changed from 'java.lang.Class<java.lang.String>' to 'java.lang.Class<org.scijava.io.location.Location>'.
[ERROR] java.class.noLongerImplementsInterface: interface org.scijava.io.IOPlugin<D extends java.lang.Object>: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.method.returnTypeTypeParametersChanged: method java.lang.Class<org.scijava.io.location.Location> org.scijava.io.IOService::getType(): The return type changed from 'java.lang.Class<java.lang.String>' to 'java.lang.Class<org.scijava.io.location.Location>'.
[ERROR] java.class.noLongerImplementsInterface: interface org.scijava.io.IOService: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.class.noLongerImplementsInterface: class org.scijava.script.io.ScriptIOPlugin: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.class.noLongerImplementsInterface: class org.scijava.script.io.ScriptIOPlugin: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.class.noLongerImplementsInterface: class org.scijava.script.io.ScriptIOPlugin: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.class.noLongerImplementsInterface: class org.scijava.text.io.TextIOPlugin: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.class.noLongerImplementsInterface: class org.scijava.text.io.TextIOPlugin: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.
[ERROR] java.class.noLongerImplementsInterface: class org.scijava.text.io.TextIOPlugin: Class no longer implements interface 'org.scijava.Typed<java.lang.String>'.

Not sure if there are more possibilities to fix any of this or if it would be desired at all.

I probably don't have anything to add to this PR until someone gives feedback.

This was referenced Aug 13, 2020
Merged
Merged
Merged
@ctrueden
ctrueden merged commit 6ada478 into master Aug 13, 2020
@ctrueden
ctrueden deleted the use-location branch August 13, 2020 16:35
@imagejan

imagejan commented Nov 1, 2020

Copy link
Copy Markdown
Member

@frauzufall wrote:

re-added the String methods to hopefully keep backwards compatibility.

Unfortunately, this still is a breaking change, as IOPlugin changes from HandlerPlugin<String> to HandlerPlugin<Location>, so existing IOPlugin implementations (as far as the extend AbstractIOPlugin) need at least be recompiled against this newer version of scijava-common (if I understand correctly).

See also #406.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@frauzufall @imagejan @imagesc-bot @ctrueden