[jsword-devel] NPE in RawFileBackend#getCardinality

Tue Feb 12 15:05:02 MST 2013

But presumably, you could test for the existence of files named asdf*
within the containing directory?

On 12 February 2013 20:16, DM Smith <dmsmith at crosswire.org> wrote:

> Most of the locations specified in a conf are to a directory.
>
> For some it is to the prefix of the name of a data file.
>
> So there might be modules/comments/rawcom/asdf/asdf for an asdf module.
> But that is not a file or a directory. The last element of the path is a
> prefix. So the directory is modules/comments/rawcom/asdf and the files
> probably are asdf.idx and asdf.dat.
>
> So if a module has been partially uninstalled, such that the directory has
> been removed but the conf remains, then you are in trouble. The code looks
> at the last element and sure enough it is not a directory. It doesn't
> exist. Now it backs up one, and since it is not a conf that specifies the
> prefix as part of the path, you get the common directory for holding all
> files of that type.
>
> At this point, one can only test for the existence of expected files to
> find that they are not there. Then you know the module has been partially
> wacked.
>
> Note, it is ok in JSword and SWORD for the module data to remain if the
> conf is gone. If there is no conf, there is no module.
>
> Hope that helps.
>
> In Him,
> DM
>
>
> On Feb 12, 2013, at 3:02 PM, Chris Burrell <chris at burrell.me.uk> wrote:
>
> Can you explain this piece of code to me. It doesn't work very well to
> test the containing directory, as that might well be the directory holding
> all the modules of a particular type. For example, I have mak.conf, but it
> is not present in: C:\Users\Chris\Application
> Data\Sword\modules\comments\rawcom
>
> Why are we then testing that rawcom exists and if it does accepting it?
>
> SwordBookMetaData.java:
>
>         URI location = NetUtil.lengthenURI(library, datapath);
>         File bookDir = new File(location.getPath());
>         // For some modules, the last element of the DataPath
>         // is a prefix for file names.
>         if (!bookDir.isDirectory()) {
>             // Shorten it by one segment and test again.
>             lastSlash = datapath.lastIndexOf('/');
>             datapath = datapath.substring(0, lastSlash);
>             location = NetUtil.lengthenURI(library, datapath);
>             bookDir = new File(location.getPath());
>             if (!bookDir.isDirectory()) {
>                 // TRANSLATOR: This indicates that the Book is only
> partially installed.
>                 throw new MissingDataFilesException(JSMsg.gettext("The
> book is missing its data files", cet.getValue(ConfigEntryType.INITIALS)));
>             }
>         }
>
>
>
>
> On 12 February 2013 19:26, DM Smith <dmsmith at crosswire.org> wrote:
>
>> Sounds good.
>>
>> On Feb 12, 2013, at 2:18 PM, Chris Burrell <chris at burrell.me.uk> wrote:
>>
>> I agree with not coding against dodgy developers (although to be fair I'm
>> not entirely sure how the conf files have been left over).
>>
>> But, I'd like to suggest we add a line in the reading of the conf file to
>> validate that the module data directory actually exists and is non-empty.
>> If the directory is empty/doesn't exist, we remove the book from the
>> returned set of installed()
>>
>> Chris
>>
>>
>>
>> On 11 February 2013 23:23, DM Smith <dmsmith at crosswire.org> wrote:
>>
>>> Ah. I see. The JSword code (IIRC) deletes the conf first and then the
>>> files. I downloaded the module just fine and deleted it from w/in the app
>>> as well. I think we should not go out of our way for a developer that wacks
>>> parts of modules. :)
>>>
>>> That said, your question regarding the error is still worthy of solving.
>>> There are many reasons that a backend might fail and it would make sense to
>>> have robust code to handle such failures.
>>>
>>>  In Him,
>>> DM
>>>
>>>
>>>
>>> On Feb 11, 2013, at 5:59 PM, Chris Burrell <chris at burrell.me.uk> wrote:
>>>
>>> Dm
>>>
>>> The problem is that I must likely deleted the files. But not the Conf
>>> file.
>>> Just wanted to highlight npe as we probably want to avoid having one at
>>> that stage
>>> Chris
>>> On 11 Feb 2013 22:54, "DM Smith" <dmsmith at crosswire.org> wrote:
>>>
>>>>
>>>> On Feb 11, 2013, at 5:27 PM, Chris Burrell <chris at burrell.me.uk> wrote:
>>>>
>>>> > Hi
>>>> >
>>>> > On line 124, we have a potential NPE:
>>>> >
>>>> > state.getIdxRaf() can return a null value if for some reason we are
>>>> attempting to read the cardinality of a module, but the idx file for the
>>>> module is not present.
>>>> >
>>>> > In this case, I'm looking at the 2babdict module, for which I have a
>>>> conf file but no module files under modules/lexdict/rawld
>>>> >
>>>> > Obviously, this is an issue, but I wonder if the reading of the .conf
>>>> file should identify the absence of modules and therefore
>>>> > a- report early to the user that something is wrong
>>>> > b- not show-up in the list of installed() modules
>>>>
>>>> The code regarding backends is in process of change, maybe done.
>>>>
>>>> The way the code used to work:
>>>> For the list of installed books (i.e. for each conf) build an
>>>> appropriate backend, which would log an error and throw an exception if the
>>>> files for the module were not present and mark the module as unsupported.
>>>>
>>>> The problem with that model is that it is expensive on startup when
>>>> lots of modules are installed. There is a Jira issue regarding improving
>>>> this performance.
>>>>
>>>> The other problem is that it has no memory of the error and on next
>>>> startup, it does the work again.
>>>>
>>>> It seems to me that when the module is installed, that'd be a great
>>>> time to validate it fully and mark it as validated.
>>>>
>>>> In JSword we have ReporterUtil to let the user know of events like
>>>> this. It might not work in STEP as it is tied to a listener mechanism.
>>>>
>>>> It'd be good to iterate on this to get a good solution.
>>>>
>>>> I'm looking at 2babdict to see if I can identify the problem.
>>>>
>>>> In Him,
>>>>         DM
>>>>
>>>>
>>>
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.crosswire.org/pipermail/jsword-devel/attachments/20130212/c907efd0/attachment-0001.html>