Skip to content

Commit

Permalink
Merge branch 'master' into improve-duplicate-column-detection-for-ord…
Browse files Browse the repository at this point in the history
…er-by
  • Loading branch information
stariy95 authored May 31, 2024
2 parents 29dadda + e25fbbd commit 1e7d25a
Show file tree
Hide file tree
Showing 18 changed files with 308 additions and 39 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/verify-deploy-on-push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ jobs:

steps:
- name: Checkout repository
uses: actions/checkout@v3
uses: actions/checkout@v4

- name: Setup java
uses: actions/setup-java@v3
uses: actions/setup-java@v4
with:
distribution: 'temurin'
java-version: ${{ matrix.jdk }}
Expand All @@ -60,13 +60,13 @@ jobs:

steps:
- name: Checkout repository
uses: actions/checkout@v3
uses: actions/checkout@v4

- name: Export version
uses: ./.github/actions/export-pom-version

- name: Setup java
uses: actions/setup-java@v3
uses: actions/setup-java@v4
if: contains(env.POM_VERSION, '-SNAPSHOT')
with:
distribution: 'temurin'
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/verify-on-pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ jobs:

steps:
- name: Checkout repository
uses: actions/checkout@v3
uses: actions/checkout@v4

- name: Setup java
uses: actions/setup-java@v3
uses: actions/setup-java@v4
with:
distribution: 'temurin'
java-version: ${{ matrix.jdk }}
Expand Down
5 changes: 4 additions & 1 deletion RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -103,5 +103,8 @@ CAY-2841 Multi column ColumnSelect with SHARED_CACHE fails after 1st select
CAY-2844 Joint prefetch doesn't use ObjEntity qualifier
CAY-2842 Prevent duplicate select columns when using distinct with order by
CAY-2847 Improve duplicate select column detection when using order by
CAY-2848 Vertical Inheritance: Updating one-to-many with inverse nullifies other columns
CAY-2850 Query using Clob comparison with empty String fails
CAY-2851 Replace Existing OneToOne From New Object
CAY-2851 Replace Existing OneToOne From New Object
CAY-2853 Incorrect deletion of entities from flattened attributes
CAY-2854 Improve delete prevention detection of flattened attribute row
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.apache.cayenne.map.DbEntity;
import org.apache.cayenne.map.EntityResolver;
import org.apache.cayenne.map.ObjEntity;
import org.apache.cayenne.reflect.AdditionalDbEntityDescriptor;
import org.apache.cayenne.reflect.ClassDescriptor;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -182,8 +183,8 @@ private void resolveAdditionalIds(DataRow row, Persistent object, ClassDescripto
return;
}

