Added support for decrement

[jeje]

Potential pull-request for #10.

[jeje]

Here is the modified test in counterdb.test.js :

    describe('counters', function() {
      let address

      it('creates and opens a database', async () => {
        const db = await orbitdb1.counter('counter database')
        assert.notEqual(db, null)
        assert.equal(db.type, 'counter')
        assert.equal(db.dbname, 'counter database')
      })

      it('value is zero when it\'s a fresh database', async () => {
        const db = await orbitdb1.counter('counter database')
        assert.equal(db.value, 0)
      })

      it('increases a counter value', async () => {
        const counter = await orbitdb1.counter('counter test inc', { path: dbPath1 })
        await mapSeries([13, 1], (f) => counter.inc(f))
        assert.equal(counter.value, 14)
        await counter.close()
      })

      it('decreases a counter value', async () => {
        const counter = await orbitdb1.counter('counter test dec', { path: dbPath1 })
        address = counter.address.toString()
        assert.equal(counter.value, 0)
        await mapSeries([13, 1], (f) => counter.inc(f))
        await mapSeries([8, 1], (f) => counter.dec(f))
        assert.equal(counter.value, 5)
        await counter.close()
      })

      it('opens a saved counter', async () => {
        const counter = await orbitdb1.counter(address, { path: dbPath1 })
        await counter.load()
        assert.equal(counter.value, 5)
        await counter.close()
      })

      it('syncs counters', async () => {
        let options = {
          // Set write access for both clients
          write: [
            orbitdb1.key.getPublic('hex'),
            orbitdb2.key.getPublic('hex')
          ],
        }

        const numbers = [[13, 10], [2, 5]]
        const increaseCounter = (counterDB, i) => mapSeries(numbers[i], n => counterDB.inc(n))

        // Create a new counter database in the first client
        options = Object.assign({}, options, { path: dbPath1 })
        const counter1 = await orbitdb1.counter(new Date().getTime().toString(), options)
        // Open the database in the second client
        options = Object.assign({}, options, { path: dbPath2, sync: true })
        const counter2 = await orbitdb2.counter(counter1.address.toString(), options)

        // Wait for peers to connect first
        await waitForPeers(ipfs1, [orbitdb2.id], counter1.address.toString())
        await waitForPeers(ipfs2, [orbitdb1.id], counter1.address.toString())

        // Increase the counters sequentially
        await mapSeries([counter1, counter2], increaseCounter)

        return new Promise(resolve => {
          // Wait for a while to make sure db's have been synced
          setTimeout(() => {
            assert.equal(counter1.value, 30)
            assert.equal(counter2.value, 30)
            resolve()
          }, 1000)
        })
      })
    })

[jeje]

When running this test in orbitdb module, I'm having warnings though that I don't understand yet:

$ mocha --grep 'orbit-db - Counters' 

  orbit-db - Counters (js-ipfs)
Swarm listening on /ip4/127.0.0.1/tcp/62546/ipfs/QmPUCW3uH9v1BJVxYACzw1podtSyff7aq75V8ATVP1m5rr
Swarm listening on /ip4/10.20.0.5/tcp/62546/ipfs/QmPUCW3uH9v1BJVxYACzw1podtSyff7aq75V8ATVP1m5rr
Swarm listening on /ip4/127.0.0.1/tcp/62551/ipfs/QmZ5HyvKG8MBGa6PavbtmUN8seJjW2KSpufx8h22krry1V
Swarm listening on /ip4/10.20.0.5/tcp/62551/ipfs/QmZ5HyvKG8MBGa6PavbtmUN8seJjW2KSpufx8h22krry1V
    counters
(node:59494) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.
      ✓ creates and opens a database
      ✓ value is zero when it's a fresh database
      ✓ increases a counter value
      ✓ decreases a counter value
      ✓ opens a saved counter
"WARNING! Head hash didn't match the contents
"WARNING! Head hash didn't match the contents
"WARNING! Head hash didn't match the contents
      ✓ syncs counters (2778ms)


  6 passing (8s)

[jeje]

I can't make the example HTML page work as well.
I tried rebuiling orbitdb.js and orbit-min.js but I ended up with errors.

I fixed the Path.join issue by changing the username variable to this:
const username = '' + new Date().getTime()

But now I have another error in my web console.
Not sure what to do/what I did wrong.

[shamb0t]

createCounter should return a PNCounter, is there a reason to not keep it as this._index.merge(e)?

[jeje]

I don't fully understand why, but when I do I end up with this error:

