70%: Introduce namespaces #56

rsinger · 2015-05-19T13:46:37Z

This branch introduces namespaces to Tripod.

Feel free to argue about which classes should go in which namespaces and how they should be named.

The way I've divided them up currently (although this is totally open for discussion) is:

Tripod

~~ITripod~~ IDriver
ITripodStat
ChangeSet
ExtendedGraph
Labeller
Timer
~~ITripodSearchProvider (perhaps could just be iSearchProvider, thoughts?)~~ ISearchProvider (formerly iTripodSearchProvider)

Tripod\Mongo (btw, it will take forever to get used to this inversion)

~~Tripod~~ Driver (formerly MongoTripod)
~~TripodBase~~ DriverBase (formerly MongoTripodBase)
MongoGraph
Labeller (formerly MongoTripodLabeller)
ImpactedSubject (in current master, this is ModifiedSubject, but that concept has been changed in this feature)
Config (formerly MongoTripodConfig)
TriplesUtil
IndexUtils
NQuadSerializer (formerly MongoTripodNQuadSerializer)
MongoSearchProvider (this wasn't renamed because this is defining a Mongo-based search provider)
Updates (formerly MongoTripodUpdates)
TransactionLog (formerly MongoTransactionLog - this might have been a mistake, as it's probably more like MongoSearchProvider)
SearchDocuments (formerly MongoTripodSearchDocuments)

Tripod\Mongo\Composites

IComposite (views, tables, and search are now defined as composites, this is an interface common among all)
CompositeBase
Views (formerly MongoTripodViews. ~~Do the composites need their own part of the namespace? I feel that might be overkill~~)
Tables (formerly MongoTripodTables)
SearchIndexer (formerly MongoTripodSearchIndexer)

Tripod\Mongo\Jobs

ApplyOperation (this is the background job to generate the composites - ~~the jobs could arguably have their own Tripod\Mongo\Jobs namespace. Discuss~~ moved into their own package)
DiscoverImpactedSubjects (same as above)
JobBase (same)

Tripod\Exceptions

Exception (formerly TripodException)
CardinalityException (formerly TripodCardinalityException)
ConfigException (now it's own file, previously was bundled with MongoTripodConfig as MongoTripodConfigException)
LabellerException (formerly TripodLabellerException)
SearchException (formerly TripodSearchException)
ViewException (formerly TripodViewException - is this MongoTripod-specific?)
TimerException

Where class names changed, if they had any associated getter methods, these would have changed, as well. This was kind of a big undertaking, so there may be some that were missed, so feel free to point any out that you might run across.

Also, as I was going through the classes that were imported from legacy libraries (e.g. ExtendedGraph), I cleaned things up a bit, adding docblocks, removing unused (or broken) methods, etc.

scaleupcto · 2015-05-22T07:01:29Z

Should MongoGraph and Extended graph simply be Tripod\Graph and Tripod\Mongo\Graph respectively?

scaleupcto · 2015-05-22T07:03:25Z

It bugs be that ITripod and Tripod\Mongo\Tripod are not nouns. Everything else is (apart from Jobs which are verbs but I think we can let that slide as classname = jobname in resque).

Apart from the obvious Manager I can't think of a better name right now but I do think we need to and now is the time ;-)

rsinger · 2015-05-22T07:03:42Z

I do like that. Part of me kind of thinks that ExtendedGraph could useful as its own library, but lets not go insane.

scaleupcto · 2015-05-22T07:04:24Z

ITripodSearchProvider -> ISearchProvider +1. We are using capital I right?

scaleupcto · 2015-05-22T07:05:03Z

jobs should have their own package.

rsinger · 2015-05-22T07:05:12Z

Yes, i was a typo.

scaleupcto · 2015-05-22T07:05:41Z

Composites should have their own package, i.e. Tripod\Mongo\Composites\Views

rsinger · 2015-05-22T07:06:50Z

@RobotRobot I can easily buy Tripod\Mongo\Jobs, but I think I'm on the fence on Composites - what's your conviction to it?

rsinger · 2015-05-22T07:07:29Z

Tripod -> Client ?

rsinger · 2015-05-22T07:07:50Z

i.e. new \Tripod\Mongo\Client()

rsinger · 2015-05-22T07:08:42Z

(also, in what dialect is 'Tripod' a verb? Is this a Staffordshire thing?)

scaleupcto · 2015-05-22T07:15:06Z

It's not a verb. But it's not a common noun in the sense that it describes the purpose of the class, if you see what I mean.

I like Client.

rsinger · 2015-05-22T07:15:54Z

Oh, I see, right. I agree that the 'library' is Tripod, not the class.

scaleupcto · 2015-05-22T07:16:36Z

Another idea (not necessarily better than client, and keep them coming) we could keep ITripod, but Tripod itself could become Driver. That way you have a mongo Driver for ITripod. Let's keep going on this...

rsinger · 2015-05-22T07:17:15Z

I'm not sure I like IClient very much, though. Could the interface name be ITripodClient (or Driver, I like that, too -- can't say I prefer one over the other)

scaleupcto · 2015-05-22T07:17:56Z

re. Composites, just general tidyness. That package looks crowded. I would drop the Base from the base classnames and move them to a sub package too.

rsinger · 2015-05-22T07:18:05Z

Actually, the more I think of it, the more I like the argument behind 'mongo Driver for iTripod.

scaleupcto · 2015-05-22T07:20:04Z

Right that's it in terms of feedback from me today, will be offline rest of day so if you need to get this merged you have my opinions... ;-)

rsinger · 2015-05-22T07:21:18Z

Ok, that's reasonable. When spiking this out, I was concerned that it was just pushing the clutter to the namespaces (and that that might be harder for the uninitiated to navigate), but I don't have a strong opinion on that (especially now that I'm on the other end of the process)

malcyL · 2015-05-22T08:04:11Z

scripts/mongo/createTables.php

@@ -82,10 +82,10 @@ function showUsage()
 */
 function generateTables($id, $tableId, $storeName, $stat = null)
 {
-    $tableSpec = MongoTripodConfig::getInstance()->getTableSpecification($storeName, $tableId);
+    $tableSpec = TripodConfig::getInstance()->getTableSpecification($storeName, $tableId);


We seem to have \Tripod\Mongo\Config in other places, but TripodConfig here. Is this correctly named?

rgubby · 2015-05-28T11:02:47Z

src/ITripod.php

@@ -1,4 +1,11 @@
 <?php
+
+namespace Tripod;


We've prefixed the namespace in another lib with Talis - do we want to do something similar here?
https://github.com/talis/persona-php-client/blob/master/src/Talis/Persona/Client/Login.php#L2

If we do it'll need an update to the composer.json file in order to work

I'm not terribly keen on this, but I'll defer to @RobotRobot

rgubby · 2015-05-28T12:08:01Z

test/unit/mongo/MongoTripodViewsTest.php

@@ -251,7 +253,7 @@ public function testGenerateViewWithCountAggregate()
            )
        );

-        $actualView = MongoTripodConfig::getInstance()->getCollectionForView('tripod_php_testing', 'v_counts')->findOne(array('_id'=>array("r"=>'http://talisaspire.com/works/4d101f63c10a6',"c"=>"http://talisaspire.com/","type"=>'v_counts')));
+        $actualView = \Tripod\Mongo\Config::getInstance()->getCollectionForView('tripod_php_testing', 'v_counts')->findOne(array('_id'=>array("r"=>'http://talisaspire.com/works/4d101f63c10a6',"c"=>"http://talisaspire.com/","type"=>'v_counts')));
 //        var_dump($actualView); die;


remove the var_dumps

… Jobs and Composites

Conflicts: scripts/mongo/createTables.php src/mongo/Config.class.php src/mongo/delegates/TransactionLog.class.php src/mongo/delegates/Views.class.php test/unit/mongo/MongoTripodTablesTest.php

rsinger · 2015-05-29T10:20:27Z

From the feedback, I have:

Renamed Tripod to Driver, TripodBase to DriverBase, and ITripod to IDriver
Added \Tripod\Mongo\Jobs and \Tripod\Mongo\Composites and moved the related classes into them
Added TimerException to \Tripod\Exceptions

Any other comments before I merge into the feature branch?

scaleupcto · 2015-05-29T10:53:40Z

We're renaming the interface to IDriver too?

rsinger · 2015-05-29T10:56:09Z

Should we not? It's the interface for a Tripod Driver, right?

scaleupcto · 2015-05-29T11:28:51Z

I guess. I'm not going to argue about this too much given the chainable API #23 should be the right way to use tripod in the future.

rsinger · 2015-05-29T11:33:53Z

Agreed. I am certainly not married to the change: I just thought it seemed more consistent. I could be just as happy (well, ambivalent, really) with either name.

70%: Introduce namespaces

rsinger added 7 commits May 19, 2015 00:38

Initial pass at namespaces + lots of broken window repairs

e97ed41

Tests are passing

bb62490

Migrate the scripts; update IndexUtils to actually work :O

7c0e336

Space. The final frontier

38779f9

More code cleanup

066d8a4

More code cleanup

44d00d5

Docblock

8121290

rsinger mentioned this pull request May 20, 2015

90%: Treat tables, views, and search differently #57

Merged

Delete the indexes for views and tables if $reindex=true

bd41194

Rename ITripodSearchProvider to ISearchProvider

054d370

malcyL reviewed May 22, 2015
View reviewed changes

rgubby reviewed May 28, 2015
View reviewed changes

Stat->start

f7ee2fa

rgubby reviewed May 28, 2015
View reviewed changes

rsinger added 5 commits May 28, 2015 14:04

PR feedback

34bfd5b

Rename MongoTripod to \Tripod\Mongo\Driver add namespace packages for…

7baa8ec

… Jobs and Composites

Merge branch 'decouple-invalidation-from-save' into introduce-namespaces

9354f22

Conflicts: scripts/mongo/createTables.php src/mongo/Config.class.php src/mongo/delegates/TransactionLog.class.php src/mongo/delegates/Views.class.php test/unit/mongo/MongoTripodTablesTest.php

Errant line from merge conflict

7240fb3

Move IComposite into Composites package; docs

e5348be

rsinger changed the title ~~10%: Introduce namespaces~~ 70%: Introduce namespaces May 29, 2015

ITripod->IDriver

d629f33

Use Tripod Exception rather than standard Exception

fa1cfb1

rsinger added a commit that referenced this pull request May 29, 2015

Merge pull request #56 from talis/introduce-namespaces

856286f

70%: Introduce namespaces

rsinger merged commit 856286f into decouple-invalidation-from-save May 29, 2015

rsinger deleted the introduce-namespaces branch May 29, 2015 12:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

70%: Introduce namespaces #56

70%: Introduce namespaces #56

rsinger commented May 19, 2015

scaleupcto commented May 22, 2015

scaleupcto commented May 22, 2015

rsinger commented May 22, 2015

scaleupcto commented May 22, 2015

scaleupcto commented May 22, 2015

rsinger commented May 22, 2015

scaleupcto commented May 22, 2015

rsinger commented May 22, 2015

rsinger commented May 22, 2015

rsinger commented May 22, 2015

rsinger commented May 22, 2015

scaleupcto commented May 22, 2015

rsinger commented May 22, 2015

scaleupcto commented May 22, 2015

rsinger commented May 22, 2015

scaleupcto commented May 22, 2015

rsinger commented May 22, 2015

scaleupcto commented May 22, 2015

rsinger commented May 22, 2015

malcyL May 22, 2015

rgubby May 28, 2015

rsinger May 28, 2015

rgubby May 28, 2015

rsinger commented May 29, 2015

scaleupcto commented May 29, 2015

rsinger commented May 29, 2015

scaleupcto commented May 29, 2015

rsinger commented May 29, 2015

70%: Introduce namespaces #56

70%: Introduce namespaces #56

Conversation

rsinger commented May 19, 2015

scaleupcto commented May 22, 2015

scaleupcto commented May 22, 2015

rsinger commented May 22, 2015

scaleupcto commented May 22, 2015

scaleupcto commented May 22, 2015

rsinger commented May 22, 2015

scaleupcto commented May 22, 2015

rsinger commented May 22, 2015

rsinger commented May 22, 2015

rsinger commented May 22, 2015

rsinger commented May 22, 2015

scaleupcto commented May 22, 2015

rsinger commented May 22, 2015

scaleupcto commented May 22, 2015

rsinger commented May 22, 2015

scaleupcto commented May 22, 2015

rsinger commented May 22, 2015

scaleupcto commented May 22, 2015

rsinger commented May 22, 2015

malcyL May 22, 2015

Choose a reason for hiding this comment

rgubby May 28, 2015

Choose a reason for hiding this comment

rsinger May 28, 2015

Choose a reason for hiding this comment

rgubby May 28, 2015

Choose a reason for hiding this comment

rsinger commented May 29, 2015

scaleupcto commented May 29, 2015

rsinger commented May 29, 2015

scaleupcto commented May 29, 2015

rsinger commented May 29, 2015