for(Map.Entry<CayennePath, DbEntity> entry : classDescriptor.getAdditionalDbEntities().entrySet()) {
DbEntity dbEntity = entry.getValue();
for(Map.Entry<CayennePath, AdditionalDbEntityDescriptor> entry : classDescriptor.getAdditionalDbEntities().entrySet()) {
DbEntity dbEntity = entry.getValue().getDbEntity();
CayennePath path = entry.getKey();
CayennePath prefix = path.length() == 1 ? path : path.tail(path.length() - 1);
ObjectId objectId = createObjectId(row, "db:" + dbEntity.getName(), dbEntity.getPrimaryKeys(), prefix, false);
Expand Down
14 changes: 13 additions & 1 deletion cayenne/src/main/java/org/apache/cayenne/access/ObjectStore.java
Original file line number Diff line number Diff line change
Expand Up @@ -979,7 +979,7 @@ public void arcDeleted(Object nodeId, Object targetNodeId, ArcId arcId) {
* Check that flattened path for given object ID has data row in DB.
* @since 4.1
*/
boolean hasFlattenedPath(ObjectId objectId, String path) {
boolean hasFlattenedPath(ObjectId objectId, CayennePath path) {
if(trackedFlattenedPaths == null) {
return false;
}
Expand Down Expand Up @@ -1011,6 +1011,18 @@ public Collection<ObjectId> getFlattenedIds(ObjectId objectId) {
.getOrDefault(objectId, Collections.emptyMap()).values();
}

/**
* @since 5.0
*/
public Map<CayennePath,ObjectId> getFlattenedPathIdMap(ObjectId objectId) {
if(trackedFlattenedPaths == null) {
return Collections.emptyMap();
}

return trackedFlattenedPaths
.getOrDefault(objectId, Collections.emptyMap());
}

/**
* Mark that flattened path for object has data row in DB.
* @since 4.1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,23 +89,29 @@ private void processArcChange(Object nodeId, Object targetNodeId, ArcId arcId, b
}

if(objRelationship.isFlattened()) {
processFlattenedPath(arcTarget.getSourceId(), arcTarget.getTargetId(), entity.getDbEntity(),
FlattenedPathProcessingResult result = processFlattenedPath(arcTarget.getSourceId(), arcTarget.getTargetId(), entity.getDbEntity(),
objRelationship.getDbRelationshipPath(), created);
if(result.isProcessed()) {
factory.getProcessedArcs().add(arcTarget);
}
} else {
DbRelationship dbRelationship = objRelationship.getDbRelationships().get(0);
processRelationship(dbRelationship, arcTarget.getSourceId(), arcTarget.getTargetId(), created);
factory.getProcessedArcs().add(arcTarget);
}

factory.getProcessedArcs().add(arcTarget);
}

ObjectId processFlattenedPath(ObjectId id, ObjectId finalTargetId, DbEntity entity, CayennePath dbPath, boolean add) {
Iterator<CayenneMapEntry> dbPathIterator = entity.resolvePathComponents(dbPath);
FlattenedPathProcessingResult processFlattenedPath(ObjectId id, ObjectId finalTargetId, DbEntity entity, CayennePath dbPath, boolean add) {
if(shouldSkipFlattenedOp(id, finalTargetId)) {
return flattenedResultNotProcessed();
}

CayennePath flattenedPath = CayennePath.EMPTY_PATH;

ObjectId srcId = id;
ObjectId targetId = null;

Iterator<CayenneMapEntry> dbPathIterator = entity.resolvePathComponents(dbPath);
while(dbPathIterator.hasNext()) {
CayenneMapEntry entry = dbPathIterator.next();
flattenedPath = flattenedPath.dot(entry.getName());
Expand Down Expand Up @@ -139,8 +145,8 @@ ObjectId processFlattenedPath(ObjectId id, ObjectId finalTargetId, DbEntity enti
} else {
type = add ? DbRowOpType.INSERT : DbRowOpType.UPDATE;
factory.<DbRowOpWithValues>getOrCreate(target, targetId, type)
.getValues()
.addFlattenedId(flattenedPath, targetId);
.getValues()
.addFlattenedId(flattenedPath, targetId);
}
} else if(dbPathIterator.hasNext()) {
// should update existing DB row
Expand All @@ -151,7 +157,15 @@ ObjectId processFlattenedPath(ObjectId id, ObjectId finalTargetId, DbEntity enti
}
}

return targetId;
return flattenedResultId(targetId);
}

private boolean shouldSkipFlattenedOp(ObjectId id, ObjectId finalTargetId) {
// as we get two sides of the relationship processed,
// check if we got more information for a reverse operation
return finalTargetId != null
&& factory.getStore().getFlattenedIds(id).isEmpty()
&& !factory.getStore().getFlattenedIds(finalTargetId).isEmpty();
}

private boolean shouldProcessAsAddition(DbRelationship relationship, boolean add) {
Expand Down Expand Up @@ -281,4 +295,30 @@ public Void visitDelete(DeleteDbRowOp dbRow) {
return null;
}
}

static FlattenedPathProcessingResult flattenedResultId(ObjectId id) {
return new FlattenedPathProcessingResult(true, id);
}

static FlattenedPathProcessingResult flattenedResultNotProcessed() {
return new FlattenedPathProcessingResult(false, null);
}

final static class FlattenedPathProcessingResult {
private final boolean processed;
private final ObjectId id;

private FlattenedPathProcessingResult(boolean processed, ObjectId id) {
this.processed = processed;
this.id = id;
}

public boolean isProcessed() {
return processed;
}

public ObjectId getId() {
return id;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

package org.apache.cayenne.access.flush;

import java.util.Collection;
import java.util.Map;

import org.apache.cayenne.CayenneRuntimeException;
import org.apache.cayenne.ObjectId;
Expand All @@ -29,8 +29,11 @@
import org.apache.cayenne.access.flush.operation.DeleteDbRowOp;
import org.apache.cayenne.access.flush.operation.InsertDbRowOp;
import org.apache.cayenne.access.flush.operation.UpdateDbRowOp;
import org.apache.cayenne.exp.path.CayennePath;
import org.apache.cayenne.graph.GraphChangeHandler;
import org.apache.cayenne.map.ObjEntity;
import org.apache.cayenne.reflect.AdditionalDbEntityDescriptor;
import org.apache.cayenne.reflect.ClassDescriptor;

/**
* Visitor that runs all required actions based on operation type.
Expand Down Expand Up @@ -74,14 +77,23 @@ public Void visitUpdate(UpdateDbRowOp dbRow) {

@Override
public Void visitDelete(DeleteDbRowOp dbRow) {
if (dbRowOpFactory.getDescriptor().getEntity().isReadOnly()) {
ObjEntity entity = dbRowOpFactory.getDescriptor().getEntity();
if (entity.isReadOnly()) {
throw new CayenneRuntimeException("Attempt to modify object(s) mapped to a read-only entity: '%s'. " +
"Can't commit changes.", dbRowOpFactory.getDescriptor().getEntity().getName());
"Can't commit changes.", entity.getName());
}
diff.apply(deleteHandler);
Collection<ObjectId> flattenedIds = dbRowOpFactory.getStore().getFlattenedIds(dbRow.getChangeId());
flattenedIds.forEach(id -> dbRowOpFactory.getOrCreate(dbRowOpFactory.getDbEntity(id), id, DbRowOpType.DELETE));
if (dbRowOpFactory.getDescriptor().getEntity().getDeclaredLockType() == ObjEntity.LOCK_TYPE_OPTIMISTIC) {

ClassDescriptor descriptor = dbRowOpFactory.getDescriptor();
Map<CayennePath,ObjectId> flattenedPathIdMap = dbRowOpFactory.getStore().getFlattenedPathIdMap(dbRow.getChangeId());
flattenedPathIdMap.forEach((path, id) -> {
AdditionalDbEntityDescriptor addEntity = descriptor.getAdditionalDbEntities().get(path);
if (!addEntity.noDelete()) {// See PersistentDescriptorFactory.indexAdditionalDbEntities
dbRowOpFactory.getOrCreate(addEntity.getDbEntity(), id, DbRowOpType.DELETE);
}
});

if (entity.getDeclaredLockType() == ObjEntity.LOCK_TYPE_OPTIMISTIC) {
dbRowOpFactory.getDescriptor().visitAllProperties(new OptimisticLockQualifierBuilder(dbRow, diff));
}
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ public void nodePropertyChanged(Object nodeId, String property, Object oldValue,

if(attribute.isFlattened()) {
// get target row ID
id = processFlattenedPath(id, null, dbEntity, attribute.getDbAttributePath(), newValue != null);
FlattenedPathProcessingResult result
= processFlattenedPath(id, null, dbEntity, attribute.getDbAttributePath(), newValue != null);
if(result.isProcessed()) {
id = result.getId();
}
}

if(id == null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*****************************************************************
* 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
*
* https://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.cayenne.reflect;

import org.apache.cayenne.exp.path.CayennePath;
import org.apache.cayenne.map.DbEntity;

/**
* A descriptor for an additional DB entity attached to the main one.
* For now additional entity is spawn by flattened attributes.
*
* @see PersistentDescriptorFactory#indexAdditionalDbEntities(PersistentDescriptor)
*
* @since 5.0
*/
public class AdditionalDbEntityDescriptor {
private final CayennePath path;
private final DbEntity entity;
private final boolean noDelete;

/**
* @param path relative to the root entity path
* @param entity target of this descriptor
* @param noDelete should row deletion of the root entity trigger deletion of the additional entity
*/
AdditionalDbEntityDescriptor(CayennePath path, DbEntity entity, boolean noDelete) {
this.noDelete = noDelete;
this.entity = entity;
this.path = path;
}

public DbEntity getDbEntity() {
return entity;
}

public CayennePath getPath() {
return path;
}

public boolean noDelete() {
return noDelete;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,11 @@ public interface ClassDescriptor {
* <p>
* Keys are full paths for corresponding flattened attributes.
* <p>
*
* @since 4.1
*
* @since 5.0
* @return information about additional db entities
*/
Map<CayennePath, DbEntity> getAdditionalDbEntities();
Map<CayennePath, AdditionalDbEntityDescriptor> getAdditionalDbEntities();

/**
* @since 3.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public Collection<DbEntity> getRootDbEntities() {
}

@Override
public Map<CayennePath, DbEntity> getAdditionalDbEntities() {
public Map<CayennePath, AdditionalDbEntityDescriptor> getAdditionalDbEntities() {
checkDescriptorInitialized();
return descriptor.getAdditionalDbEntities();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public class PersistentDescriptor implements ClassDescriptor {

protected ObjEntity entity;
protected Collection<DbEntity> rootDbEntities;
protected Map<CayennePath, DbEntity> additionalDbEntities;
protected Map<CayennePath, AdditionalDbEntityDescriptor> additionalDbEntities;

protected EntityInheritanceTree entityInheritanceTree;

Expand Down Expand Up @@ -128,12 +128,12 @@ public void addRootDbEntity(DbEntity dbEntity) {
* @param path path for entity
* @param targetEntity additional entity
*/
void addAdditionalDbEntity(CayennePath path, DbEntity targetEntity) {
void addAdditionalDbEntity(CayennePath path, DbEntity targetEntity, boolean noDelete) {
if(additionalDbEntities == null) {
additionalDbEntities = new HashMap<>();
}

additionalDbEntities.put(path, targetEntity);
additionalDbEntities.put(path, new AdditionalDbEntityDescriptor(path, targetEntity, noDelete));
}

void sortProperties() {
Expand Down Expand Up @@ -232,7 +232,7 @@ public Collection<DbEntity> getRootDbEntities() {
}

@Override
public Map<CayennePath, DbEntity> getAdditionalDbEntities() {
public Map<CayennePath, AdditionalDbEntityDescriptor> getAdditionalDbEntities() {
if(additionalDbEntities == null) {
return Collections.emptyMap();
}
Expand Down
Loading

0 comments on commit 1e7d25a

Please sign in to comment.