Skip to content
This repository was archived by the owner on Feb 26, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/**
* Copyright (C) 2010-2014 eBusiness Information, Excilys Group
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed To in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/
package org.androidannotations.helper;

import static com.sun.codemodel.JExpr._null;

import org.androidannotations.holder.HasReceiverRegistration;
import org.androidannotations.process.ProcessHolder;

import com.sun.codemodel.JBlock;
import com.sun.codemodel.JFieldVar;

public class OrmLiteHelper {

public static void injectReleaseInDestroy(JFieldVar databaseHelperRef, HasReceiverRegistration receiverHolder, ProcessHolder.Classes classes) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think it is not really nice if we use HasReceiverRegistration, because this code has nothing to do with receivers. I suggest to reintroduce the interface that i mentioned, and use it there instead of HasReceiverRegistration.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HasLifecycleMethods?

Are you sure that the interface should have onDestroyBeforeSuperBlock method?
If not, how can I insert OpenHelperManager.release in the onDestroy method?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think that HasLifecycleMethods should not have onDestroyBeforeSuperBlock? Why?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the OrmliteDaoHandler can handle two scenarios: with an onDestory method or not.
We cannot create getOnDestroy into HasLifecycleMethods, thus will be ignore annother scenario.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EBean and @EActivity can use it well. But @EBean does not have a onDestroy method

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean we should only add onDestroyXXX in the HasLifecycleMethods?
So that it conforms to the ISP? :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't get me wrong, I just confirm that I understand

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, it would be seen as redundant to you, because basically everyting which is a HasLifecycleMethods is HasReceiverRegistration, too. But i still think this is better design. This is what i propose:

create a new HasLifecycleMethods interface:

public interface HasLifecycleMethods extends GeneratedClassHolder {
    JBlock getOnCreateAfterSuperBlock();
    JBlock getOnDestroyBeforeSuperBlock();

    JBlock getOnStartAfterSuperBlock();
    JBlock getOnStopBeforeSuperBlock();

    JBlock getOnResumeAfterSuperBlock();
    JBlock getOnPauseBeforeSuperBlock();

    JBlock getOnAttachAfterSuperBlock();
    JBlock getOnDetachBeforeSuperBlock();
}

Let HasReceiverRegistration extend HasLifecycleMethods, and remove redundant method declarations:

public interface HasReceiverRegistration extends HasLifecycleMethods {
    JExpression getContextRef();

    JFieldVar getIntentFilterField(String[] actions, String[] dataSchemas);
}

Use HasLifecycleMethods in OrmLiteHelper:

public static void injectReleaseInDestroy(JFieldVar databaseHelperRef, HasLifecycleMethods lifecycleMethodsHolder, ProcessHolder.Classes classes) { ... }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. You're right!

In fact, the most perfect solution i think is split into four interface: HasLifecycleMethods / HasStartAndStopMethods / HasAttachableMethods / HasReceiverRegistration.
There is no connection between them.

EActivityHolder and EFragmentHolder need implement: HasLifecycleMethods,HasStartAndStopMethods, HasAttachableMethods and HasReceiverRegistration
EServiceHolder need implement: HasLifecycleMethods and HasReceiverRegistration

In ReceiverHandler.registerAndUnregisterReceiver method, we can add two method with the same name.
These methods are respectively corresponding to these Interface.

However, this is a huge change. I think to do as you say.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we could split it even further, but i think my proposal will suffice
for now.

JBlock destroyBody = receiverHolder.getOnDestroyBeforeSuperBlock();

destroyBody.staticInvoke(classes.OPEN_HELPER_MANAGER, "releaseHelper");
destroyBody.assign(databaseHelperRef, _null());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,27 @@
import com.sun.codemodel.JMod;
import com.sun.codemodel.JType;
import com.sun.codemodel.JVar;

import org.androidannotations.api.SdkVersionHelper;
import org.androidannotations.helper.ActionBarSherlockHelper;
import org.androidannotations.helper.ActivityIntentBuilder;
import org.androidannotations.helper.AndroidManifest;
import org.androidannotations.helper.AnnotationHelper;
import org.androidannotations.helper.CanonicalNameConstants;
import org.androidannotations.helper.IntentBuilder;
import org.androidannotations.helper.OrmLiteHelper;
import org.androidannotations.process.ProcessHolder;

import java.util.ArrayList;
import java.util.List;

import javax.lang.model.element.Element;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.Modifier;
import javax.lang.model.element.TypeElement;
import javax.lang.model.type.TypeKind;
import javax.lang.model.type.TypeMirror;
import javax.lang.model.util.ElementFilter;
import java.util.ArrayList;
import java.util.List;

import static com.sun.codemodel.JExpr.FALSE;
import static com.sun.codemodel.JExpr.TRUE;
Expand Down Expand Up @@ -757,4 +761,11 @@ public JFieldVar getIntentFilterField(String[] actions, String[] dataSchemes) {
return receiverRegistrationHolder.getIntentFilterField(actions, dataSchemes);
}

@Override
protected JFieldVar setDatabaseHelperRef(TypeMirror databaseHelperTypeMirror) {
JFieldVar databaseHelperRef = super.setDatabaseHelperRef(databaseHelperTypeMirror);
OrmLiteHelper.injectReleaseInDestroy(databaseHelperRef, this, classes());

return databaseHelperRef;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public JFieldVar getDatabaseHelperRef(TypeMirror databaseHelperTypeMirror) {
return databaseHelperRef;
}

private JFieldVar setDatabaseHelperRef(TypeMirror databaseHelperTypeMirror) {
protected JFieldVar setDatabaseHelperRef(TypeMirror databaseHelperTypeMirror) {
JClass databaseHelperClass = refClass(databaseHelperTypeMirror.toString());
String fieldName = CaseHelper.lowerCaseFirst(databaseHelperClass.name()) + ModelConstants.GENERATION_SUFFIX;
JFieldVar databaseHelperRef = generatedClass.field(PRIVATE, databaseHelperClass, fieldName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,19 @@
import com.sun.codemodel.JMod;
import com.sun.codemodel.JTypeVar;
import com.sun.codemodel.JVar;

import org.androidannotations.annotations.EFragment;
import org.androidannotations.helper.ActionBarSherlockHelper;
import org.androidannotations.helper.AnnotationHelper;
import org.androidannotations.helper.OrmLiteHelper;
import org.androidannotations.process.ProcessHolder;

import javax.lang.model.element.TypeElement;
import java.util.ArrayList;
import java.util.List;

import javax.lang.model.element.TypeElement;
import javax.lang.model.type.TypeMirror;

import static com.sun.codemodel.JExpr.FALSE;
import static com.sun.codemodel.JExpr.TRUE;
import static com.sun.codemodel.JExpr._new;
Expand Down Expand Up @@ -540,4 +544,12 @@ public JBlock getOnDetachBeforeSuperBlock() {
}
return onDetachBeforeSuperBlock;
}

@Override
protected JFieldVar setDatabaseHelperRef(TypeMirror databaseHelperTypeMirror) {
JFieldVar databaseHelperRef = super.setDatabaseHelperRef(databaseHelperTypeMirror);
OrmLiteHelper.injectReleaseInDestroy(databaseHelperRef, this, classes());

return databaseHelperRef;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@
import com.sun.codemodel.*;
import org.androidannotations.helper.AndroidManifest;
import org.androidannotations.helper.IntentBuilder;
import org.androidannotations.helper.OrmLiteHelper;
import org.androidannotations.helper.ServiceIntentBuilder;
import org.androidannotations.process.ProcessHolder;

import javax.lang.model.element.TypeElement;
import javax.lang.model.type.TypeMirror;

import static com.sun.codemodel.JExpr._this;
import static com.sun.codemodel.JMod.PRIVATE;
Expand Down Expand Up @@ -130,4 +132,12 @@ public JBlock getOnAttachAfterSuperBlock() {
public JBlock getOnDetachBeforeSuperBlock() {
return receiverRegistrationHolder.getOnDetachBeforeSuperBlock();
}

@Override
protected JFieldVar setDatabaseHelperRef(TypeMirror databaseHelperTypeMirror) {
JFieldVar databaseHelperRef = super.setDatabaseHelperRef(databaseHelperTypeMirror);
OrmLiteHelper.injectReleaseInDestroy(databaseHelperRef, this, classes());

return databaseHelperRef;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/**
* Copyright (C) 2010-2014 eBusiness Information, Excilys Group
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed To in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/
package org.androidannotations.test15.ormlite;

import org.androidannotations.annotations.EActivity;
import org.androidannotations.annotations.OrmLiteDao;

import android.app.Activity;

@EActivity
public class OrmLiteOnDestroyActivity extends Activity {

@OrmLiteDao(helper = DatabaseHelper.class)
UserDao userDao;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/**
* Copyright (C) 2010-2014 eBusiness Information, Excilys Group
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed To in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/
package org.androidannotations.test15.ormlite;

import static org.fest.assertions.api.Assertions.assertThat;
import static org.fest.reflect.core.Reflection.field;
import static org.fest.reflect.core.Reflection.staticField;

import org.fest.reflect.field.Invoker;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.robolectric.Robolectric;
import org.robolectric.RobolectricTestRunner;
import org.robolectric.util.ActivityController;

import com.j256.ormlite.android.apptools.OpenHelperManager;
import com.j256.ormlite.android.apptools.OrmLiteSqliteOpenHelper;

@RunWith(RobolectricTestRunner.class)
public class OrmLiteOnDestroyTest {

private ActivityController<OrmLiteOnDestroyActivity_> controller;
private Invoker<Integer> instanceCountInvoker;

@Before
public void setup() throws NoSuchFieldException, IllegalAccessException {
staticField("helper").ofType(OrmLiteSqliteOpenHelper.class).in(OpenHelperManager.class).set(null);
staticField("helperClass").ofType(Class.class).in(OpenHelperManager.class).set(null);
staticField("wasClosed").ofType(boolean.class).in(OpenHelperManager.class).set(false);

instanceCountInvoker = staticField("instanceCount").ofType(int.class).in(OpenHelperManager.class);
instanceCountInvoker.set(0);

controller = Robolectric.buildActivity(OrmLiteOnDestroyActivity_.class).create();
}

@Test
public void helper_is_destroyed() throws Exception {
controller.destroy();

Object databaseHelper_ = field("databaseHelper_").ofType(OrmLiteSqliteOpenHelper.class).in(controller.get()).get();
assertThat(databaseHelper_).isNull();
assertThat(instanceCountInvoker.get()).isEqualTo(0);
}
}