Thanks Madhawa for the PR. I will have a look as well. Thanks, Imesha
On Thu, 8 Nov 2018 at 23:16, Madhawa Vidanapathirana < madhawavidanapathir...@gmail.com> wrote: > 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/>* > >> > >> > >> > >> > >> > >> >
