jabref icon indicating copy to clipboard operation
jabref copied to clipboard

Refactoring the Usage of Deprecated Java APIs

Open viktorkrimstein opened this issue 1 year ago • 4 comments

Background

When running Code -> Analyze Code -> Run Inspection by Name -> Deprecated API Usage (Java | Code Maturity in the IntelliJ IDEA, I find 142 warnings about the usage of deprecated Java APIs in the code base.

Solution

Refactor the deprecated API usages without changing the functionality and performance of the given code base.

Acceptance Criteria

When running the workflow despcibed in the Background section, no depracted API usages are documented in the main branch. In a situation, when the usage of deprecated APIs is currently necessary and inevitable, a clear JavaDoc is provided why this is the case and marked for later refactoring.

viktorkrimstein avatar Aug 26 '24 10:08 viktorkrimstein

@viktorkrimstein Please show up at my office of University of Stuttgart - or at my second office Sindelfingen; and we can pair on this. -- If you feel brave enough, you can open pull requests for yourself.

koppor avatar Aug 26 '24 18:08 koppor

@viktorkrimstein Some more ideas: Rules for refactoring could be added to OpenRewrite. Maybe, you can craft minimal examples and create issues at the OpenRewrite repository. With that, the whole Java ecosystem would benefit.

koppor avatar Aug 26 '24 18:08 koppor

@koppor I've already forked the project and am currently working an that. On the way finding more and more Bugs (e.g. that the ACS publication parser simply runs into HTTP 403: Forbidden). Trying to fix as much as I manage on the way.

Showing up at the University of Stuttgart would be great again but impossible due to family and work (Graduated with an M.Sc. years ago šŸ˜„)

I'm using JabRef since then and wanted to find some hustle to stay sane in the corporate IT world.

viktorkrimstein avatar Aug 27 '24 14:08 viktorkrimstein

@koppor I've already forked the project and am currently working an that. On the way finding more and more Bugs (e.g. that the ACS publication parser simply runs into HTTP 403: Forbidden). Trying to fix as much as I manage on the way.

Nice. We love smaller pull requests. Maybe https://gitbutler.com/ helps to craft these. -- It has all your changes integrated locally, but can manage multiple upstream PRs for the changes.

Regarding fetchers, you might find some help at https://devdocs.jabref.org/code-howtos/fetchers.html.

I'm using JabRef since then and wanted to find some hustle to stay sane in the corporate IT world.

It was a similar story here. Glad to hear about others experiencing similar things šŸ˜…

koppor avatar Aug 27 '24 17:08 koppor

I want to join this issue since I love refactor and code quality

leaf-soba avatar Sep 24 '24 12:09 leaf-soba

I want to join this issue since I love refactor and code quality

Sure. Please try to do small commits / pull requests. Maybe one for each found inspection - or split up into the automatically done things and manual adaptions.

Also check if openrewrite has a recipe for it: Example: https://github.com/JabRef/jabref/pull/11824

@leaf-soba But please keep in mind that the changes should be covered by tests (to avoid regressions) and that tests itself should only be modifeid if the code itself is modified.

We all do this project in our free time and have limited time to improve the software as is. Thus, please take refactorings as a learning for improving the whole system.

koppor avatar Sep 24 '24 12:09 koppor

I want to join this issue since I love refactor and code quality

Sure. Please try to do small commits / pull requests. Maybe one for each found inspection - or split up into the automatically done things and manual adaptions.

Also check if openrewrite has a recipe for it: Example: #11824

@leaf-soba But please keep in mind that the changes should be covered by tests (to avoid regressions) and that tests itself should only be modifeid if the code itself is modified.

We all do this project in our free time and have limited time to improve the software as is. Thus, please take refactorings as a learning for improving the whole system.

OK, so far almost all code I contributed to this project are tested, except some test code itself or some really hard to test code.

leaf-soba avatar Sep 25 '24 01:09 leaf-soba

If you open a pr within this issue scope, please add a 'refs' to this issue in the pr description, so we can track it.

calixtus avatar Sep 26 '24 06:09 calixtus

It's Hacktoberfest this month - a good chance to resume work on this!

koppor avatar Oct 08 '24 22:10 koppor

I want to replace Deprecated org.jabref.logic.bibtex.InvalidFieldValueException to org.jabref.logic.integrity.IntegrityCheck, but I find it very difficult to refactor it, since the IntegrityCheck need a lot of input parameters such as BibDatabaseContext,FilePreferences,CitationKeyPatternPreferences, but some class with InvalidFieldValueException don't have them, do I need add all these parameters to any class with InvalidFieldValueException? @koppor

leaf-soba avatar Oct 21 '24 02:10 leaf-soba

I saw you put a @Deprecated on org.jabref.logic.preferences.JabRefCliPreferences#getInstance:

/**
     * @return Instance of JaRefPreferences
     * @deprecated Use {@link CliPreferences} instead
     */
    @Deprecated
    public static JabRefCliPreferences getInstance() {
        if (JabRefCliPreferences.singleton == null) {
            JabRefCliPreferences.singleton = new JabRefCliPreferences();
        }
        return JabRefCliPreferences.singleton;
    }

but the CliPreferences is an interface which can't return a singleton, could you tell me what should I do to remove this Deprecated method? @koppor

leaf-soba avatar Oct 21 '24 03:10 leaf-soba

I remember that InvalidFieldValueException was one of the more complicated things to refactor, since we wanted to move away from exception that control program flow and only use them for really exceptional situations (see https://github.com/HugoMatilla/Effective-JAVA-Summary?tab=readme-ov-file#57-use-exceptions-only-for-exceptional-conditions). I believe refactoring that is not just about moving methods and variables around, but requires a lot of more thinking and changes to the logical behavior.

calixtus avatar Oct 21 '24 12:10 calixtus

Preferences should not be a Singleton, but created in the Launcher and then be injected.

calixtus avatar Oct 21 '24 12:10 calixtus

If you want to refactor, please work first on the easy ones, there are plenty in the code. You start with the complicated ones. As we have day jobs it's pretty hard to keep up otherwise.

calixtus avatar Oct 21 '24 12:10 calixtus

If you want to refactor, please work first on the easy ones, there are plenty in the code. You start with the complicated ones. As we have day jobs it's pretty hard to keep up otherwise.

I’d like to close this issue, but the last part has been quite challenging due to the InvalidFieldValueException deprecation problem. Apologies if this has taken up more time than expected, and thank you for your patience.

leaf-soba avatar Oct 21 '24 13:10 leaf-soba

Most issues are fixed. For the methods we want to list strike-through, we keep the annotation (and added some explanaining text)

The two small left overs are listed as follows and are not worth to keep this issue open

  • org.jabref.model.groups.GroupTreeNode#setGroup(org.jabref.model.groups.AbstractGroup)
  • org.jabref.logic.ai.util.ErrorMessage#text

@viktorkrimstein I would be interested what kept you from fixing this issue. We have plenty of other small issues labeld with good first issues, which can be used as possibility to work on in the free time. -- @leaf-soba Thank you for fixing things. Also thanks to OpenRewrite supporting mass refactoring.

koppor avatar Oct 22 '24 06:10 koppor