[jsword-devel] NPE in RawFileBackend#getCardinality

[Chris Burrell]

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/802b6568/attachment.html>