petersirka / node-filestorage

Storage for storing uploaded files in node.js
MIT License
72 stars 12 forks source link

Should have unique no-reassign id #22

Closed vinayk047 closed 7 years ago

vinayk047 commented 7 years ago

It would be very nice if it does not re-assign removed id to future file;

To produce the scenario: -> 1. store file -> 2. remove file stored in step 1 -> 3. store any file

In step 3 it gives me same file id as in step 1. It would be really nice if it does not re-assign same id to any file stored in future.

petersirka commented 7 years ago

Hi @vinaykharecha, I'll try to fix it this week! Thank you!

vinayk047 commented 7 years ago

Thank you @petersirka , btw your library is awesome; i really appreciate the work you have done on this.

PS: would you please let me know whenever you merge above changes in master?

petersirka commented 7 years ago

Hi @vinaykharecha, I have updated the whole module. Please download it and test it from GitHub 👍

vinayk047 commented 7 years ago

Hi @petersirka.

I think we got confused here.

So, in v1.5.0 you did not had re-assign id logic but it still had re-assign id once that means it is bug. let me try to find out exact steps to reproduce it and mean while i think you should rollback free id logic (if you feel it is wrong approach).

Thank you!

petersirka commented 7 years ago

My bad. I have added options for re-assign mode.

My test case:

var id = filestorage.insert('test.txt', new Buffer('Hello World 2', 'utf8'));
setTimeout(function() {
    console.log(id);
    filestorage.remove(id);
    setTimeout(function() {
        id = filestorage.insert('test.txt', new Buffer('Hello World 2', 'utf8'));
        console.log(id);
    }, 100);
}, 100);
1
2
[Finished in 0.3s]

Directory: image

Can you write me a small example?

vinayk047 commented 7 years ago

@petersirka , yes adding option is much better.

I'll give you small example; but mostly i got issue in cluster mode. So my question is it okay to use this module in cluster mode? or this module should not be used in cluster mode?

petersirka commented 7 years ago

Yes, this module can't be used in cluster. It's a bit complicated. I can do it for Total.js framework only.

vinayk047 commented 7 years ago

@petersirka , Yup, getting duplicate issue in cluster mode only

i.e.

setInterval(function () { 
    storage.insert('test.txt', new Buffer('Hello World 2', 'utf8'), null, function (err, id) {
        if (err) {
            console.log(err);
        } else {
            console.log("stored " + id);
            storage.remove(id, function (err) {
                if (err) {
                    console.log(err);
                } else {
                    console.log(id + " removed");
                }
            });
        }
    }); }, 300);

Output with same id's: nodestorage_issue

Thanks for your valuable time; you can close this issue and perhaps you can mention about cluster mode in REDME.md (If you think that's right)

Thank you!