Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): add falsey check in hasProperty; prevents error w/ namingConventions improperly mapped names #573

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

jersmart
Copy link

@jersmart jersmart commented Sep 6, 2023

There are a few issues I noticed when debugging the same error found in these issues:

#525
#551

  1. When using naming conventions, the naming convention configuration override will be incorrect during InitialMapping, I think because it uses the namingConvention provided at the mapping level. This can result in some funky behavior when initial mapping happens, like separating my_thing into ["my","_Thing"].
  2. If it finds a similarly named field in the destination (in the example above, "my", it will try to treat that field as an object even when it is not an object, resulting in hasProperty throwing an error due to the obj parameter being undefined.

Example code that causes an issue:

import {
  CamelCaseNamingConvention,
  SnakeCaseNamingConvention,
  addProfile,
  createMap,
  createMapper,
  forMember,
  mapFrom,
  namingConventions,
} from "@automapper/core";
import { PojosMetadataMap, pojos } from "@automapper/pojos";

export interface ISubThingOne {
  name: string;
  description: string;
  imageUrls: string[];
}

export interface IThingOne {
  currency: string;
  unitAmount: number;
  productData: ISubThingOne;
  somethingElse: string;
}

export interface IThingTwo {
  currency_code: string;
  value: number;
  something_else: string;
}

PojosMetadataMap.create<ISubThingOne>("ISubThingOne", {
  name: String,
  description: String,
  imageUrls: [String],
});

PojosMetadataMap.create<IThingOne>("IThingOne", {
  currency: String,
  unitAmount: Number,
  productData: "ISubThingOne",
  somethingElse: String,
});

PojosMetadataMap.create<IThingTwo>("IThingTwo", {
  currency_code: String,
  value: Number,
  something_else: String,
});

const mapper = createMapper({
  strategyInitializer: pojos(),
  namingConventions: new CamelCaseNamingConvention(),
});

addProfile(
  mapper,
  (mapper) => {
    createMap<IThingOne, IThingTwo>(
      mapper,
      "IThingOne",
      "IThingTwo",
      forMember(
        (d) => d.currency_code,
        mapFrom((s) => s.currency)
      ),
      forMember(
        (d) => d.value,
        mapFrom((s) => s.unitAmount)
      )
    );
  },
  namingConventions({
    source: new CamelCaseNamingConvention(),
    destination: new SnakeCaseNamingConvention(),
  })
);

const thingOne: IThingOne = {
  currency: "USD",
  unitAmount: 100,
  productData: {} as ISubThingOne,
  somethingElse: "else",
};

const thingTwo = mapper.map<IThingOne, IThingTwo>(
  thingOne,
  "IThingOne",
  "IThingTwo"
);

console.log(thingTwo);

This PR fixes only issue (2) from the list above. There seems to be another issue that namingConventions overrides are loaded too late for initial mapping. However, once something is actually mapped, it works as expected, resulting in the following output:

{ something_else: 'else', currency_code: 'USD', value: 100 }

ISSUES CLOSED: #525, #551

@sonarcloud
Copy link

sonarcloud bot commented Sep 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

timonmasberg and others added 6 commits January 22, 2024 19:30
* feat(nestjs): bump nestjs dependencies to v10

* feat(nestjs): target es2021
* fix(mikro): infinite loop when using OneToOne eager loading

* fix(mikro): mapping custom property of mikro entity to automapper
* fix: ignore import path transformations on win32 platform

* fix: fixed a "code smell" to pass the code analysis
Copy link

sonarcloud bot commented Jan 22, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.5% Duplication on New Code

See analysis details on SonarCloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compilation fails if property name is start (substring) of another property
7 participants