Bug 553135 - IllegalArgumentException opening git properties page due to RepositoryHandle
Summary: IllegalArgumentException opening git properties page due to RepositoryHandle
Status: RESOLVED FIXED
Alias: None
Product: EGit
Classification: Technology
Component: UI (show other bugs)
Version: 5.6   Edit
Hardware: PC All
: P3 normal (vote)
Target Milestone: 5.6   Edit
Assignee: Thomas Wolf CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-11-17 11:54 EST by Michael Keppler CLA
Modified: 2019-11-18 11:21 EST (History)
1 user (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Keppler CLA 2019-11-17 11:54:06 EST 
I get an exception opening the statistics properties page of any repo. Root cause is "if (repo instanceof FileRepository)" in GarbageCollectCommand. repo is now a RepositoryHandle, therefore returning empty properties.

@Thomas, can you please check if there are other places where an instanceof can fail since the introduction of RepositoryHandle?


java.lang.IllegalArgumentException: Cannot format given Object as a Number
	at java.text.DecimalFormat.format(DecimalFormat.java:507)
	at java.text.Format.format(Format.java:157)
	at org.eclipse.egit.ui.internal.repository.RepositoryStatisticsPage.getStatsAsString(RepositoryStatisticsPage.java:114)
	at org.eclipse.egit.ui.internal.repository.RepositoryStatisticsPage.createContents(RepositoryStatisticsPage.java:84)
	at org.eclipse.jface.preference.PreferencePage.createControl(PreferencePage.java:241)
	at org.eclipse.jface.preference.PreferenceDialog.createPageControl(PreferenceDialog.java:1430)
	at org.eclipse.jface.preference.PreferenceDialog$9.run(PreferenceDialog.java:1197)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42)
Comment 1 Thomas Wolf CLA 2019-11-17 12:28:53 EST 
A search over all Java files in EGit and JGit for "instanceof\s+\w*Repository" only finds org.eclipse.jgit.api.GarbageCollectCommand.

Not sure what's the right approach here. I wonder what that asynchronous GC does if the repo is closed asynchronously...

We can't derive RepositoryHandle from FileRepository; the constructor would get in the way.

Looks like we need a way to get the delegate as a FileRepository. I hadn't planned on making RepositoryHandle public... how about a static utility method in RepositoryCache or RepositoryUtil

  public static FileRepository toFileRepository(Repository repository) {
    Repository toConvert = repository;
    if (toConvert instanceof RepositoryHandle) {
      toConvert = ((RepositoryHandle) toConvert).getDelegate();
    }
    if (toConvert instanceof FileRepository) {
      return (FileRepository) toConvert;
    }
    return null; // Or throw an exception?
  }

and use that in org.eclipse.egit.core.op.GarbageCollectOperation and wherever else GarbageCollectCommand is used directly?
Comment 2 Michael Keppler CLA 2019-11-17 14:01:15 EST 
Sounds fine to me. I would throw IllegalStateException instead of returning null. Similar to throwing IllegalStateException after a complete switch over all enum values. :)
Comment 3 Eclipse Genie CLA 2019-11-17 17:34:51 EST 
New Gerrit change created: https://git.eclipse.org/r/152839
Comment 4 Eclipse Genie CLA 2019-11-18 07:45:04 EST 
New Gerrit change created: https://git.eclipse.org/r/152860
Comment 5 Eclipse Genie CLA 2019-11-18 08:30:12 EST 
Gerrit change https://git.eclipse.org/r/152839 was merged to [master].
Commit: http://git.eclipse.org/c/egit/egit.git/commit/?id=2fe4ef34d94c5bea0820040ff2a12ef17dde12bd
Comment 6 Eclipse Genie CLA 2019-11-18 11:20:51 EST 
Gerrit change https://git.eclipse.org/r/152860 was merged to [master].
Commit: http://git.eclipse.org/c/egit/egit.git/commit/?id=dd5fbd20a40ab0f22d35e41d5db902a612a3b3c6