Uploaded image for project: 'OpenJPA'
  1. OpenJPA
  2. OPENJPA-1563

Better parameter validation on StoreCache.pinAll() method

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.0-beta2, 2.0.0
    • Fix Version/s: 2.1.0
    • Component/s: datacache
    • Labels:
      None

      Description

      Validation of the second parameter on StoreCache.pinAll could be better. Currently, we assume that the list of oids provided are valid ids for the Class instances being cached. But, if the collection of Objects passed in are not actual ids, then we end up reserving spots in the cache that never get utilized.

      /**

      • Pin the data for the given oids to the cache.
        */
        public void pinAll(Class cls, Object... oids);

      Since the Class type is also passed in, we should be able to validate that the oids passed in are valid. At a minimum, checking if they are oids in the first place would be a good catch. Currently, we do nothing.

        Activity

        Hide
        mikedd Michael Dick added a comment -

        Closing issues which have been resolved for some time. If the problem persists, please reopen.

        Show
        mikedd Michael Dick added a comment - Closing issues which have been resolved for some time. If the problem persists, please reopen.
        Hide
        curtisr7 Rick Curtis added a comment -

        Nice catch Milosz.

        Fixed the problem and modified an existing test case to hit this method.

        Show
        curtisr7 Rick Curtis added a comment - Nice catch Milosz. Fixed the problem and modified an existing test case to hit this method.
        Hide
        milosz Milosz Tylenda added a comment -

        Hi Rick, I am afraid this will end up in StackOverlfowError

        + public static Collection<Object> toOpenJPAObjectIds(ClassMetaData meta, Collection<Object> oids) {
        + return toOpenJPAObjectIds(meta, oids);

        Show
        milosz Milosz Tylenda added a comment - Hi Rick, I am afraid this will end up in StackOverlfowError + public static Collection<Object> toOpenJPAObjectIds(ClassMetaData meta, Collection<Object> oids) { + return toOpenJPAObjectIds(meta, oids);
        Hide
        curtisr7 Rick Curtis added a comment -

        Comitted rev 940077 to trunk.

        Show
        curtisr7 Rick Curtis added a comment - Comitted rev 940077 to trunk.
        Hide
        curtisr7 Rick Curtis added a comment -

        One thing I want to point out is that JPAFacadeHelper.toOpenJPAObjectId(..) will now throw a RuntimeException. This could cause a behavior change for any users currently using the java.persistence.Cache interface improperly.

        For example, if they call javax.persistence.Cache.evict(Class, oid) and oid isn't a valid oid for the provided Class, we will now throw a UserException.

        Show
        curtisr7 Rick Curtis added a comment - One thing I want to point out is that JPAFacadeHelper.toOpenJPAObjectId(..) will now throw a RuntimeException. This could cause a behavior change for any users currently using the java.persistence.Cache interface improperly. For example, if they call javax.persistence.Cache.evict(Class, oid) and oid isn't a valid oid for the provided Class, we will now throw a UserException.

          People

          • Assignee:
            curtisr7 Rick Curtis
            Reporter:
            kwsutter Kevin Sutter
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development