You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@isis.apache.org by GitBox <gi...@apache.org> on 2022/07/14 08:02:02 UTC
[GitHub] [isis] danhaywood opened a new pull request, #1019: Isis 3002
danhaywood opened a new pull request, #1019:
URL: https://github.com/apache/isis/pull/1019
- [ ] Stringifier interface for BookmarkService, JPA id, JDO id.
- [ ] documentation for Stringifier
- [ ] rationalize packages for JDO and JPA so that the same concerns are in the same place
- [ ] commandlog integ tests
- [ ] executionlog impl +integ tests
- [ ] executionoutbox impl + integ tests
- [ ] commandreplay integ tests
- [ ] check callers of `IdentifierUtil#logicalMemberIdentifierForDeclaredMember(ObjectAction | OTOA)` - in particular `CommandDtoFactoryDefault` and `InteractionDtoFactoryDefault` - may need to use `logicalMemberIdentifierFor(InteractionHead, ObjectMember)` (which also handles case of mixins).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@isis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [isis] danhaywood commented on a diff in pull request #1019: Isis 3002
Posted by GitBox <gi...@apache.org>.
danhaywood commented on code in PR #1019:
URL: https://github.com/apache/isis/pull/1019#discussion_r926785718
##########
persistence/jpa/integration/src/main/java/org/apache/isis/persistence/jpa/integration/entity/JpaEntityFacetFactory.java:
##########
@@ -393,6 +409,12 @@ private static JpaObjectIdSerializer createJpaObjectIdSerializer(
return new ByteIdSerializer();
}
}
+ Can<BookmarkService.Stringifier> select = serviceRegistry.select(BookmarkService.Stringifier.class);
+ for (BookmarkService.Stringifier stringifier : select.toList()) {
+ if(stringifier.handles() == primaryKeyType) {
Review Comment:
It has ended up like this, indeed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@isis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [isis] danhaywood commented on a diff in pull request #1019: Isis 3002
Posted by GitBox <gi...@apache.org>.
danhaywood commented on code in PR #1019:
URL: https://github.com/apache/isis/pull/1019#discussion_r926784884
##########
extensions/core/executionlog/applib/src/main/java/org/apache/isis/extensions/executionlog/applib/util/BigDecimalUtils.java:
##########
@@ -0,0 +1,27 @@
+package org.apache.isis.extensions.executionlog.applib.util;
Review Comment:
Thanks - I've tried to do this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@isis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [isis] danhaywood commented on a diff in pull request #1019: Isis 3002
Posted by GitBox <gi...@apache.org>.
danhaywood commented on code in PR #1019:
URL: https://github.com/apache/isis/pull/1019#discussion_r926793966
##########
persistence/jdo/datanucleus/src/main/java/org/apache/isis/persistence/jdo/datanucleus/metamodel/facets/entity/JdoEntityFacet.java:
##########
@@ -80,6 +81,7 @@
@Inject private ObjectManager objectManager;
@Inject private ExceptionRecognizerService exceptionRecognizerService;
@Inject private JdoFacetContext jdoFacetContext;
+ @Inject private List<IdStringifier> idStringifiers;
Review Comment:
No longer necessary, there will always be `IdStringifier`s available.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@isis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [isis] danhaywood commented on a diff in pull request #1019: Isis 3002
Posted by GitBox <gi...@apache.org>.
danhaywood commented on code in PR #1019:
URL: https://github.com/apache/isis/pull/1019#discussion_r926786012
##########
api/applib/src/main/java/org/apache/isis/applib/services/bookmark/IdStringifier.java:
##########
@@ -0,0 +1,48 @@
+package org.apache.isis.applib.services.bookmark;
+
+import java.util.Optional;
+
+import lombok.Getter;
+
+/**
+ * SPI to allow other modules to extend the bookmarking mechanism.
+ *
+ * <p>
+ * Originally introduced to allow JPA CommandLog implementation use CommandLogEntryPK for its primary key.
+ * </p>
+ */
+public interface IdStringifier<T> {
+
+ boolean handles(Class<?> candidateClass);
+
+ Class<T> getHandledClass();
Review Comment:
Indeed ... is now supported.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@isis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [isis] danhaywood commented on a diff in pull request #1019: Isis 3002
Posted by GitBox <gi...@apache.org>.
danhaywood commented on code in PR #1019:
URL: https://github.com/apache/isis/pull/1019#discussion_r926795472
##########
core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/actions/action/invocation/IdentifierUtil.java:
##########
@@ -109,19 +120,39 @@ public Identifier memberIdentifierFor(
/**
* Whether given command corresponds to given objectMember.
* <p>
- * Is related to {@link #logicalMemberIdentifierFor(ObjectMember)}.
+ * Is related to {@link #logicalMemberIdentifierForDeclaredMember(ObjectMember)}.
*/
public boolean isCommandForMember(
final @Nullable Command command,
+ final @NonNull InteractionHead interactionHead,
final @Nullable ObjectMember objectMember) {
return command!=null
&& objectMember!=null
- && logicalMemberIdentifierFor(objectMember)
+ && logicalMemberIdentifierFor(interactionHead, objectMember)
.equals(command.getLogicalMemberIdentifier());
}
// -- HELPER
+ private String logicalMemberIdentifierFor(
+ final @NonNull InteractionHead interactionHead,
+ final ObjectMember objectMember) {
+ if (objectMember instanceof ObjectAction) {
+ ObjectAction objectAction = (ObjectAction) objectMember;
+ if (objectAction.isDeclaredOnMixin()) {
+ if (interactionHead instanceof ActionInteractionHead) {
+ ObjectAction objectActionOnMixee = ((ActionInteractionHead) interactionHead).getMetaModel();
+ ObjectSpecification specificationOfMixee = interactionHead.getOwner().getSpecification();
+ return logicalMemberIdentifierFor(specificationOfMixee, objectActionOnMixee);
+ }
+ }
+ // we fall through to the declared case, which isn't correct; but don't think it matters because
Review Comment:
I did some further clean-up, everywhere now calls the overload that passes in an `InteractionHead` and so we are abstracted from whether the identifier is for a mixin or for a regular action. (The integ tests for command log and execution log helped pick up the missing cases).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@isis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [isis] danhaywood commented on a diff in pull request #1019: Isis 3002
Posted by GitBox <gi...@apache.org>.
danhaywood commented on code in PR #1019:
URL: https://github.com/apache/isis/pull/1019#discussion_r926787409
##########
core/runtimeservices/src/main/java/org/apache/isis/core/runtimeservices/bookmarks/BookmarkServiceDefault.java:
##########
@@ -141,72 +144,47 @@ public Bookmark bookmarkForElseFail(final @Nullable Object domainObject) {
@Override
public <T> T read(final Class<T> cls, final Serializable value) {
+ if (stringifiers != null) {
+ for (IdStringifier<?> serializer : stringifiers) {
+ if (serializer.handles(cls)) {
+ return (T) serializer.parse((String)value);
+ }
+ }
+ }
+
if(Bookmark.class.equals(cls)) {
return _Casts.uncheckedCast(value);
}
if(Bookmark.class.isAssignableFrom(value.getClass())) {
- final Bookmark valueBookmark = (Bookmark) value;
- return _Casts.uncheckedCast(lookup(valueBookmark).orElse(null));
+ final Bookmark valueAsBookmark = (Bookmark) value;
+ return _Casts.uncheckedCast(lookup(valueAsBookmark).orElse(null));
}
return _Casts.uncheckedCast(value);
}
+ @SuppressWarnings("unchecked")
@Override
public Serializable write(final Object value) {
- if(isPredefinedSerializable(value.getClass())) {
- return (Serializable) value;
- } else {
- return bookmarkForElseFail(value);
+ if (stringifiers != null) {
+ for (IdStringifier stringifier : stringifiers) {
+ if (stringifier.handles(value.getClass())) {
+ return stringified(stringifier, value);
+ }
+ }
}
- }
- // -- HELPER
+ return bookmarkForElseFail(value);
+ }
- private static final Set<Class<? extends Serializable>> serializableFinalTypes = _Sets.of(
- String.class, String[].class,
- Class.class, Class[].class,
- Character.class, Character[].class, char[].class,
- Boolean.class, Boolean[].class, boolean[].class,
- // Numbers
- Byte[].class, byte[].class,
- Short[].class, short[].class,
- Integer[].class, int[].class,
- Long[].class, long[].class,
- Float[].class, float[].class,
- Double[].class, double[].class
- );
-
- private static final List<Class<? extends Serializable>> serializableTypes = _Lists.of(
- java.util.Date.class,
- java.sql.Date.class,
- Enum.class,
- Bookmark.class,
- TreeState.class
- );
-
- private static boolean isPredefinedSerializable(final Class<?> cls) {
- if(!Serializable.class.isAssignableFrom(cls)) {
- return false;
- }
- // primitive ... boolean, byte, char, short, int, long, float, and double.
- if(cls.isPrimitive() || Number.class.isAssignableFrom(cls)) {
- return true;
- }
- //[ahuber] any non-scalar values could be problematic, so we are careful with wild-cards here
- if(cls.getName().startsWith("java.time.")) {
- return true;
- }
- if(cls.getName().startsWith("org.joda.time.")) {
- return true;
- }
- if(serializableFinalTypes.contains(cls)) {
- return true;
- }
- return serializableTypes.stream().anyMatch(t->t.isAssignableFrom(cls));
+ private static <T> String stringified(IdStringifier<T> stringifier, T value) {
+ return stringifier.stringify(value);
}
+ // -- HELPER
+
+ @Inject List<IdStringifier<?>> stringifiers;
Review Comment:
ok, not sure if I did that here, but it turns out that we framework-defined `IdStringifier`s now anyway, so not an issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@isis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [isis] danhaywood commented on a diff in pull request #1019: Isis 3002
Posted by GitBox <gi...@apache.org>.
danhaywood commented on code in PR #1019:
URL: https://github.com/apache/isis/pull/1019#discussion_r926793043
##########
persistence/jpa/integration/src/main/java/org/apache/isis/persistence/jpa/integration/typeconverters/applib/IsisBookmarkConverter.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.apache.isis.persistence.jpa.integration.typeconverters.applib;
+
+import javax.persistence.AttributeConverter;
+import javax.persistence.Converter;
+
+import org.apache.isis.applib.services.bookmark.Bookmark;
+
+/**
+ * @since 2.0 {@index}
+ */
+@Converter(autoApply = true)
+public class IsisBookmarkConverter implements AttributeConverter<Bookmark, String> {
+
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ public String convertToDatabaseColumn(Bookmark bookmark) {
+ return bookmark != null
+ ? bookmark.toString()
+ : null;
+ }
+
+ @Override
+ public Bookmark convertToEntityAttribute(String datastoreValue) {
Review Comment:
agreed, will change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@isis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [isis] andi-huber commented on a diff in pull request #1019: Isis 3002
Posted by GitBox <gi...@apache.org>.
andi-huber commented on code in PR #1019:
URL: https://github.com/apache/isis/pull/1019#discussion_r921043096
##########
persistence/jpa/integration/src/main/java/org/apache/isis/persistence/jpa/integration/typeconverters/applib/IsisBookmarkConverter.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.apache.isis.persistence.jpa.integration.typeconverters.applib;
+
+import javax.persistence.AttributeConverter;
+import javax.persistence.Converter;
+
+import org.apache.isis.applib.services.bookmark.Bookmark;
+
+/**
+ * @since 2.0 {@index}
+ */
+@Converter(autoApply = true)
+public class IsisBookmarkConverter implements AttributeConverter<Bookmark, String> {
+
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ public String convertToDatabaseColumn(Bookmark bookmark) {
+ return bookmark != null
+ ? bookmark.toString()
+ : null;
+ }
+
+ @Override
+ public Bookmark convertToEntityAttribute(String datastoreValue) {
Review Comment:
we probably should fail if the bookmark cannot be parsed from non-empty _datastoreValue_:
```java
public Bookmark convertToEntityAttribute(String datastoreValue) {
return _Strings.isNotEmpty(datastoreValue)
? Bookmark.parseElseFail(datastoreValue)
: null;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@isis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [isis] danhaywood commented on a diff in pull request #1019: Isis 3002
Posted by GitBox <gi...@apache.org>.
danhaywood commented on code in PR #1019:
URL: https://github.com/apache/isis/pull/1019#discussion_r926785183
##########
persistence/jpa/integration/src/main/java/org/apache/isis/persistence/jpa/integration/entity/JpaEntityFacetFactory.java:
##########
@@ -393,6 +409,12 @@ private static JpaObjectIdSerializer createJpaObjectIdSerializer(
return new ByteIdSerializer();
}
}
+ Can<BookmarkService.Stringifier> select = serviceRegistry.select(BookmarkService.Stringifier.class);
+ for (BookmarkService.Stringifier stringifier : select.toList()) {
Review Comment:
Refactored since.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@isis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [isis] andi-huber commented on a diff in pull request #1019: Isis 3002
Posted by GitBox <gi...@apache.org>.
andi-huber commented on code in PR #1019:
URL: https://github.com/apache/isis/pull/1019#discussion_r920968718
##########
extensions/core/commandlog/applib/src/main/java/org/apache/isis/extensions/commandlog/applib/contributions/HasInteractionId_commandLogEntry.java:
##########
@@ -0,0 +1,41 @@
+package org.apache.isis.extensions.commandlog.applib.contributions;
Review Comment:
missing license header
##########
extensions/core/commandlog/applib/src/test/java/org/apache/isis/extensions/commandlog/applib/integtest/model/CommandLogTestDomainModel.java:
##########
@@ -0,0 +1,4 @@
+package org.apache.isis.extensions.commandlog.applib.integtest.model;
Review Comment:
missing license header
##########
extensions/core/commandlog/applib/src/test/java/org/apache/isis/extensions/commandlog/applib/integtest/model/Counter.java:
##########
@@ -0,0 +1,50 @@
+package org.apache.isis.extensions.commandlog.applib.integtest.model;
Review Comment:
missing license header
##########
extensions/core/commandlog/persistence-jdo/src/test/java/org/apache/isis/extensions/commandlog/jdo/model/CounterRepository.java:
##########
@@ -0,0 +1,12 @@
+package org.apache.isis.extensions.commandlog.jdo.model;
Review Comment:
missing license header
##########
extensions/core/commandlog/persistence-jpa/src/test/resources/META-INF/persistence.xml:
##########
@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8"?>
Review Comment:
missing license header
##########
extensions/core/commandlog/persistence-jpa/src/test/java/org/apache/isis/extensions/commandlog/jpa/model/CounterRepository.java:
##########
@@ -0,0 +1,13 @@
+package org.apache.isis.extensions.commandlog.jpa.model;
Review Comment:
missing license header
##########
extensions/core/executionlog/applib/src/main/java/org/apache/isis/extensions/executionlog/applib/dom/ExecutionLogEntryPK.java:
##########
@@ -0,0 +1,37 @@
+package org.apache.isis.extensions.executionlog.applib.dom;
Review Comment:
missing license header
##########
extensions/core/executionlog/applib/src/main/java/org/apache/isis/extensions/executionlog/applib/dom/ExecutionLogEntryRepository.java:
##########
@@ -0,0 +1,157 @@
+package org.apache.isis.extensions.executionlog.applib.dom;
Review Comment:
missing license header
##########
extensions/core/executionlog/applib/src/main/java/org/apache/isis/extensions/executionlog/applib/dom/ExecutionLogEntryType.java:
##########
@@ -0,0 +1,6 @@
+package org.apache.isis.extensions.executionlog.applib.dom;
Review Comment:
missing license header
##########
api/applib/src/main/java/org/apache/isis/applib/services/bookmark/IdStringifier.java:
##########
@@ -0,0 +1,48 @@
+package org.apache.isis.applib.services.bookmark;
Review Comment:
missing license header
##########
api/applib/src/main/java/org/apache/isis/applib/services/bookmark/IdStringifier.java:
##########
@@ -0,0 +1,48 @@
+package org.apache.isis.applib.services.bookmark;
+
+import java.util.Optional;
+
+import lombok.Getter;
+
+/**
+ * SPI to allow other modules to extend the bookmarking mechanism.
+ *
+ * <p>
+ * Originally introduced to allow JPA CommandLog implementation use CommandLogEntryPK for its primary key.
+ * </p>
+ */
+public interface IdStringifier<T> {
+
+ boolean handles(Class<?> candidateClass);
+
+ Class<T> getHandledClass();
Review Comment:
in theory could be more than one (handled class)
##########
persistence/jpa/integration/src/main/java/org/apache/isis/persistence/jpa/integration/typeconverters/applib/IsisBookmarkConverter.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.apache.isis.persistence.jpa.integration.typeconverters.applib;
+
+import javax.persistence.AttributeConverter;
+import javax.persistence.Converter;
+
+import org.apache.isis.applib.services.bookmark.Bookmark;
+
+/**
+ * @since 2.0 {@index}
+ */
+@Converter(autoApply = true)
+public class IsisBookmarkConverter implements AttributeConverter<Bookmark, String> {
+
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ public String convertToDatabaseColumn(Bookmark bookmark) {
+ return bookmark != null
+ ? bookmark.toString()
+ : null;
+ }
+
+ @Override
+ public Bookmark convertToEntityAttribute(String datastoreValue) {
Review Comment:
we probably should fail if the bookmark cannot be parsed from non-empty _datastoreValue_:
```java
public Bookmark convertToEntityAttribute(String datastoreValue) {
return _String.isNotEmpty(datastoreValue)
? Bookmark.parseElseFail(datastoreValue)
: null;
}
```
##########
extensions/core/commandlog/applib/src/test/java/org/apache/isis/extensions/commandlog/applib/integtest/model/CounterRepository.java:
##########
@@ -0,0 +1,31 @@
+package org.apache.isis.extensions.commandlog.applib.integtest.model;
Review Comment:
missing license header
##########
core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/actions/action/invocation/IdentifierUtil.java:
##########
@@ -109,19 +120,39 @@ public Identifier memberIdentifierFor(
/**
* Whether given command corresponds to given objectMember.
* <p>
- * Is related to {@link #logicalMemberIdentifierFor(ObjectMember)}.
+ * Is related to {@link #logicalMemberIdentifierForDeclaredMember(ObjectMember)}.
*/
public boolean isCommandForMember(
final @Nullable Command command,
+ final @NonNull InteractionHead interactionHead,
final @Nullable ObjectMember objectMember) {
return command!=null
&& objectMember!=null
- && logicalMemberIdentifierFor(objectMember)
+ && logicalMemberIdentifierFor(interactionHead, objectMember)
.equals(command.getLogicalMemberIdentifier());
}
// -- HELPER
+ private String logicalMemberIdentifierFor(
+ final @NonNull InteractionHead interactionHead,
+ final ObjectMember objectMember) {
+ if (objectMember instanceof ObjectAction) {
+ ObjectAction objectAction = (ObjectAction) objectMember;
+ if (objectAction.isDeclaredOnMixin()) {
+ if (interactionHead instanceof ActionInteractionHead) {
+ ObjectAction objectActionOnMixee = ((ActionInteractionHead) interactionHead).getMetaModel();
+ ObjectSpecification specificationOfMixee = interactionHead.getOwner().getSpecification();
+ return logicalMemberIdentifierFor(specificationOfMixee, objectActionOnMixee);
+ }
+ }
+ // we fall through to the declared case, which isn't correct; but don't think it matters because
Review Comment:
looks correct to me!
maybe I'll revisit this `IdentifierUtil` post merge (and remove ambiguities if there are any)
##########
extensions/core/commandlog/applib/src/test/java/org/apache/isis/extensions/commandlog/applib/integtest/model/Counter_bumpUsingMixinWithCommandPublishingDisabled.java:
##########
@@ -0,0 +1,17 @@
+package org.apache.isis.extensions.commandlog.applib.integtest.model;
Review Comment:
missing license header
##########
extensions/core/commandlog/applib/src/test/java/org/apache/isis/extensions/commandlog/applib/integtest/model/Counter_bumpUsingMixin.java:
##########
@@ -0,0 +1,17 @@
+package org.apache.isis.extensions.commandlog.applib.integtest.model;
Review Comment:
missing license header
##########
extensions/core/commandlog/persistence-jdo/src/test/java/org/apache/isis/extensions/commandlog/jdo/model/Counter.java:
##########
@@ -0,0 +1,48 @@
+package org.apache.isis.extensions.commandlog.jdo.model;
Review Comment:
missing license header
##########
extensions/core/commandlog/persistence-jpa/src/test/java/org/apache/isis/extensions/commandlog/jpa/model/Counter.java:
##########
@@ -0,0 +1,55 @@
+package org.apache.isis.extensions.commandlog.jpa.model;
Review Comment:
missing license header
##########
extensions/core/executionlog/applib/src/main/java/org/apache/isis/extensions/executionlog/applib/contributions/ExecutionLogEntry_siblingExecutions.java:
##########
@@ -0,0 +1,36 @@
+package org.apache.isis.extensions.executionlog.applib.contributions;
Review Comment:
missing license header
##########
extensions/core/executionlog/applib/src/main/java/org/apache/isis/extensions/executionlog/applib/contributions/HasInteractionId_executionLogEntries.java:
##########
@@ -0,0 +1,33 @@
+package org.apache.isis.extensions.executionlog.applib.contributions;
Review Comment:
missing license header
##########
extensions/core/executionlog/applib/src/main/java/org/apache/isis/extensions/executionlog/applib/dom/ExecutionLogEntry.layout.fallback.xml:
##########
@@ -0,0 +1,97 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
Review Comment:
missing license header
##########
extensions/core/executionlog/applib/src/main/java/org/apache/isis/extensions/executionlog/applib/spiimpl/ExecutionSubscriberForLog.java:
##########
@@ -0,0 +1,24 @@
+package org.apache.isis.extensions.executionlog.applib.spiimpl;
Review Comment:
missing license header
##########
persistence/jpa/integration/src/main/java/org/apache/isis/persistence/jpa/integration/entity/JpaEntityFacetFactory.java:
##########
@@ -393,6 +409,12 @@ private static JpaObjectIdSerializer createJpaObjectIdSerializer(
return new ByteIdSerializer();
}
}
+ Can<BookmarkService.Stringifier> select = serviceRegistry.select(BookmarkService.Stringifier.class);
+ for (BookmarkService.Stringifier stringifier : select.toList()) {
Review Comment:
no need to convert `Can` to `List` ... `Can` implements `Iterable`
should just work fine ...
```java
for (BookmarkService.Stringifier stringifier : select) { ...
```
##########
persistence/jpa/integration/src/main/java/org/apache/isis/persistence/jpa/integration/entity/JpaEntityFacetFactory.java:
##########
@@ -393,6 +409,12 @@ private static JpaObjectIdSerializer createJpaObjectIdSerializer(
return new ByteIdSerializer();
}
}
+ Can<BookmarkService.Stringifier> select = serviceRegistry.select(BookmarkService.Stringifier.class);
+ for (BookmarkService.Stringifier stringifier : select.toList()) {
+ if(stringifier.handles() == primaryKeyType) {
Review Comment:
If this is the only kind of use for the `stringifier.handles()` SPI method, I'd suggest to make the SPI more flexible by instead using a predicate like
```java
if(stringifier.handles(primaryKeyType)) ...
```
which would allow a `Stringifier` to handle multiple types (if need be).
If not, this is just fine.
##########
extensions/core/executionlog/applib/src/main/java/org/apache/isis/extensions/executionlog/applib/util/BigDecimalUtils.java:
##########
@@ -0,0 +1,27 @@
+package org.apache.isis.extensions.executionlog.applib.util;
Review Comment:
missing license header
With utility classes, what I usually ask myself: can its scope be made package private?
if **yes**, then do so and also prefix the class-name with an underscore.
if **no**, maybe this is a candidate for the commons artifact, otherwise leave it, its fine.
##########
api/applib/src/main/java/org/apache/isis/applib/services/bookmark/IdStringifier.java:
##########
@@ -0,0 +1,48 @@
+package org.apache.isis.applib.services.bookmark;
+
+import java.util.Optional;
+
+import lombok.Getter;
+
+/**
+ * SPI to allow other modules to extend the bookmarking mechanism.
+ *
+ * <p>
+ * Originally introduced to allow JPA CommandLog implementation use CommandLogEntryPK for its primary key.
+ * </p>
+ */
+public interface IdStringifier<T> {
+
+ boolean handles(Class<?> candidateClass);
+
+ Class<T> getHandledClass();
+
+ String stringify(final T value);
+
+ T parse(final String stringified);
+
+ abstract class Abstract<T> implements IdStringifier<T> {
+
+ @Getter
+ private final Class<T> handledClass;
+ /**
+ * Allows for a Stringifier to handle (for example) both <code>Integer.class</code> and <code>int.class</code>.
+ */
+ @Getter
+ private final Optional<Class<T>> primitiveClass;
Review Comment:
I guess we could simply imply, eg. that any `IdStringifier` that handles `Long` must also handle `long`. (Which it trivially does anyway.)
Eg. As is the case for our value-semantics design. We simply imply that `ValueSemanticsProvider<Long>` also handles the primitive type (`long`).
(Anyway just an idea, might not be applicable here.)
##########
extensions/core/commandlog/persistence-jpa/src/main/java/org/apache/isis/extensions/commandlog/jpa/dom/CommandLogEntryPK.java:
##########
@@ -0,0 +1,57 @@
+package org.apache.isis.extensions.commandlog.jpa.dom;
Review Comment:
missing license header
##########
core/runtimeservices/src/main/java/org/apache/isis/core/runtimeservices/bookmarks/BookmarkServiceDefault.java:
##########
@@ -141,72 +144,47 @@ public Bookmark bookmarkForElseFail(final @Nullable Object domainObject) {
@Override
public <T> T read(final Class<T> cls, final Serializable value) {
+ if (stringifiers != null) {
+ for (IdStringifier<?> serializer : stringifiers) {
+ if (serializer.handles(cls)) {
+ return (T) serializer.parse((String)value);
+ }
+ }
+ }
+
if(Bookmark.class.equals(cls)) {
return _Casts.uncheckedCast(value);
}
if(Bookmark.class.isAssignableFrom(value.getClass())) {
- final Bookmark valueBookmark = (Bookmark) value;
- return _Casts.uncheckedCast(lookup(valueBookmark).orElse(null));
+ final Bookmark valueAsBookmark = (Bookmark) value;
+ return _Casts.uncheckedCast(lookup(valueAsBookmark).orElse(null));
}
return _Casts.uncheckedCast(value);
}
+ @SuppressWarnings("unchecked")
@Override
public Serializable write(final Object value) {
- if(isPredefinedSerializable(value.getClass())) {
- return (Serializable) value;
- } else {
- return bookmarkForElseFail(value);
+ if (stringifiers != null) {
+ for (IdStringifier stringifier : stringifiers) {
+ if (stringifier.handles(value.getClass())) {
+ return stringified(stringifier, value);
+ }
+ }
}
- }
- // -- HELPER
+ return bookmarkForElseFail(value);
+ }
- private static final Set<Class<? extends Serializable>> serializableFinalTypes = _Sets.of(
- String.class, String[].class,
- Class.class, Class[].class,
- Character.class, Character[].class, char[].class,
- Boolean.class, Boolean[].class, boolean[].class,
- // Numbers
- Byte[].class, byte[].class,
- Short[].class, short[].class,
- Integer[].class, int[].class,
- Long[].class, long[].class,
- Float[].class, float[].class,
- Double[].class, double[].class
- );
-
- private static final List<Class<? extends Serializable>> serializableTypes = _Lists.of(
- java.util.Date.class,
- java.sql.Date.class,
- Enum.class,
- Bookmark.class,
- TreeState.class
- );
-
- private static boolean isPredefinedSerializable(final Class<?> cls) {
- if(!Serializable.class.isAssignableFrom(cls)) {
- return false;
- }
- // primitive ... boolean, byte, char, short, int, long, float, and double.
- if(cls.isPrimitive() || Number.class.isAssignableFrom(cls)) {
- return true;
- }
- //[ahuber] any non-scalar values could be problematic, so we are careful with wild-cards here
- if(cls.getName().startsWith("java.time.")) {
- return true;
- }
- if(cls.getName().startsWith("org.joda.time.")) {
- return true;
- }
- if(serializableFinalTypes.contains(cls)) {
- return true;
- }
- return serializableTypes.stream().anyMatch(t->t.isAssignableFrom(cls));
+ private static <T> String stringified(IdStringifier<T> stringifier, T value) {
+ return stringifier.stringify(value);
}
+ // -- HELPER
+
+ @Inject List<IdStringifier<?>> stringifiers;
Review Comment:
The null-safe idiom for that (eg. when JUnit testing) is rather
```java
@Autowired(required = false) List<IdStringifier<?>> stringifiers;
```
Otherwise, in the absence of a/any `IdStringifier` _Spring's_ `ApplicationContext` will fail to initialize.
##########
persistence/jpa/integration/src/main/java/org/apache/isis/persistence/jpa/integration/entity/JpaEntityFacet.java:
##########
@@ -0,0 +1,368 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.apache.isis.persistence.jpa.integration.entity;
+
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Optional;
+
+import javax.persistence.EntityManager;
+import javax.persistence.PersistenceException;
+import javax.persistence.PersistenceUnitUtil;
+import javax.persistence.metamodel.EntityType;
+
+import org.eclipse.persistence.exceptions.DescriptorException;
+import org.springframework.data.jpa.repository.JpaContext;
+
+import org.apache.isis.applib.exceptions.unrecoverable.ObjectNotFoundException;
+import org.apache.isis.applib.query.AllInstancesQuery;
+import org.apache.isis.applib.query.NamedQuery;
+import org.apache.isis.applib.query.Query;
+import org.apache.isis.applib.services.bookmark.Bookmark;
+import org.apache.isis.applib.services.bookmark.IdStringifier;
+import org.apache.isis.applib.services.registry.ServiceRegistry;
+import org.apache.isis.applib.services.repository.EntityState;
+import org.apache.isis.commons.collections.Can;
+import org.apache.isis.commons.handler.ChainOfResponsibility;
+import org.apache.isis.commons.internal.assertions._Assert;
+import org.apache.isis.commons.internal.base._Casts;
+import org.apache.isis.commons.internal.base._Lazy;
+import org.apache.isis.commons.internal.exceptions._Exceptions;
+import org.apache.isis.core.config.beans.PersistenceStack;
+import org.apache.isis.core.metamodel.facetapi.FacetAbstract;
+import org.apache.isis.core.metamodel.facetapi.FacetHolder;
+import org.apache.isis.core.metamodel.facets.object.entity.EntityFacet;
+import org.apache.isis.core.metamodel.spec.ManagedObject;
+import org.apache.isis.core.metamodel.spec.ObjectSpecification;
+
+import lombok.NonNull;
+import lombok.val;
+import lombok.extern.log4j.Log4j2;
+
+@Log4j2
+public class JpaEntityFacet
+ extends FacetAbstract
+ implements EntityFacet {
+
+ private final Class<?> entityClass;
+ private final ServiceRegistry serviceRegistry;
+ private final List<IdStringifier<?>> idStringifiers;
Review Comment:
could use a `Can` instead of `List` (so no need to convert on assignment)
(remember, `Can` has clear semantics of being immutable)
##########
persistence/jdo/datanucleus/src/main/java/org/apache/isis/persistence/jdo/datanucleus/metamodel/facets/entity/JdoEntityFacet.java:
##########
@@ -80,6 +81,7 @@
@Inject private ObjectManager objectManager;
@Inject private ExceptionRecognizerService exceptionRecognizerService;
@Inject private JdoFacetContext jdoFacetContext;
+ @Inject private List<IdStringifier> idStringifiers;
Review Comment:
might be irrelevant, but probably use
```java
@Autowired(required=false) private List<IdStringifier> idStringifiers;
```
just to be safe when JUnit testing, and no IdStringifiers are registered
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@isis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [isis] danhaywood commented on a diff in pull request #1019: Isis 3002
Posted by GitBox <gi...@apache.org>.
danhaywood commented on code in PR #1019:
URL: https://github.com/apache/isis/pull/1019#discussion_r926786447
##########
api/applib/src/main/java/org/apache/isis/applib/services/bookmark/IdStringifier.java:
##########
@@ -0,0 +1,48 @@
+package org.apache.isis.applib.services.bookmark;
+
+import java.util.Optional;
+
+import lombok.Getter;
+
+/**
+ * SPI to allow other modules to extend the bookmarking mechanism.
+ *
+ * <p>
+ * Originally introduced to allow JPA CommandLog implementation use CommandLogEntryPK for its primary key.
+ * </p>
+ */
+public interface IdStringifier<T> {
+
+ boolean handles(Class<?> candidateClass);
+
+ Class<T> getHandledClass();
+
+ String stringify(final T value);
+
+ T parse(final String stringified);
+
+ abstract class Abstract<T> implements IdStringifier<T> {
+
+ @Getter
+ private final Class<T> handledClass;
+ /**
+ * Allows for a Stringifier to handle (for example) both <code>Integer.class</code> and <code>int.class</code>.
+ */
+ @Getter
+ private final Optional<Class<T>> primitiveClass;
Review Comment:
it was applicable, this is how it's implemented.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@isis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [isis] danhaywood merged pull request #1019: Isis 3002
Posted by GitBox <gi...@apache.org>.
danhaywood merged PR #1019:
URL: https://github.com/apache/isis/pull/1019
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@isis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org