TypeError: Cannot convert undefined or null to object
    at Function.entries (<anonymous>)
    at GCounter.merge (/Users/jerome.bernard/Documents/Personnel/Development/orbit-db-counterstore/node_modules/crdts/src/G-Counter.js:39:12)
    at PNCounter.merge (/Users/jerome.bernard/Documents/Personnel/Development/orbit-db-counterstore/node_modules/crdts/src/PN-Counter.js:25:12)
    at mergeToIndex (/Users/jerome.bernard/Documents/Personnel/Development/orbit-db-counterstore/src/CounterIndex.js:17:45)
    at Array.forEach (<anonymous>)
    at CounterIndex.updateIndex (/Users/jerome.bernard/Documents/Personnel/Development/orbit-db-counterstore/src/CounterIndex.js:20:10)
    at CounterStore._updateIndex (/Users/jerome.bernard/Documents/Personnel/Development/orbit-db-counterstore/node_modules/orbit-db-store/src/Store.js:415:23)
    at Replicator.onLoadCompleted (/Users/jerome.bernard/Documents/Personnel/Development/orbit-db-counterstore/node_modules/orbit-db-store/src/Store.js:108:22)

I remember having seen at one point some mismatch in counters vs _counters, but I'm not sure if it's for that specific change.

Anyway, the opens a saved counter test fails when reverting this change.

[shamb0t]

Yes you're right it was due to toJSON method not serializing the counters correctly. This should be fixed now (here) and we can leave the line as this._index.merge(e)

[shamb0t]

Thanks for highlighting this! static from in PNCounter.js should be changed to static from (json) { return new PNCounter(json.id, new GCounter(json.id, json.p.counters), new GCounter(json.id, json.n.counters)) }

If the above is changed, this._index.get().p should return a GCounter, that way we can have this be const counter = new Counter(this.uid, this._index.get().p, this._index.get().n), what do you think?

[jeje]

Definitely a better solution as the CRDT lib may be used for something different than OrbitDB libs.

[shamb0t]

I've changed the constructor in PNCounter to expect objects instead of instances of GCounter.
This way this line can be const counter = new PNCounter(this.uid, this._index.get().p._counters, this.index.get().n._counters), what do you think?

[shamb0t]

See comment above

[jeje]

Same remark on my side as well :)

[shamb0t]

Thank you @jeje, this looks great! I've left a just a few comments

Re: the browser example, I think the library is out of date and isn't creating the keystore in a browser-friendly manner. Mind if we take care of that in a separate PR? As for the warnings, I believe those are from the ordering of property declarations in Entry but this should be fixed in the latest update.

Thanks for adding tests! Care to add those as a PR too?

[jeje]

Thank you @jeje, this looks great! I've left a just a few comments

Sorry for coming back to you that late and thanks for your review!

Re: the browser example, I think the library is out of date and isn't creating the keystore in a browser-friendly manner. Mind if we take care of that in a separate PR?

No problem. I tried to enrich the example as well but failed.

As for the warnings, I believe those are from the ordering of property declarations in Entry but this should be fixed in the latest update.

Great. Should I do something on my side or wait for this update?

Thanks for adding tests! Care to add those as a PR too?

You mean as a PR to the orbit-db lib, I guess?

[jeje]

As a side note, it would be easier to be able to test this module without having to use a forked version of the orbit-db one. I was about to take that route but when I saw how much code I'd have to duplicate (IPFS setup) from the orbit-db I stopped.
Eventually a module dedicated to ease writing such kind of tests may make sense.

Otherwise some unit tests may help a bit (with mocks) but won't be enough, I guess.

[jeje]

@shamb0t any feedback on my last questions?

[shamb0t]

Apologies for my very late response @jeje! Thank you for revisiting this! I left a couple more comments about some changes to PNCounter that I think make this easier now

Great. Should I do something on my side or wait for this update?

The warnings should stop with that update so no need for anything from your end.

Thanks for adding tests! Care to add those as a PR too?

You mean as a PR to the orbit-db lib, I guess?

Yes, I mean't as a PR to orbit-db :)

As a side note, it would be easier to be able to test this module without having to use a forked version of the orbit-db one. I was about to take that route but when I saw how much code I'd have to duplicate (IPFS setup) from the orbit-db I stopped.
Eventually a module dedicated to ease writing such kind of tests may make sense.

Otherwise some unit tests may help a bit (with mocks) but won't be enough, I guess.

Do you need to duplicate the IPFS set up code? I'm currently running npm link in my local orbit-db-counterstore directory and running the test in my local orbit-db directory with:

npm link orbit-db-counterstore and then mocha test/counterdb.test.js

This way your local orbit-db repo will point to your local counterstore repo and you won't need to duplicate code (but do need a local orbit-db repo), is that helpful? Lmk if I misunderstood your question