Details

    • Technical task
    • Status: Closed
    • Major
    • Resolution: Fixed
    • None
    • 1.5.13, 1.6.0
    • core
    • None

    Description

      Need to provide an API MountInfoProvider which would be responsible for mapping a given path to logical store name

      Attachments

        1. mounts-v2.patch
          26 kB
          Alex Deparvu
        2. mounts.patch
          16 kB
          Alex Deparvu
        3. ismounted.patch
          2 kB
          Alex Deparvu
        4. OAK-3404-v1.patch
          29 kB
          Chetan Mehrotra

        Activity

          MountInfoProvider defines the API for getting mount info by other Oak parts

          Initial implementation done with 648f207929

          chetanm Chetan Mehrotra added a comment - MountInfoProvider defines the API for getting mount info by other Oak parts Initial implementation done with 648f207929

          patch for new SPI interface MountInfoProvider and related implementation

          chetanm Chetan Mehrotra added a comment - patch for new SPI interface MountInfoProvider and related implementation

          Don't have a deep knowledge on what was the topic but by looking at the patch it, overall sounds good to me. It's definitely missing javadoc describing what the interface or the class is suppose to do. Mainly all of them are without javadoc.

          edivad Davide Giannella added a comment - Don't have a deep knowledge on what was the topic but by looking at the patch it, overall sounds good to me. It's definitely missing javadoc describing what the interface or the class is suppose to do. Mainly all of them are without javadoc.

          Thanks Davide. Main API class is MountInfoProvider which has some javadoc. Would improve that further

          chetanm Chetan Mehrotra added a comment - Thanks Davide. Main API class is MountInfoProvider which has some javadoc. Would improve that further
          stillalex Alex Deparvu added a comment -

          For work on OAK-3403, I'd like to have a way to determine if a Mount includes a specific path. The current MountInfoProvider only provides mapping the other way around (given a path, I can get the mount).

          stillalex Alex Deparvu added a comment - For work on OAK-3403 , I'd like to have a way to determine if a Mount includes a specific path. The current MountInfoProvider only provides mapping the other way around (given a path, I can get the mount).
          stillalex Alex Deparvu added a comment -

          proposed patch for new method to check if a path is under a spefic mount. I'm having some trouble distinguishing between Mount and MountInfo roles, so to minimize changes I added this to the MountInfoProvider, if this is not the good place, feel free to change it.

          stillalex Alex Deparvu added a comment - proposed patch for new method to check if a path is under a spefic mount. I'm having some trouble distinguishing between Mount and MountInfo roles, so to minimize changes I added this to the MountInfoProvider , if this is not the good place, feel free to change it.

          yeah so far Mount was more like value object (hold data) and not logic. Looking at intended usage I think it would be better to turn it into interface and have MountInfo implement it. And then have this new API added to that

          chetanm Chetan Mehrotra added a comment - yeah so far Mount was more like value object (hold data) and not logic. Looking at intended usage I think it would be better to turn it into interface and have MountInfo implement it. And then have this new API added to that
          stillalex Alex Deparvu added a comment -

          proposed patch. chetanm feedback appreciated!

          stillalex Alex Deparvu added a comment - proposed patch. chetanm feedback appreciated!
          chetanm Chetan Mehrotra added a comment - - edited

          Looks fine for now. Going forward I would refactor the API and make Mount an interface and move isMounted to Mount. For now we can go with this change so that property index work is not blocked

          Scratch that missed seeing the other patch. As discussed one thing we need to manage is how this isMounted is implemented for default mount. As for that it needs to consult other mounts to determine if anyone claims that path, if not then it part of default mount. So we would need to do away with the DEFAULT Mount as a constant and instead have a method MountInfoProvider#getDefaultMount

          chetanm Chetan Mehrotra added a comment - - edited Looks fine for now. Going forward I would refactor the API and make Mount an interface and move isMounted to Mount. For now we can go with this change so that property index work is not blocked Scratch that missed seeing the other patch. As discussed one thing we need to manage is how this isMounted is implemented for default mount. As for that it needs to consult other mounts to determine if anyone claims that path, if not then it part of default mount. So we would need to do away with the DEFAULT Mount as a constant and instead have a method MountInfoProvider#getDefaultMount
          stillalex Alex Deparvu added a comment -

          attaching a new iteration of the mounts patch, based on previous discussions.

          stillalex Alex Deparvu added a comment - attaching a new iteration of the mounts patch, based on previous discussions.

          +1. Just a minor note

          +    @Override
          +    public boolean equals(Object obj) {
          +        if (this == obj)
          +            return true;
          +        if (obj == null)
          +            return false;
          +        if (getClass() != obj.getClass())
          +            return false;
          +        MountInfo other = (MountInfo) obj;
          +        if (name == null) {
          +            if (other.name != null)
          +                return false;
          +        } else if (!name.equals(other.name))
          +            return false;
          +        return true;
               }
          

          name cannot be null so equals can be simplified.

          Thanks for taking care of this!

          chetanm Chetan Mehrotra added a comment - +1. Just a minor note + @Override + public boolean equals( Object obj) { + if ( this == obj) + return true ; + if (obj == null ) + return false ; + if (getClass() != obj.getClass()) + return false ; + MountInfo other = (MountInfo) obj; + if (name == null ) { + if (other.name != null ) + return false ; + } else if (!name.equals(other.name)) + return false ; + return true ; } name cannot be null so equals can be simplified. Thanks for taking care of this!
          stillalex Alex Deparvu added a comment -

          makes sense, I had Eclipse generate this for me

          stillalex Alex Deparvu added a comment - makes sense, I had Eclipse generate this for me
          stillalex Alex Deparvu added a comment -

          Thanks for the feedback! I applied the mount patch with http://svn.apache.org/viewvc?rev=1752400&view=rev

          stillalex Alex Deparvu added a comment - Thanks for the feedback! I applied the mount patch with http://svn.apache.org/viewvc?rev=1752400&view=rev

          All planned work done for this api as per current requirement. Would open specific issue going forward on need basis

          chetanm Chetan Mehrotra added a comment - All planned work done for this api as per current requirement. Would open specific issue going forward on need basis

          Bulk close for 1.5.13

          edivad Davide Giannella added a comment - Bulk close for 1.5.13

          People

            chetanm Chetan Mehrotra
            chetanm Chetan Mehrotra
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: