code review of obd-ws

5 views
Skip to first unread message

Hilmar Lapp

unread,
May 1, 2009, 3:49:13 PM5/1/09
to OBD Development
FYI, we did a code review of the obd-ws sub-project under obd. You may
have seen a number of commits resulting from that, and there are a
number of FIXME tags in the source code now for stuff that need more
attention (and time).

Here are the remaining results.

> * OBD-WS.war is committed to the repository - not necessary and a
> grave security hole

This is fixed.

> * context.xml contains our database URL and host name - security
> hole and won't work for anyone else

Pending.

> * .settings/org.eclipse.jst.common.project.facet.core.prefs may
> contain local paths or settings committed to the repository

Pending.

> * ant or maven build file missing
> * build/ directory is committed to the repository rather than
> created from scratch by the build process

This is fixed, thanks to Jim.

>
> * WebContent/META-INF/MANIFEST.MF should not be committed - it is
> normally generated by the war file packaging process (e.g., using jar)

This is fixed.

>
> * WebContent/WEB-INF/lib directory includes stuff that it shouldn't
> ** phenote.jar with lots of GUI classes and even OBO ontology files
> - this is surely not needed but can can lead to errors if they are
> accidentally used
> ** phenex.jar has GUI classes involving Swing - same as before applies

Both of these are removed now.

> ** oboedit.jar - same as above, and we surely don't need most of
> this stuff in a server-side middleware application
> ** bbop.jar contains lots of Swing classes that we surely don't need
> and can only lead to problems due to headless operation

Both pending.

> ** junit-4.5.jar - why is this needed while running the web
> application? (rather than only when testing it)

This is fixed. I added a lib/ directory.

> * org.nescent.informatics package needs to be refactored b/c it
> contains phenoscape code, not generic NESCent informatics code. E.g.
> org.nescent.informatics.phenoscape, or org.nescent.phenoscape, or
> (and probably best) org.phenoscape.obd

This is fixed.

> * constants need to be final

Fixed for OBDQuery.java where the constants were already declared.
There's more stuff in there that needs to be declared as constants
(tagged with FIXME).

-hilmar
--
===========================================================
: Hilmar Lapp -:- Durham, NC -:- hlapp at duke dot edu :
===========================================================


Reply all
Reply to author
Forward
0 new messages