Hi Chris, Please find the PR here - https://github.com/apache/oodt/pull/75
I have modified some unit tests involving data repository path in order to expose the issue. I didn't write new ones because they will become mere duplicates of existing ones, with the only difference being spaces added to repository path. If you think it's better to have separate unit tests, I can add them as new unit test methods adjacent to existing ones. Afterward, I replaced the deprecated method in code and re-run the unit tests and everything passed. Please let me know if any changes are required to the PR. Kind regards, *Madhawa Vidanapathirana* Student Department of Computer Science and Engineering University of Moratuwa Sri Lanka Mobile: (+94) 716874425 Email: madhawavidanapathir...@gmail.com Linked-In: *https://www.linkedin.com/in/madhawav/ <https://www.linkedin.com/in/madhawav/>* On Wed, Nov 7, 2018 at 4:06 PM Madhawa Vidanapathirana < madhawavidanapathir...@gmail.com> wrote: > Hi Chris, > > As you said, I will write unit tests to expose some scenarios where spaces > cause issues. Afterward, I would replace all the cases of File.toURL() as > described in JAVADOC, re-run all the tests and send a PR. > > Kind Regards, > > *Madhawa Vidanapathirana* > Student > Department of Computer Science and Engineering > University of Moratuwa > Sri Lanka > > Mobile: (+94) 716874425 > Email: madhawavidanapathir...@gmail.com > Linked-In: *https://www.linkedin.com/in/madhawav/ > <https://www.linkedin.com/in/madhawav/>* > > > On Wed, Nov 7, 2018 at 10:00 AM Chris Mattmann <mattm...@apache.org> > wrote: > >> Let’s add some unit tests to expose the issue, and then with your PR you >> can have your >> fix show that it makes those unit tests pass. Let’s try that? >> >> >> >> >> >> >> >> From: Imesha Sudasingha <imesha...@cse.mrt.ac.lk> >> Reply-To: "dev@oodt.apache.org" <dev@oodt.apache.org> >> Date: Tuesday, November 6, 2018 at 8:06 PM >> To: dev <dev@oodt.apache.org> >> Subject: Re: Deprecated File.toURL() and issues with space characters in >> paths >> >> >> >> Hi Madhawa, >> >> >> >> Thanks for pointing this issue out. >> >> >> >> IMO this is an issue which should be fixed in order to make sure that new >> >> comers are not experiencing failures/difficulties in the first go (since >> >> you too got blocked in the first attempt) and in overall for the stability >> >> of OODT. >> >> >> >> However, I would like if a more experienced member comment his/her opinion >> >> on fixing this issue. >> >> @Chris, @Tom what do you think? >> >> >> >> Thanks, >> >> Imesha >> >> >> >> >> >> >> >> >> >> On Mon, 5 Nov 2018 at 21:56, Madhawa Vidanapathirana < >> >> madhawavidanapathir...@gmail.com> wrote: >> >> >> >> Hi, >> >> I am Madhawa, a recent graduate from Department of Computer Science and >> >> Engineering, University of Moratuwa, Sri Lanka. I am new to Apache OODT >> and >> >> I thought of starting off with the file manager module. However, from the >> >> very beginning, I had trouble due to space characters in paths and I am >> >> interested in fixing these issues. >> >> >> >> I already fixed a space character related issue [1] on the start script of >> >> filemgr, which is now merged. Afterward, I realized that when the >> >> repository path contains space characters, ingest operations fail. After >> >> observing the code, I identified that this error occurs because the >> >> method *File.toURL >> >> (which is now deprecated) fails to automatically escape illegal characters >> >> in paths*. This deprecated method is used in multiple places of the >> >> codebase and I raised the issue [2] mentioning this situation. >> >> >> >> As a solution to the above issue, JAVADOC [3] recommends to first convert >> >> the File to a URI (using File.toURI() method) and then convert the URI to >> a >> >> URL (using URI.toURL() method). *Before going forward, since this code >> >> change affects multiple areas of the code, I would like to know >> suggestions >> >> and comments from the community on this approach.* >> >> >> >> My plan is to clone the project to a path with space characters and run >> >> unit tests to validate the fix. However, it seems this approach is not so >> >> trivial since some of the unit tests are written under the assumption that >> >> space characters would not appear in paths. Therefore, *we may have to >> >> modify those unit tests as well. * >> >> >> >> I would like to know your opinion on above-mentioned approaches to fix the >> >> issue and modify the unit tests >> >> >> >> [1] - https://issues.apache.org/jira/projects/OODT/issues/OODT-998 >> >> [2] - https://issues.apache.org/jira/projects/OODT/issues/OODT-999 >> >> [3] - https://docs.oracle.com/javase/7/docs/api/java/io/File.html#toURL() >> >> >> >> Kind Regards >> >> >> >> *Madhawa Vidanapathirana* >> >> Department of Computer Science and Engineering >> >> University of Moratuwa >> >> Sri Lanka >> >> >> >> Mobile: (+94) 716874425 >> >> Email: madhawavidanapathir...@gmail.com >> >> Linked-In: *https://www.linkedin.com/in/madhawav/ >> >> <https://www.linkedin.com/in/madhawav/>* >> >> >> >> >> >>
