acmpesuecc / Onyx

MIT License
3 stars 2 forks source link

`RemoveEdge` incorrectly throws error in specific case #1

Open Smuzzy-waiii opened 4 weeks ago

Smuzzy-waiii commented 4 weeks ago

The RemoveEgde function throws "Key not found" error while trying to remove an edge a->b if there exist no edges with a as their source. This is happening due to the following code:

    item, err := txn.Get([]byte(from))
    if err != nil {
        return err
    }

where we are not checking if the error returned by badger is ErrKeyNotFound as is being done for the AddEdge function


Modify the RemoveEdge function to return (bool, error) instead of just a error where bool signifies whether the Remove actually removed an edge or whether it silently failed because the edge didn't exist. Have the function return:

Note that you will have to check for the edge not existing is 2 places: while getting the edgelist for the src node from badger, and then again in the edgelist before calling delete(dstNodes, to)

You are also required to add a unit test to cover this case

The above change is a breaking change to the RemoveEdge API. Make sure to edit all the calls to RemoveEdge in lib_test.go to reflect that

psst, remember to update the usage guide in the README too 👀

JP-sDEV commented 3 weeks ago

Hi @Smuzzy-waiii, could I be assigned this bug?

Smuzzy-waiii commented 3 weeks ago

@JP-sDEV This repo is being prepped for a university club event called Hacknight where participants get to solve issues on FOSS repo's to get points. You can find out more about hacknight on the ACM PESU ECC instagram here. The event is from 4PM IST on Fri, 18th Oct to 12PM IST on Sat, 19th Oct.

If this issue is still unresolved at the end of the event, I would love to assign it to you

JP-sDEV commented 3 weeks ago

No worries, thanks!

velukutty2194 commented 3 weeks ago

could i be assigned?? @Smuzzy-waiii

Smuzzy-waiii commented 3 weeks ago

!assign @velukutty2194

Smuzzy-waiii commented 3 weeks ago

The bot will deassign you in 45mins. If you need more time, show your progress and I can extend the time. Please feel free to text/call me at 8618950413

bunsamosa-bot[bot] commented 3 weeks ago

Hey @Smuzzy-waiii! The timer for the @velukutty2194 to work on the issue has finished, deassign and assign a new contributor or extend the current timer. Contact maintainer leads if inactive @DedLad @polarhive @achyuthcodes30

Smuzzy-waiii commented 3 weeks ago

@velukutty2194 Are you still working on this?

Smuzzy-waiii commented 3 weeks ago

!extend

Smuzzy-waiii commented 3 weeks ago

!deassign

Smuzzy-waiii commented 3 weeks ago

@JP-sDEV Are you still interested in working on this? I can assign you

JP-sDEV commented 3 weeks ago

@Smuzzy-waiii Yes, I am still interested! I will start working on it on Monday, if assigned.

Smuzzy-waiii commented 3 weeks ago

@JP-sDEV Yes np go for it! 😃