appDB: Nice URLs live demo and code review

[Chris Morgan]

Hey John.

Looks like you've gotten the nice urls working, great job man. It will
be great to have this before 1.0 is shipped since it will make our
urls a lot nicer looking.

I had a few comments about the code changes. You don't follow the
variable naming convention of the appdb. We have some prefixes for
variables that we use to keep the types of variables straight, you can
find these in the coding standards file in the root of the appdb tree.

The code in 0005-matchByVerbAndTerm.php-view-that-calls-into-the.patch.txt
looks like it is using query_appdb() to perform a direct query. The
indexing code also appears to do this. We prefer using the query
wrapper function query_parameters() as this performs escaping of input
parameters and helps to reduce the possibility of sql injection
vulnerabilities.

I noticed the code to perform the grouped inserts. What is the
performance difference between perform the inserts individually vs.
grouped together? If we run the indexer daily and its relatively quick
we may be able to drop the complexity of worrying about how many
inserts we can group together.

0005-matchByVerbAndTerm.php-view-that-calls-into-the.patch.txt appears
to be generating output for display. It would be useful to have some
comments at the head of the file to describe what the file is used for
and why it would need to conditionally output lists when don't find a
object page match.

Thanks,
Chris




On Mon, Apr 7, 2008 at 5:05 PM, John Klehm <xixsimplicityxix at gmail.com> wrote:
> Give nice urls a whirl at:
>  http://appdb.klehm.net/app/"yoursearchtermhere"
>
>  Search Examples:
>  http://appdb.klehm.net/app/adobe
>  http://appdb.klehm.net/app/visual
>
>  Direct Link Examples:
>  http://appdb.klehm.net/app/photoshop
>  http://appdb.klehm.net/app/command-conquer-red-alert-2
>
>  See the nice urls being used internally:
>  http://appdb.klehm.net/appbrowse.php?iCatId=5
>
>
>  Right now as it is I have it indexing only the app family names; no
>  versions or vendors get any love.
>
>  There is one piece of functionality missing before this can go in and
>  that is to do an index of a new record upon creation.
>
>  If anyone wants to test just apply the patch set and then flip
>  APPDB_ENABLE_NICE_URLS_INDEXER to true and then run: runIndexer.php.
>  Be sure to flip enable indexer back to false before doing anything
>  else =P.
>
>  You also need to set allow_url_fopen = On in your php.ini.
>  If this is deemed to be unacceptable then we have to functionalize the
>  objectManager.php file so matchByVerbAndTerm can generate similar
>  output.  Using url fopen allows easiest integration.
>
>
>  After that you should be good to go to test.
>
>  Essentially less terms will work better and the matching "nice" name
>  depends entirely upon whatever the maintainer entered for the app
>  family name.
>
>  Let me know if any changes need be made otherwise ill send it in later
>  this week.
>
>  Regards,
>  John Klehm
>
>
>
>