brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From alasdairhodge <...@git.apache.org>
Subject [GitHub] incubator-brooklyn pull request: Refactor rebind code, and tidy
Date Wed, 12 Nov 2014 17:23:45 GMT
Github user alasdairhodge commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/309#discussion_r20234337
  
    --- Diff: core/src/test/java/brooklyn/entity/rebind/RebindTestFixture.java ---
    @@ -131,95 +124,75 @@ public void tearDown() throws Exception {
             origManagementContext = null;
         }
     
    -    /**
    -     * Dumps out the persisted mementos that are at the given directory.
    -     * 
    -     * Binds to the persisted state (as a "hot standby") to load the raw data (as strings),
and to write out the
    -     * entity, location, policy, enricher, feed and catalog-item data.
    -     * 
    -     * @param dir The directory containing the persisted state (e.g. {@link #mementoDir}
or {@link #mementoDirBackup})
    -     */
    -    protected void dumpMementoDir(File dir) {
    -        LocalManagementContextForTests mgmt = new LocalManagementContextForTests(BrooklynProperties.Factory.newEmpty());
    -        FileBasedObjectStore store = null;
    -        BrooklynMementoPersisterToObjectStore persister = null;
    -        try {
    -            store = new FileBasedObjectStore(dir);
    -            store.injectManagementContext(mgmt);
    -            store.prepareForSharedUse(PersistMode.AUTO, HighAvailabilityMode.HOT_STANDBY);
    -            persister = new BrooklynMementoPersisterToObjectStore(store, BrooklynProperties.Factory.newEmpty(),
classLoader);
    -            BrooklynMementoRawData data = persister.loadMementoRawData(RebindExceptionHandlerImpl.builder().build());
    -            List<BrooklynObjectType> types = ImmutableList.of(BrooklynObjectType.ENTITY,
BrooklynObjectType.LOCATION, 
    -                    BrooklynObjectType.POLICY, BrooklynObjectType.ENRICHER, BrooklynObjectType.FEED,

    -                    BrooklynObjectType.CATALOG_ITEM);
    -            for (BrooklynObjectType type : types) {
    -                LOG.info(type+" ("+data.getObjectsOfType(type).keySet()+"):");
    -                for (Map.Entry<String, String> entry : data.getObjectsOfType(type).entrySet())
{
    -                    LOG.info("\t"+type+" "+entry.getKey()+": "+entry.getValue());
    -                }
    -            }
    -        } finally {
    -            if (persister != null) persister.stop(false);
    -            if (store != null) store.close();
    -            mgmt.terminate();
    -        }
    -    }
    -    
         /** rebinds, and sets newApp */
         protected T rebind() throws Exception {
    -        return rebind(true);
    +        return rebind(RebindOptions.create());
         }
     
         /**
    -     * TODO We should (probably?!) change everywhere from asserting that they are serializable.

    -     * They only need to be xstream-serializable, which does not require `implements
Serializable`. 
    +     * Checking serializable is overly strict.
    +     * State only needs to be xstream-serializable, which does not require `implements
Serializable`. 
          * Also, the xstream serializer has some special hooks that replaces an entity reference
with 
    -     * a marker for that entity, etc. Suggest we change the default {@link #rebind()}
to use 
    -     * {@code checkSerializable==false}, and deprecate this + the other overloaded methods?
    +     * a marker for that entity, etc.
    +     * 
    +     * @deprecated since 0.7.0; use {@link #rebind()} or {@link #rebind(RebindOptions)})
          */
    +    @Deprecated
         protected T rebind(boolean checkSerializable) throws Exception {
    -        // TODO What are sensible defaults?!
    -        return rebind(checkSerializable, false);
    +        return rebind(RebindOptions.create().checkSerializable(checkSerializable));
         }
    -
    -    @SuppressWarnings("unchecked")
    +    
    +    /**
    +     * Checking serializable is overly strict.
    +     * State only needs to be xstream-serializable, which does not require `implements
Serializable`. 
    +     * Also, the xstream serializer has some special hooks that replaces an entity reference
with 
    +     * a marker for that entity, etc.
    +     * 
    +     * @deprecated since 0.7.0; use {@link #rebind(RebindOptions)})
    +     */
    +    @Deprecated
         protected T rebind(boolean checkSerializable, boolean terminateOrigManagementContext)
throws Exception {
    -        return rebind(checkSerializable, terminateOrigManagementContext, (File)null);
    +        return rebind(RebindOptions.create()
    +                .checkSerializable(checkSerializable)
    +                .terminateOrigManagementContext(terminateOrigManagementContext));
         }
    -    
    -    @Beta // temporary method while debugging; Aled will refactor all of this soon!
    -    @SuppressWarnings("unchecked")
    -    protected T rebind(boolean checkSerializable, boolean terminateOrigManagementContext,
File backupDir) throws Exception {
    -        if (newApp!=null || newManagementContext!=null) throw new IllegalStateException("already
rebound");
    -        
    -        RebindTestUtils.waitForPersisted(origApp);
    -        if (checkSerializable) {
    -            RebindTestUtils.checkCurrentMementoSerializable(origApp);
    -        }
    -        if (terminateOrigManagementContext) {
    -            origManagementContext.terminate();
    -        }
    -
    -        if (backupDir != null) {
    -            FileUtil.copyDir(mementoDir, backupDir);
    -            FileUtil.setFilePermissionsTo700(backupDir);
    -        }
     
    -        newManagementContext = createNewManagementContext();
    -        newApp = (T) RebindTestUtils.rebind((LocalManagementContext)newManagementContext,
classLoader);
    -        return newApp;
    +    protected T rebind(RebindExceptionHandler exceptionHandler) throws Exception {
    +        return rebind(RebindOptions.create().exceptionHandler(exceptionHandler));
         }
     
    +    protected T rebind(ManagementContext newManagementContext, RebindExceptionHandler
exceptionHandler) throws Exception {
    +        return rebind(RebindOptions.create()
    +                .newManagementContext(newManagementContext)
    +                .exceptionHandler(exceptionHandler));
    +    }
    +    
         @SuppressWarnings("unchecked")
    -    protected T rebind(RebindExceptionHandler exceptionHandler) throws Exception {
    +    protected T rebind(RebindOptions options) throws Exception {
    +        if (newApp!=null || newManagementContext!=null) {
    +            throw new IllegalStateException("already rebound");
    +        }
    --- End diff --
    
    Whitespace around binary operators, please.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message