oracle / opengrok

OpenGrok is a fast and usable source code search and cross reference engine, written in Java
http://oracle.github.io/opengrok/
Other
4.35k stars 749 forks source link

Project name case sensitivity confusion with ProjectsController #4006

Closed vladak closed 2 years ago

vladak commented 2 years ago

Using the API to add a project foo, then renaming the project to Foo, adding it using the same API and defining a project group with pattern ^Foo, my webapp instance ended up with the group being empty. The resulting serialized configuration had both projects, however the project group had no projects.

Debugging the issue: when RuntimeEnvironment#populateGroups() was called: https://github.com/oracle/opengrok/blob/dcf109140b725f36018062b770811181a8247045/opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/RuntimeEnvironment.java#L1645 , the getProjects().values() had one extra entry compared to the resulting TreeSet. That extra entry was the project with name starting with upper case letter, which means the TreeSet contained just the project with lower case letter and thus not matched by the project group regexp. This is because Project#equals() method converts the project name to upper case and performs the comparison of the names: https://github.com/oracle/opengrok/blob/dcf109140b725f36018062b770811181a8247045/opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/Project.java#L614-L615

The problem is that ProjectsController#addProject() (which handles the above mentioned API call) ignores this project name comparison requirement: https://github.com/oracle/opengrok/blob/dcf109140b725f36018062b770811181a8247045/opengrok-web/src/main/java/org/opengrok/web/api/v1/controller/ProjectsController.java#L121

vladak commented 2 years ago

The upper case based comparison is not really baked into the code that deals with Project map projects in Configuration which makes this difficult to fix - either make sure that each key addition to the map respects the comparator or introduce new class (say ProjectName) that would contain the comparator logic, however that would change Configuration serialization since the project map is stored there.

Another alternative would be to remove the upper case based comparison. It was introduced with project groups (d470e59c) and it is not clear to me and I don't remember what was the intention @tulinkry. The Group comparator also uses this approach.